From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 13FD5A04BC; Thu, 8 Oct 2020 10:56:04 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E35A21BAE4; Thu, 8 Oct 2020 10:56:02 +0200 (CEST) Received: from mail-il1-f193.google.com (mail-il1-f193.google.com [209.85.166.193]) by dpdk.org (Postfix) with ESMTP id 046EA1BACB for ; Thu, 8 Oct 2020 10:56:00 +0200 (CEST) Received: by mail-il1-f193.google.com with SMTP id o9so457903ilo.0 for ; Thu, 08 Oct 2020 01:55:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=5hB3Enz0UsMfNkB/TYaLcIXgNsO8r+G1TK6ElOzjq90=; b=IV+MgjdAE6vNMOGeLQZb1EcCK7XVY2wPWVaLBjfRuVEFb2duitG6PL+uwjj6iOYEag eXLwRttv6EQIFhptciDmuKGkINdJhfnss5XylzDxRkfYiGEitAujp0B0vI0U3k7jIhn4 8hzZmGvnpk6SuA1mJTkcJV9gArJuV3DMoKN4vpeehwY4xHeK6SLCFBFKBH0GODt52LOv Ls8rWfND5HEJJTRnttiUdUaDHPl3Vtw6XfzuuvqgOpsUAsLFHWgJKRusNsFa+6n1riEB kN1zOtYb+vT80TdImIzjv2zpZoF5VsX229yKUS+0XZUiT+3KUD9IbhhmzISZreYkv3DC n+HQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5hB3Enz0UsMfNkB/TYaLcIXgNsO8r+G1TK6ElOzjq90=; b=jM6nL9wrgprJmi+ZXK2MufY0XnmCNrgX/sO//CCb+RWEp+ZYSvXaZAXRsiDdWVGdEq f6eyvNgvMq6MdF3zsPS+hxtv7A8MOc5LnhCFd+Ttu8CzbnKqsPk/TdJQV4JyIWmyRw3a llzMeylMk+z072r5GgEJZQSSc3gIGenbQsc/kklREgbhsULcmkaJoB2ozcL7f7+SZ8SO lap9Sx19rZ09IDkrrti7Vz+FmnVXd3T896bP6azoWr/kvK9OL2L4t11JkKD2Z7+6EYKU sKMk/z+0IwmyHfePZbu3lVrpJ6kM9BtASNbKJESy7qNoeRd//Vk5CMDPypFDCzRzAlXT mdJQ== X-Gm-Message-State: AOAM531JQFmVFQUnv5vWnAUwjlayyiJfjAIo0de9ND7AeU9crVQ3bnr8 kLvjVFzKrJPKZTYZl1OZ3u1t8WGFdA2pjHeCHl0= X-Google-Smtp-Source: ABdhPJwTWMerSdPS+L2symkfAm2BzvQsAJO5/mWHriRw+DtBPytaxlrGMUMvh2gqiOxIsUduFITTGM2xLr+uHmnjjgI= X-Received: by 2002:a92:770e:: with SMTP id s14mr6100925ilc.130.1602147358421; Thu, 08 Oct 2020 01:55:58 -0700 (PDT) MIME-Version: 1.0 References: <20200831075333.10135-1-nipun.gupta@nxp.com> <20201005071506.28861-1-nipun.gupta@nxp.com> <20201005083455.2fdf57f2@hermes.local> In-Reply-To: From: Jerin Jacob Date: Thu, 8 Oct 2020 14:25:42 +0530 Message-ID: To: Nipun Gupta Cc: Stephen Hemminger , dpdk-dev , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Hemant Agrawal , Sachin Saxena , Rohit Raj Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH 1/3 v2] ethdev: add rx offload to drop error packets X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Oct 8, 2020 at 2:23 PM Nipun Gupta wrote: > > > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Tuesday, October 6, 2020 6:44 PM > > To: Nipun Gupta > > Cc: Stephen Hemminger ; dpdk-dev > > ; Thomas Monjalon ; Ferruh Yigit > > ; Andrew Rybchenko ; > > Hemant Agrawal ; Sachin Saxena > > ; Rohit Raj > > 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 wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Jerin Jacob > > > > Sent: Tuesday, October 6, 2020 5:31 PM > > > > To: Nipun Gupta > > > > Cc: Stephen Hemminger ; dpdk-dev > > > > ; Thomas Monjalon ; Ferruh Yigit > > > > ; Andrew Rybchenko > > ; > > > > Hemant Agrawal ; Sachin Saxena > > > > ; Rohit Raj > > > > 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 wrote: > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Jerin Jacob > > > > > > Sent: Monday, October 5, 2020 9:40 PM > > > > > > To: Stephen Hemminger > > > > > > Cc: Nipun Gupta ; dpdk-dev ; > > > > Thomas > > > > > > Monjalon ; Ferruh Yigit > > ; > > > > > > Andrew Rybchenko ; Hemant Agrawal > > > > > > ; Sachin Saxena ; > > > > Rohit > > > > > > Raj > > > > > > 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 > > > > > > wrote: > > > > > > > > > > > > > > On Mon, 5 Oct 2020 12:45:04 +0530 > > > > > > > nipun.gupta@nxp.com wrote: > > > > > > > > > > > > > > > From: Nipun Gupta > > > > > > > > > > > > > > > > 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 > > > > > > > > Signed-off-by: Rohit Raj > > > > > > > > --- > > > > > > > > These patches are based over series: > > > > > > > > > > > > > > > > > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwo > > > > > > > > > > > > rk.dpdk.org%2Fpatch%2F78630%2F&data=02%7C01%7Cnipun.gupta%40nx > > > > > > > > > > > > p.com%7C90b516fd465c48945e7008d869492b3e%7C686ea1d3bc2b4c6fa92cd9 > > > > > > > > > > > > 9c5c301635%7C0%7C0%7C637375110263097933&sdata=RBQswMBsfpM6 > > > > > > nyKur%2FaHvOMvNK7RU%2BRyhHt%2FXBsP1OM%3D&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.