From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id CD8A3A05D3 for ; Fri, 26 Apr 2019 12:00:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BA8AB1B605; Fri, 26 Apr 2019 12:00:37 +0200 (CEST) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 41BDC1B605 for ; Fri, 26 Apr 2019 12:00:36 +0200 (CEST) Received: by mail-wm1-f68.google.com with SMTP id p21so1287337wmc.0 for ; Fri, 26 Apr 2019 03:00:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=qP8tbuAeJC2nTWL7HfOUY+kN4ayC9XhrJ5HkPSsPVWs=; b=tCLhDsDIKDThO9wQsBwEWEdZEheDTjWzx71WJ3BefWVCKAYOytUEPJXsQfxaLJYsZC 1x4fK4eDl88Cs2O3hyeU7u+MmYQN/5qjVPeiW8s59GR0Xi5Ofiq1QaiAE8YCJDAgRCQj WQeRoDZ5mSzcP26XClazH6bYSYoIJOta0dPr5NBeFCfgu0Z0u5m/6g9jjPxJfk9X6r6V 6i67gdhdrfvdFcx5mgznj8wb4fu/Q60yqEB3QrnGqDDCBIlc8uTLa6UcdhFo2AKfqDQR nq7PVwN+SNr+q6OxY0kXt7prLVHezUJH7J9Q0P2CBz/44F7YGTSJ/PSDJtZQ5wapp0c/ 7fDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=qP8tbuAeJC2nTWL7HfOUY+kN4ayC9XhrJ5HkPSsPVWs=; b=dqUMMgpN5H7sNgwOe6bQm8XifxaoCmZMesjMGL6mRz9Kiexa/glHHOy17w4A8OxSt6 tmmyek4zri4y9ADf1Etsb1/nERlufMvA9RX027FGciZsNsKbSG7CSL/3hFtPBPH6ei7u AFlXTObiKB+cQzQyLJxTwSDk2HGNM72jtBcNU+Z1FO6FjVWeJBfuOJKa5ppv09k3b7si yXe2Qs1UUylw/h6Jd5/8jvEkb4CfhGcWBJiyrhQ7yIqJjTy/UZv3ny7/qqymtD9D4CTq UsIvBC72GxnaSGZ/N6SpL7U9zC82Fmb0oFiyeWZLdEx7F6uhU9+8RUgYPg6uXUwKHYvz /u/Q== X-Gm-Message-State: APjAAAVxyRaNT1WT3tQC0z5rPDrDDqS1osA4uRyoey2Ybl9zxPZtIukg rl4E/eTFBOtObYhtz4daOuLq4+6wF4o= X-Google-Smtp-Source: APXvYqyWook/88oPxAGeeyKgLviGYTuFSq+Ahyl0qSqJmmUFZ8QhnCZQyUrTomi7CY3qKu1ILge3hA== X-Received: by 2002:a1c:be08:: with SMTP id o8mr2170183wmf.76.1556272835056; Fri, 26 Apr 2019 03:00:35 -0700 (PDT) Received: from [172.20.1.137] ([217.38.71.146]) by smtp.googlemail.com with ESMTPSA id v190sm26526886wme.18.2019.04.26.03.00.33 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Fri, 26 Apr 2019 03:00:34 -0700 (PDT) To: Kevin Traynor Cc: Alejandro Lucero , dpdk stable References: <20190416143719.21601-1-ktraynor@redhat.com> <20190416143719.21601-7-ktraynor@redhat.com> <6188fc4d-eccb-d461-8df1-1f60a6e68849@netronome.com> <6cb0fd49-52f3-9069-d8e2-6ec2357bafb8@redhat.com> <6b508e91-2113-c2f5-b855-0320f6b6103e@netronome.com> <39aad816-5cac-86d6-b414-3c79aed86ca1@redhat.com> <0a5fbd33-8635-9de8-cd25-2116bc86395f@redhat.com> From: =?UTF-8?Q?Pablo_Casc=c3=b3n?= Message-ID: <0bf173e5-a332-b99f-c9da-f8e0b66720cb@netronome.com> Date: Fri, 26 Apr 2019 11:00:33 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <0a5fbd33-8635-9de8-cd25-2116bc86395f@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [dpdk-stable] patch 'net/nfp: fix setting MAC address' has been queued to LTS release 18.11.2 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 26/04/2019 10:32, Kevin Traynor wrote: > On 26/04/2019 10:26, Pablo Cascón wrote: >> On 25/04/2019 17:02, Kevin Traynor wrote: >>> On 23/04/2019 14:35, Pablo Cascón wrote: >>>> Thank you very much for walking me through it, it's crystal clear now :)  Please push the changes, at least no objection from me. >>>> >>>> (Also got the extra tip from a colleague to use Thunderbird's extension Colored Diffs where the tiny extra space that differentiates a context from the actual change is a lot more visible. Wish I had good feedback to contribute, can only think of: a) educating those struggling (thx!) b) no diff context (any downside?) and c) also sharing the stable patch?) >>>> >>> For the latest batch of patches I reduced the patch to patch diff >>> context to zero, so every line in the diff is an actual change, either >>> metadata or code. The downside of no context is it can be hard to >>> understand the changes, so I also pushed the queued patches to github. >>> >>> I think now it can be quicker to see if there is any code changes and >>> the code can be viewed in full context if needed. >>> >>> Kevin. >> That sounds like a good idea, thanks for adapting. Can't find the github for this though, just out of curiosity would you mind sharing the link? >> > Sure - it's here https://github.com/kevintraynor/dpdk-stable-queue.git That's perfect, thanks! > >>>> Pablo >>>> >>>> On 23/04/2019 14:03, Kevin Traynor wrote: >>>>> On 23/04/2019 11:24, Pablo Cascón wrote: >>>>>> Hi, >>>>>> >>>>> Hi Pablo, >>>>> >>>>>> thanks for the reply and explaining the process. >>>>>> >>>>>> Unfortunately the diff at the end of your commit doesn't match what I was expecting. Have tried to generate it here and could only get changes related to the rebase (metada diffs) but no code changes. And your diff seems to imply code changes, not 100% sure. Could you double check the stable patch and diff please. Happy to send a patch of what I would like to land into stable >>>>>> >>>>> I checked and the change is the same in stable as master, so it looks ok. >>>>> >>>>> I think I know what's causing the confusion. The diff is a diff of the >>>>> *patches*, not the code. Both patches have +/- LOC in them and one is >>>>> being brought into context by a metadata change between the patches. >>>>> There is a difference in the indent of the +/-'s of the patches >>>>> themselves vs the diff between the patches but it's hard to distinguish. >>>>> See below: >>>>> >>>>> -@@ -576,5 +576,8 @@ nfp_set_mac_addr(struct rte_eth_dev *dev, struct >>>>> ether_addr *mac_addr) >>>>> +@@ -567,5 +567,8 @@ nfp_set_mac_addr(struct rte_eth_dev *dev, struct >>>>> >>>>> ^ '+' and '-' is a metadata diff between the patches, context below >>>>> >>>>> ether_addr *mac_addr) >>>>> /* Signal the NIC about the change */ >>>>> update = NFP_NET_CFG_UPDATE_MACADDR; >>>>> - ctrl = hw->ctrl | NFP_NET_CFG_CTRL_LIVE_ADDR; >>>>> >>>>> ^ '-' is part of both patches that removes this LOC >>>>> ^ no diff between the patches >>>>> >>>>> >>>>> Thanks for checking the patch and raising this. We can think about how >>>>> to make it clearer - perhaps the diff would be better without any context. >>>>> >>>>> Kevin. >>>>> >>>>>> For clarity this is what I tried, perhaps incorrectly, please let me know: >>>>>> 1) generate patch from dpdk upstream commit: >>>>>>     dpdk⟫ git format-patch -o /tmp/ -n -1 027412f >>>>>> 2) import it into stable >>>>>>     dpdk-stable⟫ git am /tmp/0001-net-nfp-fix-setting-MAC-address.patch >>>>>> 3) generate patch of just imported one: >>>>>>     dpdk-stable⟫ git format-patch -o /tmp/stable/ -n -1 HEAD >>>>>> 4) diff the checking there are only rebase related changes: >>>>>> >>>>>> dpdk-stable⟫ diff /tmp/0001-net-nfp-fix-setting-MAC-address.patch /tmp/stable/0001-net-nfp-fix-setting-MAC-address.patch >>>>>> 1c1 >>>>>> < From 027412fe949c763fd4d536b13dcb4432f2df5534 Mon Sep 17 00:00:00 2001 >>>>>> --- >>>>>>> From 3e5c4cd12ff01df228124d92905e4e7d563b652e Mon Sep 17 00:00:00 2001 >>>>>> 30c30 >>>>>> < index a791e95..1b7b6c2 100644 >>>>>> --- >>>>>>> index 54c6da9..278e154 100644 >>>>>> 33c33 >>>>>> < @@ -575,7 +575,10 @@ nfp_set_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr) >>>>>> --- >>>>>>> @@ -566,7 +566,10 @@ nfp_set_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr) >>>>>> Thanks, >>>>>> >>>>>> Pablo >>>>>> >>>>>> On 16/04/2019 15:36, Kevin Traynor wrote: >>>>>>> Hi, >>>>>>> >>>>>>> FYI, your patch has been queued to LTS release 18.11.2 >>>>>>> >>>>>>> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. >>>>>>> It will be pushed if I get no objections before 04/24/19. So please >>>>>>> shout if anyone has objections. >>>>>>> >>>>>>> Also note that after the patch there's a diff of the upstream commit vs the >>>>>>> patch applied to the branch. This will indicate if there was any rebasing >>>>>>> needed to apply to the stable branch. If there were code changes for rebasing >>>>>>> (ie: not only metadata diffs), please double check that the rebase was >>>>>>> correctly done. >>>>>>> >>>>>>> Thanks. >>>>>>> >>>>>>> Kevin Traynor >>>>>>> >>>>>>> --- >>>>>>> From 8fb7943ea804d437fc5b7153d7c073c79eeb3837 Mon Sep 17 00:00:00 2001 >>>>>>> From: =?UTF-8?q?Pablo=20Casc=C3=B3n?= >>>>>>> Date: Fri, 8 Mar 2019 15:40:47 +0000 >>>>>>> Subject: [PATCH] net/nfp: fix setting MAC address >>>>>>> MIME-Version: 1.0 >>>>>>> Content-Type: text/plain; charset=UTF-8 >>>>>>> Content-Transfer-Encoding: 8bit >>>>>>> >>>>>>> [ upstream commit 027412fe949c763fd4d536b13dcb4432f2df5534 ] >>>>>>> >>>>>>> Some firmwares, mostly for VFs, do not advertise the feature / >>>>>>> capability of changing the MAC address while the interface is up. With >>>>>>> such firmware a request to change the MAC address that at the same >>>>>>> time also tries to enable the not available feature will be denied by >>>>>>> the firmware resulting in an error message like: >>>>>>> >>>>>>> nfp_net_reconfig(): Error nfp_net reconfig for ctrl: 80000000 update: 800 >>>>>>> >>>>>>> Fix set_mac_addr by not trying to enable a feature if it is not >>>>>>> advertised by the firmware. >>>>>>> >>>>>>> Fixes: 2fe669f4bcd2 ("net/nfp: support MAC address change") >>>>>>> >>>>>>> Signed-off-by: Pablo Cascón >>>>>>> Acked-by: Alejandro Lucero >>>>>>> --- >>>>>>> drivers/net/nfp/nfp_net.c | 5 ++++- >>>>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c >>>>>>> index 54c6da924..278e154cd 100644 >>>>>>> --- a/drivers/net/nfp/nfp_net.c >>>>>>> +++ b/drivers/net/nfp/nfp_net.c >>>>>>> @@ -567,5 +567,8 @@ nfp_set_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr) >>>>>>> /* Signal the NIC about the change */ >>>>>>> update = NFP_NET_CFG_UPDATE_MACADDR; >>>>>>> - ctrl = hw->ctrl | NFP_NET_CFG_CTRL_LIVE_ADDR; >>>>>>> + ctrl = hw->ctrl; >>>>>>> + if ((hw->ctrl & NFP_NET_CFG_CTRL_ENABLE) && >>>>>>> + (hw->cap & NFP_NET_CFG_CTRL_LIVE_ADDR)) >>>>>>> + ctrl |= NFP_NET_CFG_CTRL_LIVE_ADDR; >>>>>>> if (nfp_net_reconfig(hw, ctrl, update) < 0) { >>>>>>> PMD_INIT_LOG(INFO, "MAC address update failed");