* [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition @ 2023-10-24 9:13 Radu Nicolau 2023-10-24 9:49 ` David Marchand 2023-10-25 1:50 ` Zhang, Qi Z 0 siblings, 2 replies; 10+ messages in thread From: Radu Nicolau @ 2023-10-24 9:13 UTC (permalink / raw) To: Jingjing Wu, Beilei Xing; +Cc: dev, Radu Nicolau, stable 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: stable@dpdk.org Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> --- drivers/net/iavf/iavf_rxtx.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/iavf/iavf_rxtx.h b/drivers/net/iavf/iavf_rxtx.h index 605ea3f824..f29c90fb21 100644 --- a/drivers/net/iavf/iavf_rxtx.h +++ b/drivers/net/iavf/iavf_rxtx.h @@ -98,7 +98,7 @@ RTE_MBUF_F_TX_TUNNEL_MASK | \ RTE_MBUF_F_TX_OUTER_IP_CKSUM | \ RTE_MBUF_F_TX_OUTER_UDP_CKSUM | \ - RTE_ETH_TX_OFFLOAD_SECURITY) + RTE_MBUF_F_TX_SEC_OFFLOAD) #define IAVF_TX_OFFLOAD_NOTSUP_MASK \ (RTE_MBUF_F_TX_OFFLOAD_MASK ^ IAVF_TX_OFFLOAD_MASK) -- 2.25.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-24 9:13 [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition Radu Nicolau @ 2023-10-24 9:49 ` David Marchand 2023-10-24 10:22 ` Radu Nicolau 2023-10-25 1:50 ` Zhang, Qi Z 1 sibling, 1 reply; 10+ messages in thread From: David Marchand @ 2023-10-24 9:49 UTC (permalink / raw) To: Radu Nicolau; +Cc: Jingjing Wu, Beilei Xing, dev, stable On Tue, Oct 24, 2023 at 11:13 AM Radu Nicolau <radu.nicolau@intel.com> 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: stable@dpdk.org > > Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> 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. -- David Marchand ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-24 9:49 ` David Marchand @ 2023-10-24 10:22 ` Radu Nicolau 2023-10-24 11:24 ` Zhang, Qi Z 0 siblings, 1 reply; 10+ messages in thread From: Radu Nicolau @ 2023-10-24 10:22 UTC (permalink / raw) To: David Marchand; +Cc: Jingjing Wu, Beilei Xing, dev, stable On 24-Oct-23 10:49 AM, David Marchand wrote: > On Tue, Oct 24, 2023 at 11:13 AM Radu Nicolau <radu.nicolau@intel.com> 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: stable@dpdk.org >> >> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-24 10:22 ` Radu Nicolau @ 2023-10-24 11:24 ` Zhang, Qi Z 2023-10-24 14:48 ` Radu Nicolau 0 siblings, 1 reply; 10+ messages in thread From: Zhang, Qi Z @ 2023-10-24 11:24 UTC (permalink / raw) To: Nicolau, Radu, Marchand, David; +Cc: Wu, Jingjing, Xing, Beilei, dev, stable > -----Original Message----- > From: Radu Nicolau <radu.nicolau@intel.com> > Sent: Tuesday, October 24, 2023 6:23 PM > To: Marchand, David <david.marchand@redhat.com> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; > dev@dpdk.org; stable@dpdk.org > 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 <radu.nicolau@intel.com> > 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: stable@dpdk.org > >> > >> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> > > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-24 11:24 ` Zhang, Qi Z @ 2023-10-24 14:48 ` Radu Nicolau 2023-10-24 23:30 ` Zhang, Qi Z 0 siblings, 1 reply; 10+ messages in thread From: Radu Nicolau @ 2023-10-24 14:48 UTC (permalink / raw) To: Zhang, Qi Z, Marchand, David; +Cc: Wu, Jingjing, Xing, Beilei, dev, stable On 24-Oct-23 12:24 PM, Zhang, Qi Z wrote: > >> -----Original Message----- >> From: Radu Nicolau <radu.nicolau@intel.com> >> Sent: Tuesday, October 24, 2023 6:23 PM >> To: Marchand, David <david.marchand@redhat.com> >> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; >> dev@dpdk.org; stable@dpdk.org >> 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 <radu.nicolau@intel.com> >> 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: stable@dpdk.org >>>> >>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> >>> 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)) ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-24 14:48 ` Radu Nicolau @ 2023-10-24 23:30 ` Zhang, Qi Z 2023-10-25 9:01 ` Radu Nicolau 0 siblings, 1 reply; 10+ messages in thread From: Zhang, Qi Z @ 2023-10-24 23:30 UTC (permalink / raw) To: Nicolau, Radu, Marchand, David; +Cc: Wu, Jingjing, Xing, Beilei, dev, stable > -----Original Message----- > From: Nicolau, Radu <radu.nicolau@intel.com> > Sent: Tuesday, October 24, 2023 10:49 PM > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Marchand, David > <david.marchand@redhat.com> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; > dev@dpdk.org; stable@dpdk.org > 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 <radu.nicolau@intel.com> > >> Sent: Tuesday, October 24, 2023 6:23 PM > >> To: Marchand, David <david.marchand@redhat.com> > >> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei > >> <beilei.xing@intel.com>; dev@dpdk.org; stable@dpdk.org > >> 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 > >>> <radu.nicolau@intel.com> > >> 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: stable@dpdk.org > >>>> > >>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> > >>> 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-24 23:30 ` Zhang, Qi Z @ 2023-10-25 9:01 ` Radu Nicolau 2023-10-25 9:07 ` David Marchand 0 siblings, 1 reply; 10+ messages in thread From: Radu Nicolau @ 2023-10-25 9:01 UTC (permalink / raw) To: Zhang, Qi Z, Marchand, David; +Cc: Wu, Jingjing, Xing, Beilei, dev, stable On 25-Oct-23 12:30 AM, Zhang, Qi Z wrote: > >> -----Original Message----- >> From: Nicolau, Radu <radu.nicolau@intel.com> >> Sent: Tuesday, October 24, 2023 10:49 PM >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Marchand, David >> <david.marchand@redhat.com> >> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; >> dev@dpdk.org; stable@dpdk.org >> 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 <radu.nicolau@intel.com> >>>> Sent: Tuesday, October 24, 2023 6:23 PM >>>> To: Marchand, David <david.marchand@redhat.com> >>>> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei >>>> <beilei.xing@intel.com>; dev@dpdk.org; stable@dpdk.org >>>> 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 >>>>> <radu.nicolau@intel.com> >>>> 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: stable@dpdk.org >>>>>> >>>>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> >>>>> 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-25 9:01 ` Radu Nicolau @ 2023-10-25 9:07 ` David Marchand 2023-10-25 10:14 ` Radu Nicolau 0 siblings, 1 reply; 10+ messages in thread From: David Marchand @ 2023-10-25 9:07 UTC (permalink / raw) To: Radu Nicolau; +Cc: Zhang, Qi Z, Wu, Jingjing, Xing, Beilei, dev, stable On Wed, Oct 25, 2023 at 11:02 AM Radu Nicolau <radu.nicolau@intel.com> wrote: > > > On 25-Oct-23 12:30 AM, Zhang, Qi Z wrote: > > > >> -----Original Message----- > >> From: Nicolau, Radu <radu.nicolau@intel.com> > >> Sent: Tuesday, October 24, 2023 10:49 PM > >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Marchand, David > >> <david.marchand@redhat.com> > >> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; > >> dev@dpdk.org; stable@dpdk.org > >> 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 <radu.nicolau@intel.com> > >>>> Sent: Tuesday, October 24, 2023 6:23 PM > >>>> To: Marchand, David <david.marchand@redhat.com> > >>>> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei > >>>> <beilei.xing@intel.com>; dev@dpdk.org; stable@dpdk.org > >>>> 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 > >>>>> <radu.nicolau@intel.com> > >>>> 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: stable@dpdk.org > >>>>>> > >>>>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> > >>>>> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-25 9:07 ` David Marchand @ 2023-10-25 10:14 ` Radu Nicolau 0 siblings, 0 replies; 10+ messages in thread From: Radu Nicolau @ 2023-10-25 10:14 UTC (permalink / raw) To: David Marchand; +Cc: Zhang, Qi Z, Wu, Jingjing, Xing, Beilei, dev, stable On 25-Oct-23 10:07 AM, David Marchand wrote: > On Wed, Oct 25, 2023 at 11:02 AM Radu Nicolau <radu.nicolau@intel.com> wrote: >> >> On 25-Oct-23 12:30 AM, Zhang, Qi Z wrote: >>>> -----Original Message----- >>>> From: Nicolau, Radu <radu.nicolau@intel.com> >>>> Sent: Tuesday, October 24, 2023 10:49 PM >>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Marchand, David >>>> <david.marchand@redhat.com> >>>> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; >>>> dev@dpdk.org; stable@dpdk.org >>>> 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 <radu.nicolau@intel.com> >>>>>> Sent: Tuesday, October 24, 2023 6:23 PM >>>>>> To: Marchand, David <david.marchand@redhat.com> >>>>>> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei >>>>>> <beilei.xing@intel.com>; dev@dpdk.org; stable@dpdk.org >>>>>> 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 >>>>>>> <radu.nicolau@intel.com> >>>>>> 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: stable@dpdk.org >>>>>>>> >>>>>>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> >>>>>>> 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. TBH I don't understand much about it and looking at the implementation actually made things worse: for example from what I can see calling it when RTE_MBUF_F_TX_TCP_CKSUM is set will result in having the TCP checksum being computed (in software) in the prepare function. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition 2023-10-24 9:13 [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition Radu Nicolau 2023-10-24 9:49 ` David Marchand @ 2023-10-25 1:50 ` Zhang, Qi Z 1 sibling, 0 replies; 10+ messages in thread From: Zhang, Qi Z @ 2023-10-25 1:50 UTC (permalink / raw) To: Nicolau, Radu, Wu, Jingjing, Xing, Beilei; +Cc: dev, Nicolau, Radu, stable > -----Original Message----- > From: Radu Nicolau <radu.nicolau@intel.com> > Sent: Tuesday, October 24, 2023 5:13 PM > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com> > Cc: dev@dpdk.org; Nicolau, Radu <radu.nicolau@intel.com>; > stable@dpdk.org > Subject: [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition > > 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: stable@dpdk.org > > Signed-off-by: Radu Nicolau <radu.nicolau@intel.com> > --- > drivers/net/iavf/iavf_rxtx.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/iavf/iavf_rxtx.h b/drivers/net/iavf/iavf_rxtx.h index > 605ea3f824..f29c90fb21 100644 > --- a/drivers/net/iavf/iavf_rxtx.h > +++ b/drivers/net/iavf/iavf_rxtx.h > @@ -98,7 +98,7 @@ > RTE_MBUF_F_TX_TUNNEL_MASK | \ > RTE_MBUF_F_TX_OUTER_IP_CKSUM | \ > RTE_MBUF_F_TX_OUTER_UDP_CKSUM | \ > - RTE_ETH_TX_OFFLOAD_SECURITY) > + RTE_MBUF_F_TX_SEC_OFFLOAD) > > #define IAVF_TX_OFFLOAD_NOTSUP_MASK \ > (RTE_MBUF_F_TX_OFFLOAD_MASK ^ > IAVF_TX_OFFLOAD_MASK) > -- > 2.25.1 Acked-by: Qi Zhang <qi.z.zhang@intel.com> Applied to dpdk-next-net-intel. Thanks Qi ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-25 10:15 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-10-24 9:13 [PATCH] net/iavf: fix IAVF_TX_OFFLOAD_MASK definition Radu Nicolau 2023-10-24 9:49 ` David Marchand 2023-10-24 10:22 ` Radu Nicolau 2023-10-24 11:24 ` Zhang, Qi Z 2023-10-24 14:48 ` Radu Nicolau 2023-10-24 23:30 ` Zhang, Qi Z 2023-10-25 9:01 ` Radu Nicolau 2023-10-25 9:07 ` David Marchand 2023-10-25 10:14 ` Radu Nicolau 2023-10-25 1:50 ` Zhang, Qi Z
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).