DPDK patches and discussions
 help / color / mirror / Atom feed
From: Nipun Gupta <nipun.gupta@nxp.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	"jerinjacobk@gmail.com" <jerinjacobk@gmail.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	"arybchenko@solarflare.com" <arybchenko@solarflare.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>,
	Sachin Saxena <sachin.saxena@nxp.com>,
	Rohit Raj <rohit.raj@nxp.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"asafp@nvidia.com" <asafp@nvidia.com>
Subject: Re: [dpdk-dev] [PATCH 1/3 v3] ethdev: add rx offload to drop error packets
Date: Tue, 13 Oct 2020 07:22:24 +0000
Message-ID: <AM5PR0401MB2593A2D3FF5A3810069DD7DFE6040@AM5PR0401MB2593.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <AM5PR0401MB25932FAB45042E171CC7E4B7E6070@AM5PR0401MB2593.eurprd04.prod.outlook.com>

Hi Thomas and Jerin,

There seems two views:
1. Use of existing rx_offload_capa, to have additional error packet drop offload capabilities. This is showcased in v2, and also suggested by Thomas.
2. Adding additional offload capability flag types and configuration structure for error packet drop configuration as suggested by Jerin, which is part of v3.

Please let me know which one shall I continue with for the next spin.

Thanks,
Nipun

> -----Original Message-----
> From: Nipun Gupta
> Sent: Monday, October 12, 2020 11:11 AM
> To: Thomas Monjalon <thomas@monjalon.net>; jerinjacobk@gmail.com
> Cc: dev@dpdk.org; ferruh.yigit@intel.com; arybchenko@solarflare.com;
> Hemant Agrawal <hemant.agrawal@nxp.com>; Sachin Saxena
> <sachin.saxena@nxp.com>; Rohit Raj <rohit.raj@nxp.com>;
> stephen@networkplumber.org; asafp@nvidia.com
> Subject: RE: [dpdk-dev] [PATCH 1/3 v3] ethdev: add rx offload to drop error
> packets
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Monday, October 12, 2020 3:12 AM
> > To: Nipun Gupta <nipun.gupta@nxp.com>
> > Cc: dev@dpdk.org; ferruh.yigit@intel.com; arybchenko@solarflare.com;
> > Hemant Agrawal <hemant.agrawal@nxp.com>; Sachin Saxena
> > <sachin.saxena@nxp.com>; Rohit Raj <rohit.raj@nxp.com>;
> > jerinjacobk@gmail.com; stephen@networkplumber.org; asafp@nvidia.com
> > Subject: Re: [dpdk-dev] [PATCH 1/3 v3] ethdev: add rx offload to drop error
> > packets
> >
> > Hi,
> >
> > The configuration of this feature is not clear to me.
> > Please see the comments below.
> >
> > 09/10/2020 15:13, nipun.gupta@nxp.com:
> > > From: Nipun Gupta <nipun.gupta@nxp.com>
> > >
> > > This change adds a RX offload capability and configuration to
> > > enable hardware to drop the packets in case of any error in the
> > > packets 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>
> > > Reviewed-by: Asaf Penso <asafp@nvidia.com>
> > > ---
> > [...]
> > > +/**
> > > + * A structure used to enable/disable error packet drop on RX.
> >
> > RX -> Rx
> > same for other occurences below
> 
> Okay.
> 
> >
> > > + */
> > > +struct rte_rx_err_pkt_drop_conf {
> >
> > The name should start with rte_eth_
> >
> > > +	/** enable/disable all RX error packet drop.
> > > +	 * 0 (default) - disable, 1 enable
> > > +	 */
> > > +	uint32_t all:1;
> > > +};
> >
> > I don't understand the meaning of this struct.
> > Is it just one bit to drop packets having an error?
> > How do you determine what is an error?
> 
> The error packets which are dropped on the hardware can be from
> various reasons, like L1 FCS, L3 CSUM, L4 CSUM or HW can be configured
> to drop ALL (or any) of the error packets received on the interface.
> 
> Currently DPAA drivers support drop of ALL the error packets, but does
> not support configuration of packet drops on basis of separate error types
> like dropping only L2 CSUM, L3 CSUM. So this patch is supporting only drop
> of 'ALL' the errors packets received on HW.
> 
> Marvell supports drop on separate error types, thus Jerin suggested to have
> this structure so that it opens the room for adding separate configuration
> to drop packets on basis of separate error types.
> 
> Jerin,
> maybe you can add more on this.
> 
> >
> > [...]
> > > @@ -1236,6 +1246,8 @@ struct rte_eth_conf {
> > >  	uint32_t dcb_capability_en;
> > >  	struct rte_fdir_conf fdir_conf; /**< FDIR configuration. DEPRECATED */
> > >  	struct rte_intr_conf intr_conf; /**< Interrupt mode configuration. */
> > > +	struct rte_rx_err_pkt_drop_conf err_pkt_drop_conf;
> > > +	/**< RX error packet drop configuration. */
> >
> > Why a per-port configuration is needed in addition of the per-queue offload?
> >
> > [...]
> > > @@ -1260,6 +1272,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
> >
> > New offload names should starte with RTE_ prefix.
> 
> Okay
> 
> >
> >
> > > +/**
> > > + * RX Error Drop offload config/capabilities of a device. These
> > > + * are valid only when RX capability DEV_RX_OFFLOAD_ERR_PKT_DROP
> > > + * is supported by the device.
> > > + */
> > > +#define DEV_RX_ERR_PKT_DROP_OFFLOAD_ALL		0x00000001
> >
> > I don't understand the meaning.
> 
> For DPAA drivers there is only one offload required, but once separate offloads
> are added to drop packets on basis of separate errors, then it can bloat up the
> offload capabilities. So we added new flags ' rte_rx_err_pkt_drop_conf ' instead
> of adding this onto 'rx_offload_capa'.
> 
> There is only one flag ' DEV_RX_OFFLOAD_ERR_PKT_DROP' added in
> 'rx_offload_capa'
> and if this flag is enabled, one or more of the flag in '
> DEV_RX_ERR_PKT_DROP_OFFLOAD...'
> can be enabled (currently only DEV_RX_ERR_PKT_DROP_OFFLOAD_ALL) being
> supported
> Others like:
> - DEV_RX_ERR_PKT_DROP_OFFLOAD_L1_FCS
> - DEV_RX_ERR_PKT_DROP_OFFLOAD_L3_CSUM
> - DEV_RX_ERR_PKT_DROP_OFFLOAD_L4_CSUM etc.
> Can be added once any reference driver is supporting this.
> 
> P.S. I can add these flags in this patch but without any ref driver support.
> 
> >
> > > @@ -1411,6 +1431,8 @@ struct rte_eth_dev_info {
> > >  	/**< Device per-queue RX offload capabilities. */
> > >  	uint64_t tx_queue_offload_capa;
> > >  	/**< Device per-queue TX offload capabilities. */
> > > +	uint64_t rx_err_drop_offload_capa;
> > > +	/**< RX error packet drop offload capabilities. */
> >
> > Why adding a new field here instead of reporting in rx_offload_capa?
> 
> This was part of v2 where flag was added in rx_offload_capa.
> 
> Both ways (v2 and v3 versions) seems to have pros and cons:
> - Adding only in the rx_offload_capa' seems simpler to implement and use
> - But it shall lead to increase in the offload capability types when packet
>   drop configuration is added on individual error types.
> 
> Regards,
> Nipun
> 
> >
> >


  reply	other threads:[~2020-10-13  7:22 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-31  7:53 [dpdk-dev] [PATCH] " Nipun Gupta
2020-08-31 12:58 ` Ferruh Yigit
2020-08-31 16:04   ` Nipun Gupta
2020-08-31 17:00 ` Stephen Hemminger
2020-09-01  8:09   ` Thomas Monjalon
2020-09-01 10:56     ` Nipun Gupta
2020-09-21  7:29     ` Ori Kam
2020-10-05  7:15 ` [dpdk-dev] [PATCH 1/3 v2] " nipun.gupta
2020-10-05  7:15   ` [dpdk-dev] [PATCH 2/3 v2] net/dpaa: support RX offload for error packet drop nipun.gupta
2020-10-05  7:15   ` [dpdk-dev] [PATCH 3/3 v2] testpmd: support hardware offload to drop error packets nipun.gupta
2020-10-08 15:06     ` Asaf Penso
2020-10-08 15:45       ` Nipun Gupta
2020-10-05 15:34   ` [dpdk-dev] [PATCH 1/3 v2] ethdev: add rx " Stephen Hemminger
2020-10-05 16:10     ` Jerin Jacob
2020-10-06 10:37       ` Nipun Gupta
2020-10-06 12:01         ` Jerin Jacob
2020-10-06 13:10           ` Nipun Gupta
2020-10-06 13:13             ` Jerin Jacob
2020-10-08  8:53               ` Nipun Gupta
2020-10-08  8:55                 ` Jerin Jacob
2020-10-08 15:13                   ` Asaf Penso
2020-10-09 13:13 ` [dpdk-dev] [PATCH 1/3 v3] " nipun.gupta
2020-10-09 13:13   ` [dpdk-dev] [PATCH 2/3 v3] net/dpaa: support RX offload for error packet drop nipun.gupta
2020-10-09 13:13   ` [dpdk-dev] [PATCH 3/3 v3] app/testpmd: support hardware offload to drop error packets nipun.gupta
2020-10-11  7:22     ` Asaf Penso
2020-10-11 10:13   ` [dpdk-dev] [PATCH 1/3 v3] ethdev: add rx " Jerin Jacob
2020-10-11 21:41   ` Thomas Monjalon
2020-10-12  5:40     ` Nipun Gupta
2020-10-13  7:22       ` Nipun Gupta [this message]
2020-10-12  8:01   ` Andrew Rybchenko
2020-10-12 11:30     ` Nipun Gupta
2020-10-12 12:22       ` Andrew Rybchenko
2020-10-12 12:53         ` Nipun Gupta
2020-10-13  7:21         ` Andrew Rybchenko
2020-10-13  7:36           ` Nipun Gupta
2020-10-13  7:51             ` Andrew Rybchenko
2020-10-13  8:12               ` Nipun Gupta
2020-10-15 13:23 ` [dpdk-dev] [PATCH 1/3 v4] ethdev: add Rx " nipun.gupta
2020-10-15 13:23   ` [dpdk-dev] [PATCH 2/3 v4] net/dpaa: support Rx offload for error packet drop nipun.gupta
2020-10-15 13:23   ` [dpdk-dev] [PATCH 3/3 v4] app/testpmd: support hardware offload to drop error packets nipun.gupta
2020-10-29 17:22     ` Dharmik Thakkar
2020-10-31 18:16       ` Nipun Gupta
2020-10-19  3:30   ` [dpdk-dev] [PATCH 1/3 v4] ethdev: add Rx " Ajit Khaparde
2021-02-18 20:32   ` Ferruh Yigit
2021-02-18 20:37     ` Thomas Monjalon
2021-04-20  1:11       ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM5PR0401MB2593A2D3FF5A3810069DD7DFE6040@AM5PR0401MB2593.eurprd04.prod.outlook.com \
    --to=nipun.gupta@nxp.com \
    --cc=arybchenko@solarflare.com \
    --cc=asafp@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinjacobk@gmail.com \
    --cc=rohit.raj@nxp.com \
    --cc=sachin.saxena@nxp.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git