DPDK patches and discussions
 help / color / mirror / Atom feed
From: Asaf Penso <asafp@nvidia.com>
To: Jerin Jacob <jerinjacobk@gmail.com>, Nipun Gupta <nipun.gupta@nxp.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	dpdk-dev <dev@dpdk.org>,
	NBU-Contact-Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>,
	Sachin Saxena <sachin.saxena@nxp.com>,
	Rohit Raj <rohit.raj@nxp.com>
Subject: Re: [dpdk-dev] [PATCH 1/3 v2] ethdev: add rx offload to drop error	packets
Date: Thu, 8 Oct 2020 15:13:05 +0000
Message-ID: <DM5PR12MB240683437B0CC4204F6C077ECD0B0@DM5PR12MB2406.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CALBAE1PyUwRh3J0n3NLGh8Y=7yrF+rBbM=20YJWDQbaeOKduaw@mail.gmail.com>

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Jerin Jacob
>Sent: Thursday, October 8, 2020 11:56 AM
>To: Nipun Gupta <nipun.gupta@nxp.com>
>Cc: Stephen Hemminger <stephen@networkplumber.org>; dpdk-dev
><dev@dpdk.org>; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
>Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
><arybchenko@solarflare.com>; Hemant Agrawal
><hemant.agrawal@nxp.com>; Sachin Saxena <sachin.saxena@nxp.com>;
>Rohit Raj <rohit.raj@nxp.com>
>Subject: Re: [dpdk-dev] [PATCH 1/3 v2] ethdev: add rx offload to drop error
>packets
>
>On Thu, Oct 8, 2020 at 2:23 PM Nipun Gupta <nipun.gupta@nxp.com> wrote:
>>
>>
>>
>> > -----Original Message-----
>> > From: Jerin Jacob <jerinjacobk@gmail.com>
>> > Sent: Tuesday, October 6, 2020 6:44 PM
>> > To: Nipun Gupta <nipun.gupta@nxp.com>
>> > Cc: Stephen Hemminger <stephen@networkplumber.org>; dpdk-dev
>> > <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>; Ferruh
>Yigit
>> > <ferruh.yigit@intel.com>; Andrew Rybchenko
>> > <arybchenko@solarflare.com>; Hemant Agrawal
>> > <hemant.agrawal@nxp.com>; Sachin Saxena <sachin.saxena@nxp.com>;
>> > Rohit Raj <rohit.raj@nxp.com>
>> > Subject: Re: [dpdk-dev] [PATCH 1/3 v2] ethdev: add rx offload to
>> > drop error packets
>> >
>> > On Tue, Oct 6, 2020 at 6:40 PM Nipun Gupta <nipun.gupta@nxp.com>
>wrote:
>> > >
>> > >
>> > >
>> > > > -----Original Message-----
>> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
>> > > > Sent: Tuesday, October 6, 2020 5:31 PM
>> > > > To: Nipun Gupta <nipun.gupta@nxp.com>
>> > > > Cc: Stephen Hemminger <stephen@networkplumber.org>; dpdk-dev
>> > > > <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>;
>Ferruh
>> > > > Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
>> > <arybchenko@solarflare.com>;
>> > > > Hemant Agrawal <hemant.agrawal@nxp.com>; Sachin Saxena
>> > > > <sachin.saxena@nxp.com>; Rohit Raj <rohit.raj@nxp.com>
>> > > > Subject: Re: [dpdk-dev] [PATCH 1/3 v2] ethdev: add rx offload to
>> > > > drop error packets
>> > > >
>> > > > On Tue, Oct 6, 2020 at 4:07 PM Nipun Gupta <nipun.gupta@nxp.com>
>wrote:
>> > > > >
>> > > > >
>> > > > >
>> > > > > > -----Original Message-----
>> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
>> > > > > > Sent: Monday, October 5, 2020 9:40 PM
>> > > > > > To: Stephen Hemminger <stephen@networkplumber.org>
>> > > > > > Cc: Nipun Gupta <nipun.gupta@nxp.com>; dpdk-dev
>> > > > > > <dev@dpdk.org>;
>> > > > Thomas
>> > > > > > Monjalon <thomas@monjalon.net>; Ferruh Yigit
>> > <ferruh.yigit@intel.com>;
>> > > > > > Andrew Rybchenko <arybchenko@solarflare.com>; Hemant
>Agrawal
>> > > > > > <hemant.agrawal@nxp.com>; Sachin Saxena
>> > > > > > <sachin.saxena@nxp.com>;
>> > > > Rohit
>> > > > > > Raj <rohit.raj@nxp.com>
>> > > > > > Subject: Re: [dpdk-dev] [PATCH 1/3 v2] ethdev: add rx
>> > > > > > offload to drop
>> > error
>> > > > > > packets
>> > > > > >
>> > > > > > On Mon, Oct 5, 2020 at 9:05 PM Stephen Hemminger
>> > > > > > <stephen@networkplumber.org> wrote:
>> > > > > > >
>> > > > > > > On Mon,  5 Oct 2020 12:45:04 +0530 nipun.gupta@nxp.com
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > From: Nipun Gupta <nipun.gupta@nxp.com>
>> > > > > > > >
>> > > > > > > > This change adds a RX offload capability, which once
>> > > > > > > > enabled, hardware will drop the packets in case there of
>> > > > > > > > any error in the packet such as L3 checksum error or L4
>checksum.
>> > > > > >
>> > > > > > IMO, Providing additional support up to the level to choose
>> > > > > > the errors to drops give more control to the application.
>> > > > > > Meaning,
>> > > > > > L1 errors such as FCS error
>> > > > > > L2 errors ..
>> > > > > > L3 errors such checksum
>> > > > > > i.e ethdev spec need to have  error level supported by PMD
>> > > > > > and the application can set the layers interested to drop.
>> > > > >
>> > > > > Agree, but 'DEV_RX_OFFLOAD_ERR_PKT_DROP' shall also be there
>> > > > > to drop
>> > all
>> > > > the
>> > > > > error packets? Maybe we can rename it to
>> > > > DEV_RX_OFFLOAD_ALL_ERR_PKT_DROP.
>> > > >
>> > > > IMHO,  we introduce such shortcut for a single flag for all err
>> > > > drop then we can not change the scheme without an API/ABI break.
>> > >
>> > > Are the following offloads fine:
>> > >         DEV_RX_OFFLOAD_L1_FCS_ERR_PKT_DROP
>> > >         DEV_RX_OFFLOAD_L3_CSUM_ERR_PKT_DROP
>> > >         DEV_RX_OFFLOAD_L4_CSUM_ERR_PKT_DROP
>> > >         DEV_RX_OFFLOAD_ALL_ERR_PKT_DROP
>> > >
>> > > Please let me know in case I need to add any other too.
>> >
>> > I think, single offload flags and some config/capability structure
>> > to define the additional layer selection would be good, instead of
>> > adding a lot of new offload flags.
>>
>>
>> +/**
>> + * 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;
>> +};
>> +
>>  /**
>>   * 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. */
>>
>> Is this the kind of changes you are talking about?
>
>
>Yes.
>
>>
>> Also, more changes will be there in 'struct rte_eth_dev_info'
>> structure, defining additional separate capability something like 'uint64_t
>rx_err_drop_offload_capa'.
>>
>> Regards,
>> Nipun
>>
>> >
>> >
>> > > Ill send a v3.
>> > >
>> > > Thanks,
>> > > Nipun
>> > >
>> > > >
>> > > > >
>> > > > > Currently we have not planned to add separate knobs for
>> > > > > separate error in the driver, maybe we can define them
>> > > > > separately, or we need have them in this series itself?
>> > > >
>> > > > I think, ethdev API can have the capability on what are levels
>> > > > it supported, in your driver case, you can express the same.
>> > > >
>> > > >
>> > > > >
>> > > > > >
>> > > > > > > >
>> > > > > > > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
>> > > > > > > > Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
>> > > > > > > > ---
>> > > > > > > > These patches are based over series:
>> > > > > > > >
>> > > > > >
>> > > >
>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
>> > tchwo
>> > > > > >
>> > > >
>> >
>rk.dpdk.org%2Fpatch%2F78630%2F&amp;data=02%7C01%7Cnipun.gupta%40
>nx
>> > > > > >
>> > > >
>> >
>p.com%7C90b516fd465c48945e7008d869492b3e%7C686ea1d3bc2b4c6fa92cd9
>> > > > > >
>> > > >
>> >
>9c5c301635%7C0%7C0%7C637375110263097933&amp;sdata=RBQswMBsfpM6
>> > > > > >
>nyKur%2FaHvOMvNK7RU%2BRyhHt%2FXBsP1OM%3D&amp;reserved=0
>> > > > > > > >
>> > > > > > > > Changes in v2:
>> > > > > > > >  - Add support in DPAA1 driver (patch 2/3)
>> > > > > > > >  - Add support and config parameter in testpmd (patch
>> > > > > > > > 3/3)
>> > > > > > > >
>> > > > > > > >  lib/librte_ethdev/rte_ethdev.h | 1 +
>> > > > > > > >  1 file changed, 1 insertion(+)
>> > > > > > >
>> > > > > > > Maybe this should be an rte_flow match/action which would
>> > > > > > > then make
>> > it
>> > > > > > > more flexible?
>> > > > > >
>> > > > > > I think, it is not based on any Patten matching. So IMO, it
>> > > > > > should be best
>> > if it
>> > > > > > is part of RX offload.
>> > > > > >
>> > > > > > >
>> > > > > > > There is not much of a performance gain for this in real
>> > > > > > > life and if only one driver supports it then I am not convinced this
>is needed.
>> > > > > >
>> > > > > > Marvell HW has this feature.
Reviewed-By: Asaf Penso <asafp@nvidia.com>

  reply	other threads:[~2020-10-08 15:13 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 [this message]
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
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=DM5PR12MB240683437B0CC4204F6C077ECD0B0@DM5PR12MB2406.namprd12.prod.outlook.com \
    --to=asafp@nvidia.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinjacobk@gmail.com \
    --cc=nipun.gupta@nxp.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