* [PATCH] enic: Store permanent MAC address during probe()
@ 2017-03-20 22:41 PJ Waskiewicz
2017-03-21 0:33 ` Govindarajulu Varadarajan
2017-03-22 18:26 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: PJ Waskiewicz @ 2017-03-20 22:41 UTC (permalink / raw)
To: netdev, Christian Benvenuti, Sujith Sankar,
Govindarajulu Varadarajan, Neel Patel
Cc: PJ Waskiewicz
From: PJ Waskiewicz <pjwaskiewicz@gmail.com>
The permanent MAC address is useful to store for things like ethtool,
and when bonding with modes such as active/passive or LACP. This
follows the model of other Ethernet drivers, such as ixgbe.
This was verified on a C220 chassis with the Cisco VNIC Ethernet device.
Signed-off-by: PJ Waskiewicz <pjwaskiewicz@gmail.com>
---
drivers/net/ethernet/cisco/enic/enic_main.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 4b87bee..8bb2114 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -964,6 +964,16 @@ void enic_reset_addr_lists(struct enic *enic)
enic->flags = 0;
}
+static int enic_set_perm_mac_addr(struct net_device *netdev, char *addr)
+{
+ if (!is_valid_ether_addr(addr) && !is_zero_ether_addr(addr))
+ return -EADDRNOTAVAIL;
+
+ memcpy(netdev->perm_addr, addr, netdev->addr_len);
+
+ return 0;
+}
+
static int enic_set_mac_addr(struct net_device *netdev, char *addr)
{
struct enic *enic = netdev_priv(netdev);
@@ -2872,6 +2882,14 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_out_dev_deinit;
}
+ /* Store off permanent MAC address
+ */
+ err = enic_set_perm_mac_addr(netdev, enic->mac_addr);
+ if (err) {
+ dev_err(dev, "Invalid MAC address, aborting\n");
+ goto err_out_dev_deinit;
+ }
+
enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
/* rx coalesce time already got initialized. This gets used
* if adaptive coal is turned off
--
2.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] enic: Store permanent MAC address during probe()
2017-03-20 22:41 [PATCH] enic: Store permanent MAC address during probe() PJ Waskiewicz
@ 2017-03-21 0:33 ` Govindarajulu Varadarajan
2017-03-21 0:37 ` PJ Waskiewicz
2017-03-22 18:26 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Govindarajulu Varadarajan @ 2017-03-21 0:33 UTC (permalink / raw)
To: PJ Waskiewicz
Cc: netdev, Christian Benvenuti, Govindarajulu Varadarajan,
Neel Patel
On Mon, 20 Mar 2017, PJ Waskiewicz wrote:
> From: PJ Waskiewicz <pjwaskiewicz@gmail.com>
>
> The permanent MAC address is useful to store for things like ethtool,
> and when bonding with modes such as active/passive or LACP.
Hi Peter,
Is this patch fixing an issue with bonding drive on enic?
> This follows the model of other Ethernet drivers, such as ixgbe.
>
While other drivers set netdev->perm_addr, doesn't this actually come free in
register_netdevice().
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] enic: Store permanent MAC address during probe()
2017-03-21 0:33 ` Govindarajulu Varadarajan
@ 2017-03-21 0:37 ` PJ Waskiewicz
2017-03-21 1:49 ` Govindarajulu Varadarajan
0 siblings, 1 reply; 6+ messages in thread
From: PJ Waskiewicz @ 2017-03-21 0:37 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: netdev, Christian Benvenuti, Govindarajulu Varadarajan,
Neel Patel
On Mon, Mar 20, 2017 at 5:33 PM, Govindarajulu Varadarajan
<gvaradar@cisco.com> wrote:
> On Mon, 20 Mar 2017, PJ Waskiewicz wrote:
>
>> From: PJ Waskiewicz <pjwaskiewicz@gmail.com>
>>
>> The permanent MAC address is useful to store for things like ethtool,
>> and when bonding with modes such as active/passive or LACP.
>
>
> Hi Peter,
>
> Is this patch fixing an issue with bonding drive on enic?
We noticed that running ethtool -P <eth> on an enic, even on 4.9,
returned nothing. This has fallout when using bonding, where LACP or
Active/Passive overrides the LAA on one of the slaves, one can't
figure out what the physical MAC address is of each slave. So not a
problem with bonding directly, it's more secondary as a result of the
driver not reporting the actual permanent address.
>
>> This follows the model of other Ethernet drivers, such as ixgbe.
>>
>
> While other drivers set netdev->perm_addr, doesn't this actually come free
> in
> register_netdevice().
I thought it did as well, but in 4.9 when we tested it wasn't working.
Hence the patch. :-)
Cheers,
-PJ
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] enic: Store permanent MAC address during probe()
2017-03-21 0:37 ` PJ Waskiewicz
@ 2017-03-21 1:49 ` Govindarajulu Varadarajan
2017-03-21 6:20 ` PJ Waskiewicz
0 siblings, 1 reply; 6+ messages in thread
From: Govindarajulu Varadarajan @ 2017-03-21 1:49 UTC (permalink / raw)
To: PJ Waskiewicz
Cc: netdev, Christian Benvenuti, Govindarajulu Varadarajan,
Neel Patel
On Mon, 20 Mar 2017, PJ Waskiewicz wrote:
> On Mon, Mar 20, 2017 at 5:33 PM, Govindarajulu Varadarajan
> <gvaradar@cisco.com> wrote:
>> On Mon, 20 Mar 2017, PJ Waskiewicz wrote:
>>
>>> From: PJ Waskiewicz <pjwaskiewicz@gmail.com>
>>>
>>> The permanent MAC address is useful to store for things like ethtool,
>>> and when bonding with modes such as active/passive or LACP.
>>
>> Is this patch fixing an issue with bonding drive on enic?
>
> We noticed that running ethtool -P <eth> on an enic, even on 4.9,
> returned nothing. This has fallout when using bonding, where LACP or
> Active/Passive overrides the LAA on one of the slaves, one can't
> figure out what the physical MAC address is of each slave. So not a
> problem with bonding directly, it's more secondary as a result of the
> driver not reporting the actual permanent address.
>
>>
>>> This follows the model of other Ethernet drivers, such as ixgbe.
>>>
>>
>> While other drivers set netdev->perm_addr, doesn't this actually come free
>> in
>> register_netdevice().
>
> I thought it did as well, but in 4.9 when we tested it wasn't working.
> Hence the patch. :-)
>
Can you try with net-next? In my setup I do not see the issue on net-next and on
4.9 kernel. The issue for all drivers was fixed in
948b337e62ca9 ("net: init perm_addr in register_netdevice()")
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] enic: Store permanent MAC address during probe()
2017-03-21 1:49 ` Govindarajulu Varadarajan
@ 2017-03-21 6:20 ` PJ Waskiewicz
0 siblings, 0 replies; 6+ messages in thread
From: PJ Waskiewicz @ 2017-03-21 6:20 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: netdev, Christian Benvenuti, Govindarajulu Varadarajan,
Neel Patel
On Mon, Mar 20, 2017 at 6:49 PM, Govindarajulu Varadarajan
<gvaradar@cisco.com> wrote:
> On Mon, 20 Mar 2017, PJ Waskiewicz wrote:
>
>> On Mon, Mar 20, 2017 at 5:33 PM, Govindarajulu Varadarajan
>> <gvaradar@cisco.com> wrote:
>>>
>>> On Mon, 20 Mar 2017, PJ Waskiewicz wrote:
>>>
>>>> From: PJ Waskiewicz <pjwaskiewicz@gmail.com>
>>>>
>>>> The permanent MAC address is useful to store for things like ethtool,
>>>> and when bonding with modes such as active/passive or LACP.
>>>
>>>
>>> Is this patch fixing an issue with bonding drive on enic?
>>
>>
>> We noticed that running ethtool -P <eth> on an enic, even on 4.9,
>> returned nothing. This has fallout when using bonding, where LACP or
>> Active/Passive overrides the LAA on one of the slaves, one can't
>> figure out what the physical MAC address is of each slave. So not a
>> problem with bonding directly, it's more secondary as a result of the
>> driver not reporting the actual permanent address.
>>
>>>
>>>> This follows the model of other Ethernet drivers, such as ixgbe.
>>>>
>>>
>>> While other drivers set netdev->perm_addr, doesn't this actually come
>>> free
>>> in
>>> register_netdevice().
>>
>>
>> I thought it did as well, but in 4.9 when we tested it wasn't working.
>> Hence the patch. :-)
>>
>
> Can you try with net-next? In my setup I do not see the issue on net-next
> and on
> 4.9 kernel. The issue for all drivers was fixed in
> 948b337e62ca9 ("net: init perm_addr in register_netdevice()")
The fix looks like it went in after 4.9 was tagged and released. 4.9
was tagged 12/11/2016, and 948b337e62ca9 was committed 1/8/2017. That
would explain why I didn't see it in 4.9.
That being said, looks like 4.10 does work as expected without my
patch, so I'm fine carrying the patch internally to our 4.9 tree. I'm
not sure it's worth sending either this patch or the netdev-level
patch to -stable though, it's a small issue that is already fixed
upstream.
Consider this patch rescinded.
Cheers,
-PJ
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] enic: Store permanent MAC address during probe()
2017-03-20 22:41 [PATCH] enic: Store permanent MAC address during probe() PJ Waskiewicz
2017-03-21 0:33 ` Govindarajulu Varadarajan
@ 2017-03-22 18:26 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2017-03-22 18:26 UTC (permalink / raw)
To: pjwaskiewicz; +Cc: netdev, benve, ssujith, _govind, neepatel
From: PJ Waskiewicz <pjwaskiewicz@gmail.com>
Date: Mon, 20 Mar 2017 15:41:20 -0700
> From: PJ Waskiewicz <pjwaskiewicz@gmail.com>
>
> The permanent MAC address is useful to store for things like ethtool,
> and when bonding with modes such as active/passive or LACP. This
> follows the model of other Ethernet drivers, such as ixgbe.
>
> This was verified on a C220 chassis with the Cisco VNIC Ethernet device.
>
> Signed-off-by: PJ Waskiewicz <pjwaskiewicz@gmail.com>
As per the discussion, upstream fixes this generically, so I'm dropping
this.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-22 18:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-20 22:41 [PATCH] enic: Store permanent MAC address during probe() PJ Waskiewicz
2017-03-21 0:33 ` Govindarajulu Varadarajan
2017-03-21 0:37 ` PJ Waskiewicz
2017-03-21 1:49 ` Govindarajulu Varadarajan
2017-03-21 6:20 ` PJ Waskiewicz
2017-03-22 18:26 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).