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 40D3AA0487 for ; Mon, 1 Jul 2019 12:07:48 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 676D12C38; Mon, 1 Jul 2019 12:07:47 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 4D7082C23 for ; Mon, 1 Jul 2019 12:07:46 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 743BC48005F; Mon, 1 Jul 2019 10:07:44 +0000 (UTC) Received: from [192.168.1.11] (85.187.13.180) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 1 Jul 2019 11:07:30 +0100 To: Ferruh Yigit , , CC: , , , , Jerin Jacob Kollanukkaran References: <1555486846-20764-1-git-send-email-viveksharma@marvell.com> <1555653564-27263-1-git-send-email-viveksharma@marvell.com> From: Andrew Rybchenko Message-ID: <6d469b41-7261-34d2-3c20-9aa32b0042ce@solarflare.com> Date: Mon, 1 Jul 2019 13:07:05 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.180] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24732.003 X-TM-AS-Result: No-9.618500-8.000000-10 X-TMASE-MatchedRID: xcONGPdDH5rmLzc6AOD8DfHkpkyUphL9D+jls0cSwJMIoD7Gyec7oy1S BwRaWLSlp+uiLk4HyZHwLCakS3oFFYxoNLEIeCiOh2VzUlo4HVOiIpNv3rjMdWfbjGiFA7TlFPu ahfa4tRU3h/JWvQVu0V7LYHGSSpoCeRNW9TTmWAI2oY2TxKHZF1aKKVQb5vbN4mJSqMd68jRiP4 TkprBAH/oSRTSXUKtzUKVvtpGOi8WUK/Rru2u6IjKVTrGMDe/DAZn/4A9db2Sjm/4813nI8qgoO ync5E+eULqNWfM8FP+J+bkgix96SifUTFI6hNdWEhGH3CRdKUVJaD67iKvY06Rea8wUAdQ60gIK Oa68XpazoOI+z7HtsMBBukfouTGCN2GXzj6bC8KInASnzB5VfPiH64jt3FfEWgyC8H9R1eGvr2G Qc90liuLzNWBegCW2RYvisGWbbS+3sNbcHjySQd0H8LFZNFG7/nnwJ52QYi/M/gWu/p797g5f/r Lp7SkGkrnz18YUTzBbiFasqdOKVmP6ejblYfN6ftwZ3X11IV0= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--9.618500-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24732.003 X-MDID: 1561975665-m-ubbw6U0Ekl Subject: Re: [dpdk-dev] [PATCH v3] ethdev: support QinQ strip dynamic configuration 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 27.06.2019 14:08, Ferruh Yigit wrote: > On 4/19/2019 6:59 AM, viveksharma@marvell.com wrote: >> From: Vivek Sharma >> >> Enable missing support for runtime configuration (setting/getting) >> of QinQ strip rx offload for a given ethdev. >> >> Signed-off-by: Vivek Sharma > <...> > >> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h >> index 40a068f..c1792f4 100644 >> --- a/lib/librte_ethdev/rte_ethdev.h >> +++ b/lib/librte_ethdev/rte_ethdev.h >> @@ -550,11 +550,13 @@ struct rte_eth_rss_conf { >> #define ETH_VLAN_STRIP_OFFLOAD 0x0001 /**< VLAN Strip On/Off */ >> #define ETH_VLAN_FILTER_OFFLOAD 0x0002 /**< VLAN Filter On/Off */ >> #define ETH_VLAN_EXTEND_OFFLOAD 0x0004 /**< VLAN Extend On/Off */ >> +#define ETH_QINQ_STRIP_OFFLOAD 0x0008 /**< QINQ Strip On/Off */ >> >> /* Definitions used for mask VLAN setting */ >> #define ETH_VLAN_STRIP_MASK 0x0001 /**< VLAN Strip setting mask */ >> #define ETH_VLAN_FILTER_MASK 0x0002 /**< VLAN Filter setting mask*/ >> #define ETH_VLAN_EXTEND_MASK 0x0004 /**< VLAN Extend setting mask*/ >> +#define ETH_QINQ_STRIP_MASK 0x0008 /**< QINQ Strip setting mask */ >> #define ETH_VLAN_ID_MAX 0x0FFF /**< VLAN ID is in lower 12 bits*/ > On its own patch looks ok but a few high level questions: > > 1- Why we need this interim defines, instead of using actual offload values? > Although changing this will be API/ABI breakage. > > 2- Why we have specific function to set vlan offloads, for other offloads the > way is stop device and reconfigure it with new offload flags. > > 3- If devices can change offload configuration dynamically, do we need a generic > API to alter the offload configs? (similar to these vlan APIs but more generic)? If dynamic switching of offloads is really required, I think it would be good if it is unified. If so, we can deprecate rte_eth_dev_{s,g}et_vlan_offload() and implement it using the new one to remove the old one from driver interface at least. It requires reporting of dynamically switchable offloads on device and queue level. Or just dynamically switchable offloads in assumption that if the offload is supported on queue level and dynamically switchable, it is dynamically switchable on queue level. > Related to the patch, what do you think about following options: > a) > - Get this patch > - Send a deprecation notice for 1) in this release > - Next release remove these flags, which will be practically reverse this patch > and more > > b) > - Send a deprecation notice for 1) in this release > - Next release update the APIs and a smaller/different version of this patch > will be required. I'm OK with both options, but I think it would be fair to go (a) to avoid postponing of the feature too long. The patch itself should update rte_eth_dev_{s,g}et_vlan_offload() descriptions which enumerate switchable offloads. Andrew.