Re: [dpdk-stable] patch 'net/nfp: fix setting MAC address' has been queued to LTS release 18.11.2

[Kevin Traynor <ktraynor@redhat.com>]

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?= <pablo.cascon@netronome.com>
>> 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 <pablo.cascon@netronome.com>
>> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>> ---
>>  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");
>