| Message ID | 20200831075333.10135-1-nipun.gupta@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ethdev: add rx offload to drop error packets | expand |
On 8/31/2020 8:53 AM, Nipun Gupta wrote: > This change adds a RX offload capability where hardware can drop the > packets in case there is an error in the packet such as L3 checksum > error or L4 checksum. > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com> > Signed-off-by: Rohit Raj <rohit.raj@nxp.com> > --- > lib/librte_ethdev/rte_ethdev.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h > index a49242bcd..bb9df2fb9 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -1120,6 +1120,7 @@ struct rte_eth_conf { > #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 > #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 > #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 > +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000 > > #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \ > DEV_RX_OFFLOAD_UDP_CKSUM | \ > Hi Nipun, You adding a new offload type for network devices, it should have some more description and relevant PMD/testpmd updates too. But before that, is dropping faulty packets an offload or a config option for the NIC? Or is this common for all NICs to have in ethdev?
Hi Ferruh, > -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@intel.com> > Sent: Monday, August 31, 2020 6:29 PM > To: Nipun Gupta <nipun.gupta@nxp.com>; dev@dpdk.org > Cc: thomas@monjalon.net; arybchenko@solarflare.com; Hemant Agrawal > <hemant.agrawal@nxp.com>; Rohit Raj <rohit.raj@nxp.com> > Subject: Re: [PATCH] ethdev: add rx offload to drop error packets > > On 8/31/2020 8:53 AM, Nipun Gupta wrote: > > This change adds a RX offload capability where hardware can drop the > > packets in case there is an error in the packet such as L3 checksum > > error or L4 checksum. > > > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com> > > Signed-off-by: Rohit Raj <rohit.raj@nxp.com> > > --- > > lib/librte_ethdev/rte_ethdev.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h > > index a49242bcd..bb9df2fb9 100644 > > --- a/lib/librte_ethdev/rte_ethdev.h > > +++ b/lib/librte_ethdev/rte_ethdev.h > > @@ -1120,6 +1120,7 @@ struct rte_eth_conf { > > #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 > > #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 > > #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 > > +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000 > > > > #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \ > > DEV_RX_OFFLOAD_UDP_CKSUM | \ > > > > > Hi Nipun, > > You adding a new offload type for network devices, it should have some more > description and relevant PMD/testpmd updates too. I should have added an RFC tag in the patch. I sent this for comment for error packet drop offload support. I will send patches after proper implementation. > > But before that, is dropping faulty packets an offload or a config option for > the NIC? Or is this common for all NICs to have in ethdev? We would like to have it as an offload capability, and on the basis of this capability user can configure the Ethernet device to drop the error packets on the device itself without providing them to the core. Sorry I did not get your comment "Or is this common for all NICs to have in ethdev?" Thanks, Nipun
On Mon, 31 Aug 2020 13:23:33 +0530 Nipun Gupta <nipun.gupta@nxp.com> wrote: > This change adds a RX offload capability where hardware can drop the > packets in case there is an error in the packet such as L3 checksum > error or L4 checksum. > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com> > Signed-off-by: Rohit Raj <rohit.raj@nxp.com> > --- > lib/librte_ethdev/rte_ethdev.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h > index a49242bcd..bb9df2fb9 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -1120,6 +1120,7 @@ struct rte_eth_conf { > #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 > #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 > #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 > +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000 > > #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \ > DEV_RX_OFFLOAD_UDP_CKSUM | \ Could/should this be an rte_flow action as well?
31/08/2020 19:00, Stephen Hemminger: > On Mon, 31 Aug 2020 13:23:33 +0530 > Nipun Gupta <nipun.gupta@nxp.com> wrote: > > > This change adds a RX offload capability where hardware can drop the > > packets in case there is an error in the packet such as L3 checksum > > error or L4 checksum. > > > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com> > > Signed-off-by: Rohit Raj <rohit.raj@nxp.com> > > --- > > +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000 Please add RTE_ prefix, even if older macros don't have it. We could (in a separate effort) alias old ones with RTE_ prefixed names. > Could/should this be an rte_flow action as well? I feel rte_flow API is not appropriate here. Ori, any opinion?
> -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, September 1, 2020 1:39 PM > To: Nipun Gupta <nipun.gupta@nxp.com>; Stephen Hemminger > <stephen@networkplumber.org>; orika@mellanox.com > Cc: dev@dpdk.org; ferruh.yigit@intel.com; arybchenko@solarflare.com; > Hemant Agrawal <hemant.agrawal@nxp.com>; Rohit Raj <rohit.raj@nxp.com>; > olivier.matz@6wind.com > Subject: Re: [dpdk-dev] [PATCH] ethdev: add rx offload to drop error packets > > 31/08/2020 19:00, Stephen Hemminger: > > On Mon, 31 Aug 2020 13:23:33 +0530 > > Nipun Gupta <nipun.gupta@nxp.com> wrote: > > > > > This change adds a RX offload capability where hardware can drop the > > > packets in case there is an error in the packet such as L3 checksum > > > error or L4 checksum. > > > > > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com> > > > Signed-off-by: Rohit Raj <rohit.raj@nxp.com> > > > --- > > > +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000 > > Please add RTE_ prefix, even if older macros don't have it. > We could (in a separate effort) alias old ones with RTE_ prefixed names. Agree, will update and send the change along with testpmd and dpaa2 driver change. > > > Could/should this be an rte_flow action as well? > > I feel rte_flow API is not appropriate here. > Ori, any opinion? >
Hi > -----Original Message----- > Subject: Re: [dpdk-dev] [PATCH] ethdev: add rx offload to drop error packets > > 31/08/2020 19:00, Stephen Hemminger: > > On Mon, 31 Aug 2020 13:23:33 +0530 > > Nipun Gupta <nipun.gupta@nxp.com> wrote: > > > > > This change adds a RX offload capability where hardware can drop the > > > packets in case there is an error in the packet such as L3 checksum > > > error or L4 checksum. > > > > > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com> > > > Signed-off-by: Rohit Raj <rohit.raj@nxp.com> > > > --- > > > +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000 > > Please add RTE_ prefix, even if older macros don't have it. > We could (in a separate effort) alias old ones with RTE_ prefixed names. > > > Could/should this be an rte_flow action as well? > > I feel rte_flow API is not appropriate here. > Ori, any opinion? > I also don't think this is relevant action in case of rte_flow, If we want to support such a thing in rte_flow we should add item that matches on error and then use the drop action. Best, Ori
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h index a49242bcd..bb9df2fb9 100644 --- a/lib/librte_ethdev/rte_ethdev.h +++ b/lib/librte_ethdev/rte_ethdev.h @@ -1120,6 +1120,7 @@ struct rte_eth_conf { #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \ DEV_RX_OFFLOAD_UDP_CKSUM | \