On Wed, Oct 25, 2023 at 11:02 AM Radu Nicolau <[email protected]> wrote: > > > On 25-Oct-23 12:30 AM, Zhang, Qi Z wrote: > > > >> -----Original Message----- > >> From: Nicolau, Radu <[email protected]> > >> Sent: Tuesday, October 24, 2023 10:49 PM > >> To: Zhang, Qi Z <[email protected]>; Marchand, David > >> <[email protected]> > >> Cc: Wu, Jingjing <[email protected]>; Xing, Beilei > >> <[email protected]>; > >> [email protected]; [email protected] > >> Subject: Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition > >> > >> > >> On 24-Oct-23 12:24 PM, Zhang, Qi Z wrote: > >>>> -----Original Message----- > >>>> From: Radu Nicolau <[email protected]> > >>>> Sent: Tuesday, October 24, 2023 6:23 PM > >>>> To: Marchand, David <[email protected]> > >>>> Cc: Wu, Jingjing <[email protected]>; Xing, Beilei > >>>> <[email protected]>; [email protected]; [email protected] > >>>> Subject: Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition > >>>> > >>>> > >>>> On 24-Oct-23 10:49 AM, David Marchand wrote: > >>>>> On Tue, Oct 24, 2023 at 11:13 AM Radu Nicolau > >>>>> <[email protected]> > >>>> wrote: > >>>>>> IAVF_TX_OFFLOAD_MASK definition contained > >>>> RTE_ETH_TX_OFFLOAD_SECURITY > >>>>>> instead of RTE_MBUF_F_TX_SEC_OFFLOAD. > >>>>>> > >>>>>> Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto") > >>>>>> Cc: [email protected] > >>>>>> > >>>>>> Signed-off-by: Radu Nicolau <[email protected]> > >>>>> Something is not clear to me. > >>>>> How was the IPsec inline crypto feature supposed to work with this > >>>>> driver so far? > >>>>> > >>>>> Any packet with the RTE_MBUF_F_TX_SEC_OFFLOAD flag should have > >> been > >>>>> refused in iavf_prep_pkts. > >>>>> > >>>> It worked because the IPsec sample app doesn't call > >>>> rte_eth_tx_prepare, and from what I can see no other sample app does. > >>> To keep consistent, its better to refine the > >> IAVF_TX_OFFLOAD_NOTSUP_MASK definition. > >> > >> You mean like this? > >> > >> > >> #define IAVF_TX_OFFLOAD_NOTSUP_MASK ( \ > >> RTE_MBUF_F_TX_OFFLOAD_MASK ^ ( \ > >> RTE_MBUF_F_TX_OUTER_IPV6 | \ > >> RTE_MBUF_F_TX_OUTER_IPV4 | \ > >> RTE_MBUF_F_TX_IPV6 | \ > >> RTE_MBUF_F_TX_IPV4 | \ > >> RTE_MBUF_F_TX_VLAN | \ > >> RTE_MBUF_F_TX_IP_CKSUM | \ > >> RTE_MBUF_F_TX_L4_MASK | \ > >> RTE_MBUF_F_TX_TCP_SEG | \ > >> RTE_MBUF_F_TX_UDP_SEG | \ > >> RTE_MBUF_F_TX_TUNNEL_MASK | \ > >> RTE_MBUF_F_TX_OUTER_IP_CKSUM | \ > >> RTE_MBUF_F_TX_OUTER_UDP_CKSUM | \ > >> RTE_MBUF_F_TX_SEC_OFFLOAD)) > > Sorry, I miss understanding this code change, actually you didn't remove a > > flag, but just replace it, NOTSUP_MASK no need to be changed > > > > Then I don't understand why "Any packet with the RTE_MBUF_F_TX_SEC_OFFLOAD > > flag should have refused in iavf_prep_pkts" > > But I assume tx_pkt_prepare should reject only invalid packets while still > > functioning correctly with inline IPsec. > > rte_eth_tx_prepare would have rejected the packets before this fix, but > no app calls rte_eth_tx_prepare. The only app that calls it is testpmd.
>From my understanding, applications that want checksum offload are required to call rte_eth_tx_prepare. -- David Marchand
