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 E07E7A051A; Sat, 20 Jun 2020 08:42:57 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A7EF91BF95; Sat, 20 Jun 2020 08:42:56 +0200 (CEST) Received: from huawei.com (szxga04-in.huawei.com [45.249.212.190]) by dpdk.org (Postfix) with ESMTP id 269691BF93 for ; Sat, 20 Jun 2020 08:42:53 +0200 (CEST) Received: from DGGEMS404-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 6BAD29B238CA318E9D87; Sat, 20 Jun 2020 14:42:52 +0800 (CST) Received: from [10.69.31.206] (10.69.31.206) by DGGEMS404-HUB.china.huawei.com (10.3.19.204) with Microsoft SMTP Server id 14.3.487.0; Sat, 20 Jun 2020 14:42:48 +0800 To: Andrew Rybchenko References: <1592538166-18617-1-git-send-email-xavier.huwei@huawei.com> <1592538166-18617-3-git-send-email-xavier.huwei@huawei.com> <2ca80248-08fb-b986-5617-12dcec747ce3@solarflare.com> CC: , Thomas Monjalon , Ferruh Yigit From: "Wei Hu (Xavier)" Message-ID: Date: Sat, 20 Jun 2020 14:42:48 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <2ca80248-08fb-b986-5617-12dcec747ce3@solarflare.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.69.31.206] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities 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" Hi, Andrew Rybchenko On 2020/6/19 16:37, Andrew Rybchenko wrote: > On 6/19/20 6:42 AM, Wei Hu (Xavier) wrote: >> From: Chengchang Tang >> >> Currently, there is a potential problem that calling the API function >> rte_eth_dev_set_vlan_offload to start a vlan hardware offloads which the >> driver does not support. if the PMD driver does not support the relative >> hardware offloads and does not check for it, the hardware setting will >> not change, but the relative offload in dev->data->dev_conf.rxmode.offloads >> will be turned on. > I'm sorry, but I don't understand what 'relative' means in the > context. > > Also, please, use 'VLAN' instead of 'vlan' in the description > and log message below. OK, I will fix it in V2. >> It is supposed to check the hardware capabilities to decide whether the >> relative callback needs to be called just like the behavior in the API >> function named rte_eth_dev_configure. > I think the direction of the fix is right, but it definitely > duplicates checks which are done in some PMDs. I think that it > would be good to do the cleanup in follow up patches. I will clean them in V2. >> Fixes: 81f9db8ecc2c ("ethdev: add vlan offload support") > Most likely it is incorrect, since generic Rx offloads were > introduced later. OK, I will replace it with the following statement in V2. Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API") Thanks, Xavier > >> Cc: stable@dpdk.org >> >> Signed-off-by: Chengchang Tang >> Signed-off-by: Wei Hu (Xavier) >> --- >> lib/librte_ethdev/rte_ethdev.c | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >> index 122fd2a..5176a0e 100644 >> --- a/lib/librte_ethdev/rte_ethdev.c >> +++ b/lib/librte_ethdev/rte_ethdev.c >> @@ -3261,12 +3261,14 @@ rte_eth_dev_set_vlan_ether_type(uint16_t port_id, >> int >> rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) >> { >> + struct rte_eth_dev_info dev_info; >> struct rte_eth_dev *dev; >> int ret = 0; >> int mask = 0; >> int cur, org = 0; >> uint64_t orig_offloads; >> uint64_t dev_offloads; >> + uint64_t new_offloads; >> >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> dev = &rte_eth_devices[port_id]; >> @@ -3320,6 +3322,25 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) >> if (mask == 0) >> return ret; >> >> + ret = rte_eth_dev_info_get(port_id, &dev_info); >> + if (ret != 0) >> + return ret; >> + >> + /* >> + * New added Rx vlan offloading which are not enabled in >> + * rte_eth_dev_configure() must be within its device capabilities >> + */ >> + if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) { >> + new_offloads = dev_offloads & ~orig_offloads; >> + RTE_ETHDEV_LOG(ERR, >> + "Ethdev port_id=%u requested new added vlan offloads " >> + "0x%" PRIx64 " must be within Rx offloads capabilities " >> + "0x%"PRIx64 " in %s()\n", >> + port_id, new_offloads, dev_info.rx_offload_capa, >> + __func__); >> + return -EINVAL; >> + } >> + >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); >> dev->data->dev_conf.rxmode.offloads = dev_offloads; >> ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); >> >