From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A045FA054A; Thu, 18 Feb 2021 21:33:04 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 13A3C40040; Thu, 18 Feb 2021 21:33:04 +0100 (CET) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 9A0DB4003D for ; Thu, 18 Feb 2021 21:33:02 +0100 (CET) IronPort-SDR: gO6mmBGyX04nn7xySxrq1dLIJ+ultv2klABvr0KHbyOnrr+cFJ6aQAQ5d01Ta6/dBdE7J2EgIs B7qPYRkj69dA== X-IronPort-AV: E=McAfee;i="6000,8403,9899"; a="183707380" X-IronPort-AV: E=Sophos;i="5.81,187,1610438400"; d="scan'208";a="183707380" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Feb 2021 12:33:01 -0800 IronPort-SDR: TWv+NXO1xCYxb/M9e7Om+8TLwQSRT5ygnpcgxqtLoSO4vBaqUTrtc37OYYGn79v9pE5fTtXPnd aKhlVy/FFs1Q== X-IronPort-AV: E=Sophos;i="5.81,187,1610438400"; d="scan'208";a="386154322" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.252.26.139]) ([10.252.26.139]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Feb 2021 12:32:58 -0800 To: nipun.gupta@nxp.com, dev@dpdk.org Cc: thomas@monjalon.net, arybchenko@solarflare.com, hemant.agrawal@nxp.com, sachin.saxena@nxp.com, rohit.raj@nxp.com, jerinjacobk@gmail.com, stephen@networkplumber.org, asafp@nvidia.com References: <20200831075333.10135-1-nipun.gupta@nxp.com> <20201015132343.4050-1-nipun.gupta@nxp.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <4c3aeb4a-262e-6959-25cc-320bfa490774@intel.com> Date: Thu, 18 Feb 2021 20:32:54 +0000 MIME-Version: 1.0 In-Reply-To: <20201015132343.4050-1-nipun.gupta@nxp.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 1/3 v4] ethdev: add Rx offload to drop error packets X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 10/15/2020 2:23 PM, nipun.gupta@nxp.com wrote: > From: Nipun Gupta > > 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 > Signed-off-by: Rohit Raj > Reviewed-by: Asaf Penso > --- > > v4: > - renamed 'rte_rx_err_pkt_drop_conf' to > 'rte_eth_rx_err_pkt_drop_conf' > - updated function 'port_offload_cap_display' to display newly > added offloads > - added placeholder for L1 FCS, L3 Checksum, L4 Checksum error > packet drops > - updated doc/guides/nics/features.rst > - updated new added 'DEV_RX_OFFLOAD_*' to 'RTE_DEV_RX_OFFLOAD*' > - updated RX to Rx > > 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 | 39 +++++++++++++++++++++++++++++++++- > 2 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c This feature touches many main parts, - new config item for 'rte_eth_dev_configure()' - a new offload flag - new capability reporting for 'rte_eth_dev_info_get()' The feature doesn't look very mainstream to touch all these main parts and add complexity to them, which will affect almost all users. And has some inconsistencies, like configuration is done via config struct, but capability is returned as bit-wise. Or I think config option taken into account only if offload is requested has a chance to confuse people in both app and driver end. What do you think having two specific APIs to get_capabilities and set drop config? The responsibility of those APIs will be clear and narrowed down, which makes it harder to make it wrong. Also it is an ABI break as it is and needs to wait 21.11, and even after that it has a potential to cause more ABI breaks, like trying to add a new error type to drop will need to wait 22.11 ..., but if we can have them as separate APIs we can have them as experimental without waiting next LTS.