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 13040A0526; Thu, 9 Jul 2020 09:40:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EA8EF1D538; Thu, 9 Jul 2020 09:40:01 +0200 (CEST) Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by dpdk.org (Postfix) with ESMTP id 6B63E1C2DD for ; Thu, 9 Jul 2020 09:40:00 +0200 (CEST) Received: from DGGEMS409-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id B3F50E6E8308274A3631; Thu, 9 Jul 2020 15:39:57 +0800 (CST) Received: from [10.69.31.206] (10.69.31.206) by DGGEMS409-HUB.china.huawei.com (10.3.19.209) with Microsoft SMTP Server id 14.3.487.0; Thu, 9 Jul 2020 15:39:47 +0800 To: Thomas Monjalon References: <1594019191-54524-1-git-send-email-xavier.huwei@huawei.com> <3633587.h79vMgjPB0@thomas> <40e3a1b0-3c7e-1807-67e1-41dd2850e350@huawei.com> <4862684.ztQDoVm40t@thomas> CC: , , , , , , , , From: "Wei Hu (Xavier)" Message-ID: <54cebe1e-b6a0-30c5-824d-6ed56248e5ca@huawei.com> Date: Thu, 9 Jul 2020 15:39:47 +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: <4862684.ztQDoVm40t@thomas> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.69.31.206] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v6 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" On 2020/7/8 18:14, Thomas Monjalon wrote: > 08/07/2020 05:37, Wei Hu (Xavier): >> On 2020/7/7 22:11, Thomas Monjalon wrote: >>> 06/07/2020 09:06, Wei Hu (Xavier): >>>> Currently, there is a potential problem that calling the API function >>>> rte_eth_dev_set_vlan_offload to start VLAN hardware offloads which the >>>> driver does not support. If the PMD driver does not support certain VLAN >>>> hardware offloads and does not check for it, the hardware setting will >>>> not change, but the VLAN offloads in dev->data->dev_conf.rxmode.offloads >>>> will be turned on. >>>> >>>> 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. And it is also needed to cleanup >>>> duplicated checks which are done in some PMDs. Also, note that it is >>>> behaviour change for some PMDs which simply ignore (with error/warning log >>>> message) unsupported VLAN offloads, but now it will fail. > [...] >>>> @@ -3317,6 +3319,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 >>>> + */ >>> What means "New added Rx VLAN offloading"? >> The parameter offload_mask of rte_eth_dev_set_vlan_offload() function >> includes some Rx VLAN offload, and some of them maybe are not enabled >> in rte_eth_dev_configure(). > OK > > I don't understand why checking only new features. > All enabled features must be within capabilities, right? Yes，you are right. all enabled features must be within capabilities， Some features enabled in rte_eth_dev_configure() had been already checked, So the comment here emphasizes 'new added Rx VLAN offloading'. Thanks，Xavier > > > > . >