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 9436FA051A; Fri, 17 Jan 2020 12:46:01 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2E1FD11A4; Fri, 17 Jan 2020 12:46:00 +0100 (CET) Received: from m12-11.163.com (m12-11.163.com [220.181.12.11]) by dpdk.org (Postfix) with ESMTP id AD2BBF11; Fri, 17 Jan 2020 12:45:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Subject:From:Message-ID:Date:MIME-Version; bh=xzJlh hLBVMWkl5yUmiaL3KLFsKSwh0XWrMgvYeyaks8=; b=IhxlIf9gkXD8HjwZm6xvt MJg5bAW3VTQ2AZGZQP2nnDpSDmiseq/s+TZMjBbicpxWPqavMXIzKCwhmRggLOkg QXYb7JN5tjJetzY6ErMt3mpymXA8cDeRkhhkKMThLl0V5YpCwxgSbcOL7aCokHX8 kHV7CeoCRCPOnaBkNO9oMs= Received: from [192.168.1.199] (unknown [139.159.243.11]) by smtp7 (Coremail) with SMTP id C8CowABnbxtyniFeMtJHHg--.653S2; Fri, 17 Jan 2020 19:45:55 +0800 (CST) To: Andrew Rybchenko , "Wei Hu (Xavier)" , dev@dpdk.org Cc: stable@dpdk.org, thomas@monjalon.net, ferruh.yigit@intel.com References: <20200117062342.32870-1-huwei013@chinasoftinc.com> From: "Wei Hu (Xavier)" Message-ID: <1adaf1bc-c917-1c1d-baeb-9827248ce9f0@163.com> Date: Fri, 17 Jan 2020 19:45:54 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-CM-TRANSID: C8CowABnbxtyniFeMtJHHg--.653S2 X-Coremail-Antispam: 1Uf129KBjvJXoW7AF4DtrWkCr4rCr1UXFWUJwb_yoW8Kw4UpF WfJay0kFWkGr1fur1xCayIkryUAF4DJF4YyrW7ta43AasxZryruryDK34jkFy8Awn3Kr4Y vFWFqFySgr1Uua7anT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07b83ktUUUUU= X-Originating-IP: [139.159.243.11] X-CM-SenderInfo: 50dyxv5ubk34lhl6il2tof0z/1tbiVBmto1UML1-2aAABs1 Subject: Re: [dpdk-dev] [PATCH v2] ethdev: fix updating device data when ops pointer is NULL 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/1/17 15:57, Andrew Rybchenko wrote: > On 1/17/20 9:23 AM, Wei Hu (Xavier) wrote: >> From: "Wei Hu (Xavier)" > > I think summary is misleading. It is too generic and does not > mention VLAN offloads. Consider something like: > ethdev: fix VLAN offloads set if no driver callback > >> Currently, there is a potential problem that changing the content of >> dev->data->dev_conf.rxmode.offloads even when the pointer named >> dev->dev_ops->vlan_offload_set is NULL in the API function named >> rte_eth_dev_set_vlan_offload. > > Consider to change > "when the pointer named dev->dev_ops->vlan_offload_set is NULL in the > API function named rte_eth_dev_set_vlan_offload." to "when there is no > vlan_offload_set driver callback." > >> It is a good idea that prevent the side effect and make the API return >> success if no change requested. This patch fixes the problem, the detail >> information as below: >> - Update a local 'dev_offloads' variable, instead of directly updating the >> device config data >> - if no change requested, "mask == 0", return success >> - if change requested, check the 'vlan_offload_set' dev_ops in this stage >> - assign the local 'dev_offloads' to 'dev->data->dev_conf.rxmode.offloads' >> and call the dev_ops >> - On error recover the 'dev->data->dev_conf.rxmode.offloads' to >> 'orig_offloads' > > I see no point to repeat in description what the code does. > IMHO it is sufficient to mention main ideas: > - keep possibility to do dummy set even if there is no driver > callback > - do not touch Rx mode offloads in device data before > checking the driver callback availability > - ensure that Rx mode offloads are rolled back correctly if > driver callback returns error > >> Fixes: 01eb53eefeb4 ("ethdev: rename folder to library name") > > I think it is wrong Fixes. Rename just moves the code, it is > not the changeset which introduces the bug. > >> Cc: stable@dpdk.org >> >> Signed-off-by: Wei Hu (Xavier) >> Signed-off-by: Chunsong Feng >> Signed-off-by: Min Wang (Jushui) >> Signed-off-by: Min Hu (Connor) > > with above notes processed > > Reviewed-by: Andrew Rybchenko > Thanks for your comments, I will update it and send V3. Regards Xavier