DPDK patches and discussions
 help / color / mirror / Atom feed
From: Nipun Gupta <nipun.gupta@nxp.com>
To: Andrew Rybchenko <Andrew.Rybchenko@oktetlabs.ru>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
	"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>,
	"jerinjacobk@gmail.com" <jerinjacobk@gmail.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: Mon, 12 Oct 2020 12:53:54 +0000	[thread overview]
Message-ID: <AM5PR0401MB2593D81C68A96654AA331A6FE6070@AM5PR0401MB2593.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <f3baa74b-0c5d-6561-281f-61525d4db404@oktetlabs.ru>



> -----Original Message-----
> From: Andrew Rybchenko <Andrew.Rybchenko@oktetlabs.ru>
> Sent: Monday, October 12, 2020 5:53 PM
> To: Nipun Gupta <nipun.gupta@nxp.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; 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
> 
> On 10/12/20 2:30 PM, Nipun Gupta wrote:
> >
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <Andrew.Rybchenko@oktetlabs.ru>
> >> Sent: Monday, October 12, 2020 1:32 PM
> >> To: Nipun Gupta <nipun.gupta@nxp.com>; dev@dpdk.org
> >> Cc: thomas@monjalon.net; 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
> >>
> >> On 10/9/20 4:13 PM, nipun.gupta@nxp.com wrote:
> >>> 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>
> >>> ---
> >>>
> >>> v3:
> >>>  - Add additional rx_err_drop_offload_capa, which is specific
> >>>    capability flag for RX packets error drop offload. Currently
> >>>    only 'all' error packet drops are enabled, but can be extended
> >>>    to provide capability to drop any specific errors like L1 FCS,
> >>>    L3 Checksum etc.
> >>>  - Added separate config structure to enable the drop configuration.
> >>>  - Updated doc with the new updated option in testbbdev (patch 3/3)
> >>>
> >>> v2:
> >>>  - Add support in DPAA1 driver (patch 2/3)
> >>>  - Add support and config parameter in testpmd (patch 3/3)
> >>>
> >>>  lib/librte_ethdev/rte_ethdev.c |  1 +
> >>>  lib/librte_ethdev/rte_ethdev.h | 22 ++++++++++++++++++++++
> >>>  2 files changed, 23 insertions(+)
> >>>
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> >>> index 48d1333b1..be25e947e 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.c
> >>> +++ b/lib/librte_ethdev/rte_ethdev.c
> >>> @@ -128,6 +128,7 @@ static const struct {
> >>>  	RTE_RX_OFFLOAD_BIT2STR(SCTP_CKSUM),
> >>>  	RTE_RX_OFFLOAD_BIT2STR(OUTER_UDP_CKSUM),
> >>>  	RTE_RX_OFFLOAD_BIT2STR(RSS_HASH),
> >>> +	RTE_RX_OFFLOAD_BIT2STR(ERR_PKT_DROP),
> >>>  };
> >>>
> >>>  #undef RTE_RX_OFFLOAD_BIT2STR
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> >>> index d2bf74f12..cb968d38a 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>> @@ -1194,6 +1194,16 @@ struct rte_intr_conf {
> >>>  	uint32_t rmv:1;
> >>>  };
> >>>
> >>> +/**
> >>> + * A structure used to enable/disable error packet drop on RX.
> >>> + */
> >>> +struct rte_rx_err_pkt_drop_conf {
> >>> +	/** enable/disable all RX error packet drop.
> >>> +	 * 0 (default) - disable, 1 enable
> >>> +	 */
> >>> +	uint32_t all:1;
> >>
> >> "all" is bad. It should be granular and in the same terms
> >> as mask in dev_info capability.
> >
> > Consider that application do not want to receive any error packets as it will
> drop all
> > the error packets after increasing the error counter maintainer in the
> application.
> > If same functionality can be done by the hardware where hardware maintains
> the error
> > statistics, then why not have the same?
> > When hardware does parsing of packet headers and found error during that
> parsing,
> > this is to say to HW - hey drop any such error packets which you found during
> parsing
> > and just increase specific error counter (if present).
> >
> > I agree, granular shall also be added, but as our driver does not support them,
> so I did
> > not add those bits here.
> 
> There is always precise definition behind "all".
> You can report it in dev_info, an application can get it and
> use as is (all reported capability bits) in configuration.

Maybe Ill define individual error also (like l3_csum, l4_csum etc), and then 'all'
will make more sense. It will indicate that all the bits are on irrespective of
individual bits and can also be used in case if offload of individual error packet
drops type is not supported, but dropping all of them can be supported.

> 
> >
> >>
> >>> +};
> >>> +
> >>>  /**
> >>>   * A structure used to configure an Ethernet port.
> >>>   * Depending upon the RX multi-queue mode, extra advanced
> >>> @@ -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 not per queue?
> >
> > We do not support per queue configuration for error packet drop, but only
> > on ethernet basis.
> >
> >>
> >>>  };
> >>>
> >>>  /**
> >>> @@ -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
> >>>
> >>>  #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM |
> \
> >>>  				 DEV_RX_OFFLOAD_UDP_CKSUM | \
> >>> @@ -1274,6 +1287,13 @@ struct rte_eth_conf {
> >>>   * mentioned in rte_rx_offload_names in rte_ethdev.c file.
> >>>   */
> >>>
> >>> +/**
> >>> + * 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 strictly dislike "all". It will always be bad defined.
> >> It must be granular.
> >>
> >>>  /**
> >>>   * TX offload capabilities of a device.
> >>>   */
> >>> @@ -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. */
> >>>  	uint16_t reta_size;
> >>>  	/**< Device redirection table size, the total number of entries. */
> >>>  	uint8_t hash_key_size; /**< Hash key size in bytes */
> >>>
> >


  reply	other threads:[~2020-10-12 12:54 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
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 [this message]
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=AM5PR0401MB2593D81C68A96654AA331A6FE6070@AM5PR0401MB2593.eurprd04.prod.outlook.com \
    --to=nipun.gupta@nxp.com \
    --cc=Andrew.Rybchenko@oktetlabs.ru \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).