Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-17 Thread Geert Uytterhoeven
On Tue, Nov 17, 2015 at 10:02 AM, Geert Uytterhoeven
 wrote:
> On Fri, Nov 13, 2015 at 1:26 AM, Florian Fainelli  
> wrote:
>> On 04/11/15 18:56, David Miller wrote:
 Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
 features down stack")
>>>  ...
 Reported-by: Nikolay Aleksandrov 
 Signed-off-by: Jarod Wilson 
 ---
 v2: Based on suggestions from Alex, and with not changing err to ret, this
 patch actually becomes quite minimal and doesn't ugly up the code much.
>>>
>>> Applied, thanks.
>>
>> This causes some warnings to be displayed for DSA stacked devices:
>
> And a few more:
>
> sh-eth ee70.ethernet eth0: set_features() failed (-1); wanted
> 0x4000, left 0x4800
> g_ether gadget usb0: set_features() failed (-1); wanted
> 0x4000, left 0x4800

smsc911x 800.ethernet eth0: set_features() failed (-1); wanted
0x4000, left 0x4800

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-17 Thread Geert Uytterhoeven
On Fri, Nov 13, 2015 at 1:26 AM, Florian Fainelli  wrote:
> On 04/11/15 18:56, David Miller wrote:
>>> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
>>> features down stack")
>>  ...
>>> Reported-by: Nikolay Aleksandrov 
>>> Signed-off-by: Jarod Wilson 
>>> ---
>>> v2: Based on suggestions from Alex, and with not changing err to ret, this
>>> patch actually becomes quite minimal and doesn't ugly up the code much.
>>
>> Applied, thanks.
>
> This causes some warnings to be displayed for DSA stacked devices:

And a few more:

sh-eth ee70.ethernet eth0: set_features() failed (-1); wanted
0x4000, left 0x4800
g_ether gadget usb0: set_features() failed (-1); wanted
0x4000, left 0x4800

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-17 Thread Geert Uytterhoeven
On Tue, Nov 17, 2015 at 10:02 AM, Geert Uytterhoeven
 wrote:
> On Fri, Nov 13, 2015 at 1:26 AM, Florian Fainelli  
> wrote:
>> On 04/11/15 18:56, David Miller wrote:
 Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
 features down stack")
>>>  ...
 Reported-by: Nikolay Aleksandrov 
 Signed-off-by: Jarod Wilson 
 ---
 v2: Based on suggestions from Alex, and with not changing err to ret, this
 patch actually becomes quite minimal and doesn't ugly up the code much.
>>>
>>> Applied, thanks.
>>
>> This causes some warnings to be displayed for DSA stacked devices:
>
> And a few more:
>
> sh-eth ee70.ethernet eth0: set_features() failed (-1); wanted
> 0x4000, left 0x4800
> g_ether gadget usb0: set_features() failed (-1); wanted
> 0x4000, left 0x4800

smsc911x 800.ethernet eth0: set_features() failed (-1); wanted
0x4000, left 0x4800

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-17 Thread Geert Uytterhoeven
On Fri, Nov 13, 2015 at 1:26 AM, Florian Fainelli  wrote:
> On 04/11/15 18:56, David Miller wrote:
>>> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
>>> features down stack")
>>  ...
>>> Reported-by: Nikolay Aleksandrov 
>>> Signed-off-by: Jarod Wilson 
>>> ---
>>> v2: Based on suggestions from Alex, and with not changing err to ret, this
>>> patch actually becomes quite minimal and doesn't ugly up the code much.
>>
>> Applied, thanks.
>
> This causes some warnings to be displayed for DSA stacked devices:

And a few more:

sh-eth ee70.ethernet eth0: set_features() failed (-1); wanted
0x4000, left 0x4800
g_ether gadget usb0: set_features() failed (-1); wanted
0x4000, left 0x4800

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-13 Thread Laura Abbott

On 11/13/2015 02:51 AM, Nikolay Aleksandrov wrote:

On 11/13/2015 11:29 AM, Jiri Pirko wrote:

Fri, Nov 13, 2015 at 01:26:18AM CET, f.faine...@gmail.com wrote:

On 04/11/15 18:56, David Miller wrote:

Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features down 
stack")

  ...

Reported-by: Nikolay Aleksandrov 
Signed-off-by: Jarod Wilson 
---
v2: Based on suggestions from Alex, and with not changing err to ret, this
patch actually becomes quite minimal and doesn't ugly up the code much.


Applied, thanks.


This causes some warnings to be displayed for DSA stacked devices:

[1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
[1.283181] libphy: dsa slave smi: probed
[1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
patch: 3
[1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
attached PHY at address 5 [Broadcom BCM7445]
[1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
(uninitialized): attached PHY at address 0 [Generic PHY]
[1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
(uninitialized): attached PHY at address 1 [Generic PHY]
[1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
attached PHY at address 2 [Generic PHY]
[1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
failed (-1); wanted 0x4020, left 0x4820

DSA slave network devices are not associated with their master network
device using the typical lower/upper netdev helpers.

I do not have a good fix to come up with yet, but if you see something
obvious with net/dsa/slave.c, feel free to send patches for testing, I
can boot net-next on this platform.


I'm having similar issues with bridge, with linus's git now:


[snip]

Hmm, I think it's because the bridge and dsa/slave don't have ndo_set_features()
so err is left as -1 and thus an error is reported which isn't actually true.
Before in this case the features would just get set, so could you please try
the following patch ?


diff --git a/net/core/dev.c b/net/core/dev.c
index ab9b8d0d115e..4a1d198dbbff 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6426,6 +6426,8 @@ int __netdev_update_features(struct net_device *dev)

if (dev->netdev_ops->ndo_set_features)
err = dev->netdev_ops->ndo_set_features(dev, features);
+   else
+   err = 0;

if (unlikely(err < 0)) {
netdev_err(dev,



The patch seems to be working for at least one person who reported the
problem in Fedora rawhide https://bugzilla.redhat.com/show_bug.cgi?id=1281674

Thanks,
Laura

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-13 Thread Nikolay Aleksandrov
On 11/13/2015 11:29 AM, Jiri Pirko wrote:
> Fri, Nov 13, 2015 at 01:26:18AM CET, f.faine...@gmail.com wrote:
>> On 04/11/15 18:56, David Miller wrote:
 Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
 features down stack")
>>>  ...
 Reported-by: Nikolay Aleksandrov 
 Signed-off-by: Jarod Wilson 
 ---
 v2: Based on suggestions from Alex, and with not changing err to ret, this
 patch actually becomes quite minimal and doesn't ugly up the code much.
>>>
>>> Applied, thanks.
>>
>> This causes some warnings to be displayed for DSA stacked devices:
>>
>> [1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
>> 4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
>> [1.283181] libphy: dsa slave smi: probed
>> [1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
>> patch: 3
>> [1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
>> attached PHY at address 5 [Broadcom BCM7445]
>> [1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>> [1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
>> (uninitialized): attached PHY at address 0 [Generic PHY]
>> [1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>> [1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
>> (uninitialized): attached PHY at address 1 [Generic PHY]
>> [1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>> [1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
>> attached PHY at address 2 [Generic PHY]
>> [1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>>
>> DSA slave network devices are not associated with their master network
>> device using the typical lower/upper netdev helpers.
>>
>> I do not have a good fix to come up with yet, but if you see something
>> obvious with net/dsa/slave.c, feel free to send patches for testing, I
>> can boot net-next on this platform.
> 
> I'm having similar issues with bridge, with linus's git now:
> 
[snip]

Hmm, I think it's because the bridge and dsa/slave don't have ndo_set_features()
so err is left as -1 and thus an error is reported which isn't actually true.
Before in this case the features would just get set, so could you please try
the following patch ?


diff --git a/net/core/dev.c b/net/core/dev.c
index ab9b8d0d115e..4a1d198dbbff 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6426,6 +6426,8 @@ int __netdev_update_features(struct net_device *dev)
 
if (dev->netdev_ops->ndo_set_features)
err = dev->netdev_ops->ndo_set_features(dev, features);
+   else
+   err = 0;
 
if (unlikely(err < 0)) {
netdev_err(dev,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-13 Thread Jiri Pirko
Fri, Nov 13, 2015 at 01:26:18AM CET, f.faine...@gmail.com wrote:
>On 04/11/15 18:56, David Miller wrote:
>>> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
>>> features down stack")
>>  ...
>>> Reported-by: Nikolay Aleksandrov 
>>> Signed-off-by: Jarod Wilson 
>>> ---
>>> v2: Based on suggestions from Alex, and with not changing err to ret, this
>>> patch actually becomes quite minimal and doesn't ugly up the code much.
>> 
>> Applied, thanks.
>
>This causes some warnings to be displayed for DSA stacked devices:
>
>[1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
>4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
>[1.283181] libphy: dsa slave smi: probed
>[1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
>patch: 3
>[1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
>attached PHY at address 5 [Broadcom BCM7445]
>[1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>[1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
>(uninitialized): attached PHY at address 0 [Generic PHY]
>[1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>[1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
>(uninitialized): attached PHY at address 1 [Generic PHY]
>[1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>[1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
>attached PHY at address 2 [Generic PHY]
>[1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>
>DSA slave network devices are not associated with their master network
>device using the typical lower/upper netdev helpers.
>
>I do not have a good fix to come up with yet, but if you see something
>obvious with net/dsa/slave.c, feel free to send patches for testing, I
>can boot net-next on this platform.

I'm having similar issues with bridge, with linus's git now:


...
[   14.354362] br0: set_features() failed (-1); wanted 0x00801fd978a9, left 
0x00801fff78e9
[   14.430480] br0: set_features() failed (-1); wanted 0x00801fd978a9, left 
0x00801fff78e9
[   14.430550] IPv6: ADDRCONF(NETDEV_UP): br0: link is not ready
[   17.938637] tg3 :01:00.0 eno1: Link is up at 1000 Mbps, full duplex
[   17.938647] tg3 :01:00.0 eno1: Flow control is off for TX and off for RX
[   17.938651] tg3 :01:00.0 eno1: EEE is disabled
[   17.938669] IPv6: ADDRCONF(NETDEV_CHANGE): eno1: link becomes ready
[   17.938753] br0: port 1(eno1) entered forwarding state
[   17.938762] br0: port 1(eno1) entered forwarding state
[   17.938834] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
[   29.763514] FS-Cache: Loaded
[   29.917680] FS-Cache: Netfs 'nfs' registered for caching
[   29.936739] Key type dns_resolver registered
[   30.637482] NFS: Registering the id_resolver key type
[   30.637502] Key type id_resolver registered
[   30.637504] Key type id_legacy registered
[   31.286444] ip6_tables: (C) 2000-2006 Netfilter Core Team
[   31.403005] Ebtables v2.0 registered
[   31.630354] tun: Universal TUN/TAP device driver, 1.6
[   31.630358] tun: (C) 1999-2004 Max Krasnyansky 
[   31.630824] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   31.677764] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   31.677855] device virbr0-nic entered promiscuous mode
[   31.677898] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   31.904892] nf_conntrack version 0.5.0 (65536 buckets, 262144 max)
[   32.087094] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.087196] virbr0: port 1(virbr0-nic) entered listening state
[   32.087205] virbr0: port 1(virbr0-nic) entered listening state
[   32.093676] br0: set_features() failed (-1); wanted 0x00801fd978a9, left 
0x00801fff78e9
[   32.093786] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.093872] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   32.093966] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.094051] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   32.094132] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.124341] virbr0: port 1(virbr0-nic) entered disabled state

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  

Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-13 Thread Laura Abbott

On 11/13/2015 02:51 AM, Nikolay Aleksandrov wrote:

On 11/13/2015 11:29 AM, Jiri Pirko wrote:

Fri, Nov 13, 2015 at 01:26:18AM CET, f.faine...@gmail.com wrote:

On 04/11/15 18:56, David Miller wrote:

Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features down 
stack")

  ...

Reported-by: Nikolay Aleksandrov 
Signed-off-by: Jarod Wilson 
---
v2: Based on suggestions from Alex, and with not changing err to ret, this
patch actually becomes quite minimal and doesn't ugly up the code much.


Applied, thanks.


This causes some warnings to be displayed for DSA stacked devices:

[1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
[1.283181] libphy: dsa slave smi: probed
[1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
patch: 3
[1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
attached PHY at address 5 [Broadcom BCM7445]
[1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
(uninitialized): attached PHY at address 0 [Generic PHY]
[1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
(uninitialized): attached PHY at address 1 [Generic PHY]
[1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
attached PHY at address 2 [Generic PHY]
[1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
failed (-1); wanted 0x4020, left 0x4820

DSA slave network devices are not associated with their master network
device using the typical lower/upper netdev helpers.

I do not have a good fix to come up with yet, but if you see something
obvious with net/dsa/slave.c, feel free to send patches for testing, I
can boot net-next on this platform.


I'm having similar issues with bridge, with linus's git now:


[snip]

Hmm, I think it's because the bridge and dsa/slave don't have ndo_set_features()
so err is left as -1 and thus an error is reported which isn't actually true.
Before in this case the features would just get set, so could you please try
the following patch ?


diff --git a/net/core/dev.c b/net/core/dev.c
index ab9b8d0d115e..4a1d198dbbff 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6426,6 +6426,8 @@ int __netdev_update_features(struct net_device *dev)

if (dev->netdev_ops->ndo_set_features)
err = dev->netdev_ops->ndo_set_features(dev, features);
+   else
+   err = 0;

if (unlikely(err < 0)) {
netdev_err(dev,



The patch seems to be working for at least one person who reported the
problem in Fedora rawhide https://bugzilla.redhat.com/show_bug.cgi?id=1281674

Thanks,
Laura

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-13 Thread Jiri Pirko
Fri, Nov 13, 2015 at 01:26:18AM CET, f.faine...@gmail.com wrote:
>On 04/11/15 18:56, David Miller wrote:
>>> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
>>> features down stack")
>>  ...
>>> Reported-by: Nikolay Aleksandrov 
>>> Signed-off-by: Jarod Wilson 
>>> ---
>>> v2: Based on suggestions from Alex, and with not changing err to ret, this
>>> patch actually becomes quite minimal and doesn't ugly up the code much.
>> 
>> Applied, thanks.
>
>This causes some warnings to be displayed for DSA stacked devices:
>
>[1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
>4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
>[1.283181] libphy: dsa slave smi: probed
>[1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
>patch: 3
>[1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
>attached PHY at address 5 [Broadcom BCM7445]
>[1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>[1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
>(uninitialized): attached PHY at address 0 [Generic PHY]
>[1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>[1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
>(uninitialized): attached PHY at address 1 [Generic PHY]
>[1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>[1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
>attached PHY at address 2 [Generic PHY]
>[1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
>failed (-1); wanted 0x4020, left 0x4820
>
>DSA slave network devices are not associated with their master network
>device using the typical lower/upper netdev helpers.
>
>I do not have a good fix to come up with yet, but if you see something
>obvious with net/dsa/slave.c, feel free to send patches for testing, I
>can boot net-next on this platform.

I'm having similar issues with bridge, with linus's git now:


...
[   14.354362] br0: set_features() failed (-1); wanted 0x00801fd978a9, left 
0x00801fff78e9
[   14.430480] br0: set_features() failed (-1); wanted 0x00801fd978a9, left 
0x00801fff78e9
[   14.430550] IPv6: ADDRCONF(NETDEV_UP): br0: link is not ready
[   17.938637] tg3 :01:00.0 eno1: Link is up at 1000 Mbps, full duplex
[   17.938647] tg3 :01:00.0 eno1: Flow control is off for TX and off for RX
[   17.938651] tg3 :01:00.0 eno1: EEE is disabled
[   17.938669] IPv6: ADDRCONF(NETDEV_CHANGE): eno1: link becomes ready
[   17.938753] br0: port 1(eno1) entered forwarding state
[   17.938762] br0: port 1(eno1) entered forwarding state
[   17.938834] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
[   29.763514] FS-Cache: Loaded
[   29.917680] FS-Cache: Netfs 'nfs' registered for caching
[   29.936739] Key type dns_resolver registered
[   30.637482] NFS: Registering the id_resolver key type
[   30.637502] Key type id_resolver registered
[   30.637504] Key type id_legacy registered
[   31.286444] ip6_tables: (C) 2000-2006 Netfilter Core Team
[   31.403005] Ebtables v2.0 registered
[   31.630354] tun: Universal TUN/TAP device driver, 1.6
[   31.630358] tun: (C) 1999-2004 Max Krasnyansky 
[   31.630824] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   31.677764] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   31.677855] device virbr0-nic entered promiscuous mode
[   31.677898] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   31.904892] nf_conntrack version 0.5.0 (65536 buckets, 262144 max)
[   32.087094] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.087196] virbr0: port 1(virbr0-nic) entered listening state
[   32.087205] virbr0: port 1(virbr0-nic) entered listening state
[   32.093676] br0: set_features() failed (-1); wanted 0x00801fd978a9, left 
0x00801fff78e9
[   32.093786] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.093872] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   32.093966] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.094051] virbr0-nic: set_features() failed (-1); wanted 
0x008048c1, left 0x0080001b48c9
[   32.094132] virbr0: set_features() failed (-1); wanted 0x00801fdb78c9, 
left 0x00801fff78e9
[   32.124341] virbr0: port 1(virbr0-nic) entered disabled state

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-13 Thread Nikolay Aleksandrov
On 11/13/2015 11:29 AM, Jiri Pirko wrote:
> Fri, Nov 13, 2015 at 01:26:18AM CET, f.faine...@gmail.com wrote:
>> On 04/11/15 18:56, David Miller wrote:
 Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
 features down stack")
>>>  ...
 Reported-by: Nikolay Aleksandrov 
 Signed-off-by: Jarod Wilson 
 ---
 v2: Based on suggestions from Alex, and with not changing err to ret, this
 patch actually becomes quite minimal and doesn't ugly up the code much.
>>>
>>> Applied, thanks.
>>
>> This causes some warnings to be displayed for DSA stacked devices:
>>
>> [1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
>> 4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
>> [1.283181] libphy: dsa slave smi: probed
>> [1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
>> patch: 3
>> [1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
>> attached PHY at address 5 [Broadcom BCM7445]
>> [1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>> [1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
>> (uninitialized): attached PHY at address 0 [Generic PHY]
>> [1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>> [1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
>> (uninitialized): attached PHY at address 1 [Generic PHY]
>> [1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>> [1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
>> attached PHY at address 2 [Generic PHY]
>> [1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
>> failed (-1); wanted 0x4020, left 0x4820
>>
>> DSA slave network devices are not associated with their master network
>> device using the typical lower/upper netdev helpers.
>>
>> I do not have a good fix to come up with yet, but if you see something
>> obvious with net/dsa/slave.c, feel free to send patches for testing, I
>> can boot net-next on this platform.
> 
> I'm having similar issues with bridge, with linus's git now:
> 
[snip]

Hmm, I think it's because the bridge and dsa/slave don't have ndo_set_features()
so err is left as -1 and thus an error is reported which isn't actually true.
Before in this case the features would just get set, so could you please try
the following patch ?


diff --git a/net/core/dev.c b/net/core/dev.c
index ab9b8d0d115e..4a1d198dbbff 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6426,6 +6426,8 @@ int __netdev_update_features(struct net_device *dev)
 
if (dev->netdev_ops->ndo_set_features)
err = dev->netdev_ops->ndo_set_features(dev, features);
+   else
+   err = 0;
 
if (unlikely(err < 0)) {
netdev_err(dev,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-12 Thread Florian Fainelli
On 04/11/15 18:56, David Miller wrote:
>> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
>> features down stack")
>  ...
>> Reported-by: Nikolay Aleksandrov 
>> Signed-off-by: Jarod Wilson 
>> ---
>> v2: Based on suggestions from Alex, and with not changing err to ret, this
>> patch actually becomes quite minimal and doesn't ugly up the code much.
> 
> Applied, thanks.

This causes some warnings to be displayed for DSA stacked devices:

[1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
[1.283181] libphy: dsa slave smi: probed
[1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
patch: 3
[1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
attached PHY at address 5 [Broadcom BCM7445]
[1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
(uninitialized): attached PHY at address 0 [Generic PHY]
[1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
(uninitialized): attached PHY at address 1 [Generic PHY]
[1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
attached PHY at address 2 [Generic PHY]
[1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
failed (-1); wanted 0x4020, left 0x4820

DSA slave network devices are not associated with their master network
device using the typical lower/upper netdev helpers.

I do not have a good fix to come up with yet, but if you see something
obvious with net/dsa/slave.c, feel free to send patches for testing, I
can boot net-next on this platform.
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-12 Thread Florian Fainelli
On 04/11/15 18:56, David Miller wrote:
>> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev 
>> features down stack")
>  ...
>> Reported-by: Nikolay Aleksandrov 
>> Signed-off-by: Jarod Wilson 
>> ---
>> v2: Based on suggestions from Alex, and with not changing err to ret, this
>> patch actually becomes quite minimal and doesn't ugly up the code much.
> 
> Applied, thanks.

This causes some warnings to be displayed for DSA stacked devices:

[1.272297] brcm-sf2 f0b0.ethernet_switch: Starfighter 2 top:
4.00, core: 2.00 base: 0xf0c8, IRQs: 68, 69
[1.283181] libphy: dsa slave smi: probed
[1.344088] f0b403c0.mdio:05: Broadcom BCM7445 PHY revision: 0xd0,
patch: 3
[1.658917] brcm-sf2 f0b0.ethernet_switch gphy (uninitialized):
attached PHY at address 5 [Broadcom BCM7445]
[1.669414] brcm-sf2 f0b0.ethernet_switch gphy: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.734202] brcm-sf2 f0b0.ethernet_switch rgmii_1
(uninitialized): attached PHY at address 0 [Generic PHY]
[1.744486] brcm-sf2 f0b0.ethernet_switch rgmii_1: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.809091] brcm-sf2 f0b0.ethernet_switch rgmii_2
(uninitialized): attached PHY at address 1 [Generic PHY]
[1.819364] brcm-sf2 f0b0.ethernet_switch rgmii_2: set_features()
failed (-1); wanted 0x4020, left 0x4820
[1.884090] brcm-sf2 f0b0.ethernet_switch moca (uninitialized):
attached PHY at address 2 [Generic PHY]
[1.894109] brcm-sf2 f0b0.ethernet_switch moca: set_features()
failed (-1); wanted 0x4020, left 0x4820

DSA slave network devices are not associated with their master network
device using the typical lower/upper netdev helpers.

I do not have a good fix to come up with yet, but if you see something
obvious with net/dsa/slave.c, feel free to send patches for testing, I
can boot net-next on this platform.
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-04 Thread David Miller
From: Jarod Wilson 
Date: Tue,  3 Nov 2015 23:09:32 -0500

> With moving netdev_sync_lower_features() after the .ndo_set_features
> calls, I neglected to verify that devices added *after* a flag had been
> disabled on an upper device were properly added with that flag disabled as
> well. This currently happens, because we exit __netdev_update_features()
> when we see dev->features == features for the upper dev. We can retain the
> optimization of leaving without calling .ndo_set_features with a bit of
> tweaking and a goto here.
> 
> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features 
> down stack")
 ...
> Reported-by: Nikolay Aleksandrov 
> Signed-off-by: Jarod Wilson 
> ---
> v2: Based on suggestions from Alex, and with not changing err to ret, this
> patch actually becomes quite minimal and doesn't ugly up the code much.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-04 Thread David Miller
From: Jarod Wilson 
Date: Tue,  3 Nov 2015 23:09:32 -0500

> With moving netdev_sync_lower_features() after the .ndo_set_features
> calls, I neglected to verify that devices added *after* a flag had been
> disabled on an upper device were properly added with that flag disabled as
> well. This currently happens, because we exit __netdev_update_features()
> when we see dev->features == features for the upper dev. We can retain the
> optimization of leaving without calling .ndo_set_features with a bit of
> tweaking and a goto here.
> 
> Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features 
> down stack")
 ...
> Reported-by: Nikolay Aleksandrov 
> Signed-off-by: Jarod Wilson 
> ---
> v2: Based on suggestions from Alex, and with not changing err to ret, this
> patch actually becomes quite minimal and doesn't ugly up the code much.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-03 Thread Jarod Wilson
With moving netdev_sync_lower_features() after the .ndo_set_features
calls, I neglected to verify that devices added *after* a flag had been
disabled on an upper device were properly added with that flag disabled as
well. This currently happens, because we exit __netdev_update_features()
when we see dev->features == features for the upper dev. We can retain the
optimization of leaving without calling .ndo_set_features with a bit of
tweaking and a goto here.

Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features 
down stack")
CC: "David S. Miller" 
CC: Eric Dumazet 
CC: Jay Vosburgh 
CC: Veaceslav Falico 
CC: Andy Gospodarek 
CC: Jiri Pirko 
CC: Nikolay Aleksandrov 
CC: Michal Kubecek 
CC: Alexander Duyck 
CC: net...@vger.kernel.org
Reported-by: Nikolay Aleksandrov 
Signed-off-by: Jarod Wilson 
---
v2: Based on suggestions from Alex, and with not changing err to ret, this
patch actually becomes quite minimal and doesn't ugly up the code much.

 net/core/dev.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8ce3f74..ab9b8d0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6402,7 +6402,7 @@ int __netdev_update_features(struct net_device *dev)
struct net_device *upper, *lower;
netdev_features_t features;
struct list_head *iter;
-   int err = 0;
+   int err = -1;
 
ASSERT_RTNL();
 
@@ -6419,7 +6419,7 @@ int __netdev_update_features(struct net_device *dev)
features = netdev_sync_upper_features(dev, upper, features);
 
if (dev->features == features)
-   return 0;
+   goto sync_lower;
 
netdev_dbg(dev, "Features changed: %pNF -> %pNF\n",
>features, );
@@ -6434,6 +6434,7 @@ int __netdev_update_features(struct net_device *dev)
return -1;
}
 
+sync_lower:
/* some features must be disabled on lower devices when disabled
 * on an upper device (think: bonding master or bridge)
 */
@@ -6443,7 +6444,7 @@ int __netdev_update_features(struct net_device *dev)
if (!err)
dev->features = features;
 
-   return 1;
+   return err < 0 ? 0 : 1;
 }
 
 /**
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 net-next] net/core: ensure features get disabled on new lower devs

2015-11-03 Thread Jarod Wilson
With moving netdev_sync_lower_features() after the .ndo_set_features
calls, I neglected to verify that devices added *after* a flag had been
disabled on an upper device were properly added with that flag disabled as
well. This currently happens, because we exit __netdev_update_features()
when we see dev->features == features for the upper dev. We can retain the
optimization of leaving without calling .ndo_set_features with a bit of
tweaking and a goto here.

Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features 
down stack")
CC: "David S. Miller" 
CC: Eric Dumazet 
CC: Jay Vosburgh 
CC: Veaceslav Falico 
CC: Andy Gospodarek 
CC: Jiri Pirko 
CC: Nikolay Aleksandrov 
CC: Michal Kubecek 
CC: Alexander Duyck 
CC: net...@vger.kernel.org
Reported-by: Nikolay Aleksandrov 
Signed-off-by: Jarod Wilson 
---
v2: Based on suggestions from Alex, and with not changing err to ret, this
patch actually becomes quite minimal and doesn't ugly up the code much.

 net/core/dev.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8ce3f74..ab9b8d0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6402,7 +6402,7 @@ int __netdev_update_features(struct net_device *dev)
struct net_device *upper, *lower;
netdev_features_t features;
struct list_head *iter;
-   int err = 0;
+   int err = -1;
 
ASSERT_RTNL();
 
@@ -6419,7 +6419,7 @@ int __netdev_update_features(struct net_device *dev)
features = netdev_sync_upper_features(dev, upper, features);
 
if (dev->features == features)
-   return 0;
+   goto sync_lower;
 
netdev_dbg(dev, "Features changed: %pNF -> %pNF\n",
>features, );
@@ -6434,6 +6434,7 @@ int __netdev_update_features(struct net_device *dev)
return -1;
}
 
+sync_lower:
/* some features must be disabled on lower devices when disabled
 * on an upper device (think: bonding master or bridge)
 */
@@ -6443,7 +6444,7 @@ int __netdev_update_features(struct net_device *dev)
if (!err)
dev->features = features;
 
-   return 1;
+   return err < 0 ? 0 : 1;
 }
 
 /**
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/