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

[Pablo Cascón]

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