> -----Original Message----- > From: David Marchand <david.march...@redhat.com> > Sent: Tuesday, August 22, 2023 2:12 PM > To: Zhang, Qi Z <qi.z.zh...@intel.com> > Cc: dev@dpdk.org; echau...@redhat.com; m...@redhat.com; > sta...@dpdk.org; Wu, Jingjing <jingjing...@intel.com>; Xing, Beilei > <beilei.x...@intel.com>; Doherty, Declan <declan.dohe...@intel.com>; Sinha, > Abhijit <abhijit.si...@intel.com>; Nicolau, Radu <radu.nico...@intel.com> > Subject: Re: [PATCH] net/iavf: fix checksum offloading > > On Tue, Aug 22, 2023 at 3:52 AM Zhang, Qi Z <qi.z.zh...@intel.com> wrote: > > > > > > > > > -----Original Message----- > > > From: David Marchand <david.march...@redhat.com> > > > Sent: Tuesday, August 22, 2023 1:29 AM > > > To: Zhang, Qi Z <qi.z.zh...@intel.com> > > > Cc: dev@dpdk.org; echau...@redhat.com; m...@redhat.com; > > > sta...@dpdk.org; Wu, Jingjing <jingjing...@intel.com>; Xing, Beilei > > > <beilei.x...@intel.com>; Doherty, Declan <declan.dohe...@intel.com>; > > > Sinha, Abhijit <abhijit.si...@intel.com>; Nicolau, Radu > > > <radu.nico...@intel.com> > > > Subject: Re: [PATCH] net/iavf: fix checksum offloading > > > > > > On Mon, Aug 21, 2023 at 1:54 PM Zhang, Qi Z <qi.z.zh...@intel.com> > wrote: > > > > > Subject: [PATCH] net/iavf: fix checksum offloading > > > > > > > > > > The only presence of RTE_MBUF_F_TX_IPV4 can't be used as an > > > > > indicator that a checksum offload has been requested by an > > > > > application. > > > > > > > > According to current implementation, actually the only presence of > > > RTE_MBUF_F_TX_IPV4 will cause IIPT = 10b, this scenario corresponds > > > to an > > > 'IPv4 packet with no IP checksum offload,' according to datasheet. > > > > So, I assume in this situation, the PMD continues to operate > > > > under the > > > assumption that the application has not requested checksum offloading. > > > > > > > > Could you share more insight what is the failure, maybe we can > > > > perform a > > > more comprehensive investigation? > > > > > > I think the missing piece is that OVS passes a l2_len == l3_len == 0. > > > In our tests, we could see that tx_errors get incremented for each > > > failed packet to transmit. > > > > OK, do you think to ignore RTE_MBUF_F_TX_IPV4 when l3_len = 0 is a better > fix? > > Looking at the mbuf API, l2_len and l3_len should be considered by a driver if > ol_flags contains at least one of RTE_MBUF_F_TX_SEC_OFFLOAD, > RTE_MBUF_F_TX_TUNNEL_*, RTE_MBUF_F_TX_TCP_SEG, > RTE_MBUF_F_TX_(IP|TCP|UDP|SCTP)_CKSUM. > Here, it is not the case. > > If the driver reads l2_len or l3_len, this is an undefined behavior: > for example, OVS might have been using l2_len or l3_len for its internal uses > (though I agree it would be risky for an application to do so). > > We probably need to fix access to l2_len a few lines before my patch. > > if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK && > !(m->ol_flags & RTE_MBUF_F_TX_SEC_OFFLOAD)) > offset |= (m->outer_l2_len >> 1) > << IAVF_TX_DESC_LENGTH_MACLEN_SHIFT; > else > offset |= (m->l2_len >> 1) > << IAVF_TX_DESC_LENGTH_MACLEN_SHIFT; > > > But to be clear, no I don't think looking at l3_len value is better: > it should not be read at all.
Yes, you may be correct; it appears that this issue is unrelated to l3_len. The primary concern is to prevent the configuration of Tx descriptors with incorrect values. Based on your description, it seems the problem arises when the PMD sets MACLEN to 0 and also configures IIPT as 01b, Is this correct? To prevent this issue, we could implement a check where, if l2_len is 0, we simply ignore the IIPT configuration and keep it at 0. (which leads to same configure with your patch) Regarding your mention of 'fix access to l2_len,' if l2_len is 0, there's no change in the offset regardless of whether l2_len is accessed or not. Did you mean setting a fixed value of MACLEN to 14?" > > > -- > David Marchand
