> -----Original Message----- > From: ALOK TIWARI <[email protected]> > Sent: Wednesday, November 5, 2025 11:02 AM > To: Loktionov, Aleksandr <[email protected]>; Kitszel, > Przemyslaw <[email protected]>; Nguyen, Anthony L > <[email protected]> > Cc: [email protected]; [email protected] > Subject: [query] i40e: Clarification on mask.src_ip[0] & tcf.dst_ip[0] > usage in i40e_vc_del_cloud_filter() > > > > Hi, > > In the function i40e_vc_del_cloud_filter() in > drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c, > I came across the following segment handling IPv4 flow deletion: > > case VIRTCHNL_TCP_V4_FLOW: > cfilter.n_proto = ETH_P_IP; > if (mask.dst_ip[0] & tcf.dst_ip[0]) > memcpy(&cfilter.ip.v4.dst_ip, tcf.dst_ip, > ARRAY_SIZE(tcf.dst_ip)); > else if (mask.src_ip[0] & tcf.dst_ip[0]) > memcpy(&cfilter.ip.v4.src_ip, tcf.src_ip, > ARRAY_SIZE(tcf.dst_ip)); > break; > > I wanted to check the intent behind using tcf.dst_ip[0] in the > mask.src_ip[0] condition. > is there a specific reason for referencing the destination IP in this > context? > > > Thanks, > Alok > --- > diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > index 081a4526a2f0..1553157dc53a 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > @@ -3819,9 +3819,9 @@ static int i40e_vc_del_cloud_filter(struct > i40e_vf *vf, u8 *msg) > if (mask.dst_ip[0] & tcf.dst_ip[0]) > memcpy(&cfilter.ip.v4.dst_ip, tcf.dst_ip, > ARRAY_SIZE(tcf.dst_ip)); > - else if (mask.src_ip[0] & tcf.dst_ip[0]) > + else if (mask.src_ip[0] & tcf.src_ip[0]) > memcpy(&cfilter.ip.v4.src_ip, tcf.src_ip, > - ARRAY_SIZE(tcf.dst_ip)); > + ARRAY_SIZE(tcf.src_ip)); > break; > case VIRTCHNL_TCP_V6_FLOW: > cfilter.n_proto = ETH_P_IPV6;
Good day, Alok Good catch! This is indeed a bug - a copy-paste error introduced in the original implementation. The condition should check tcf.src_ip[0] when validating source IP, not tcf.dst_ip[0]. You can see this is correct in the IPv6 flow handling in the same function: case VIRTCHNL_TCP_V6_FLOW: ... if (mask.src_ip[3] & tcf.src_ip[3]) /* Correctly uses src_ip */ memcpy(&cfilter.ip.v6.src_ip6, tcf.src_ip, sizeof(cfilter.ip.v6.src_ip6)); The IPv4 code should follow the same pattern. Additionally, using ARRAY_SIZE(tcf.src_ip) is more accurate than ARRAY_SIZE(tcf.dst_ip) when copying the source IP, even though they're likely the same size. Your fix looks correct. The same bug also exists in i40e_vc_add_cloud_filter() and should be fixed there as well. This likely went unnoticed because: - The else if means this path only executes when dst_ip is not set - Most cloud filter use cases primarily filter on destination IP - The buggy condition could accidentally succeed in some cases Thank you Alex
