Message-ID: <2f23dc48-a042-4fcf-cc5b-11d247061239@gmail.com>
Date:   Fri, 1 Mar 2019 09:52:12 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Michal Vokáč <michal.vokac@...ft.com>,
        Heiner Kallweit <hkallweit1@...il.com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] net: dsa: Do not configure VLAN ID 0

On 3/1/19 2:34 AM, Michal Vokáč wrote:
> On 01. 03. 19 0:49, Florian Fainelli wrote:
>> Because we skip the prepare phase, we would not get a chance to have the
>> port_vlan_prepare() callback return -EOPNOTSUPP and tell us about that.
>> This causes problems with mv88e6xxx which specifically checks for VLAN
>> ID = 0. Turns out we do not actually need to program that VLAN ID since
>> it should be the default one for switches anyway.
>>
>> Reported-by: Heiner Kallweit <hkallweit1@...il.com>
>> Fixes: 061f6a505ac3 ("net: dsa: Add ndo_vlan_rx_{add, kill}_vid
>> implementation")
>> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
>> ---
>>   net/dsa/slave.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>> index 1808a2cd6872..ec54d579645a 100644
>> --- a/net/dsa/slave.c
>> +++ b/net/dsa/slave.c
>> @@ -996,6 +996,9 @@ static int dsa_slave_vlan_rx_add_vid(struct
>> net_device *dev, __be16 proto,
>>       struct bridge_vlan_info info;
>>       int ret;
>>   +    if (vid == 0)
>> +        return 0;
>> +
>>       /* Check for a possible bridge VLAN entry now since there is no
>>        * need to emulate the switchdev prepare + commit phase.
>>        */
>> @@ -1029,6 +1032,9 @@ static int dsa_slave_vlan_rx_kill_vid(struct
>> net_device *dev, __be16 proto,
>>       struct bridge_vlan_info info;
>>       int ret;
>>   +    if (vid == 0)
>> +        return 0;
>> +
>>       /* Check for a possible bridge VLAN entry now since there is no
>>        * need to emulate the switchdev prepare + commit phase.
>>        */
>>
> 
> Tested-by: Michal Vokáč <michal.vokac@...ft.com>
> 
> I am not sure what was the original problem as I could not find the
> thread where Heiner reported the issue but with the latest version
> next-20190228 I get this error with QCA8334 switch:

Thanks Michal. There is still a lingering issue though, because we
currently skip the prepare phase (we don't have a switchdev_trans
structure that would make sense here), we are not checking that the
underlying switch device driver does actually support
port_vlan_add/port_vlan_del, and qca8k currently does not. I will submit
a v2 patch that also takes care of that.

> 
> root@...raco:~# ifconfig eth0 up
> [   58.323015] Generic PHY fixed-0:00: attached PHY driver [Generic PHY]
> (mii_bus:phy_addr=fixed-0:00, irq=POLL)
> [   58.333960] fec 2188000.ethernet eth0: Link is Up - 1Gbps/Full - flow
> control off
> [   58.352092] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> 
> root@...raco:~# ifconfig eth1 up
> [   63.560167] qca8k 2188000.ethernet-1:00 eth1: configuring for phy/
> link mode
> [   63.568613] qca8k 2188000.ethernet-1:00 eth1: Link is Up - 1Gbps/Full
> - flow control rx/tx
> [   63.574751] 8021q: adding VLAN 0 to HW filter on device eth1
> [   63.587791] Unable to handle kernel NULL pointer dereference at
> virtual address 00000000
> [   63.595901] pgd = (ptrval)
> [   63.598930] [00000000] *pgd=78d90831, *pte=00000000, *ppte=00000000
> [   63.605234] Internal error: Oops: 80000007 [#1] PREEMPT SMP ARM
> [   63.611170] CPU: 1 PID: 375 Comm: ifconfig Not tainted
> 5.0.0-rc8-next-20190228-00001-g760ce972e969 #96
> [   63.620484] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [   63.627019] PC is at   (null)
> [   63.630007] LR is at dsa_switch_event+0x3ec/0x5a0
> [   63.634716] pc : [<00000000>]    lr : [<80a630b8>]    psr: 200e0013
> [   63.640987] sp : e9201b50  ip : 00000000  fp : e9201b7c
> [   63.646216] r10: e8a3ce1c  r9 : 00000000  r8 : 00000000
> [   63.651446] r7 : e9201bf8  r6 : e8a3c840  r5 : e8a3c884  r4 : e8a3c84c
> [   63.657976] r3 : 00000000  r2 : e9201bf8  r1 : 00000000  r0 : e8a3c840
> [   63.664508] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM 
> Segment none
> [   63.671648] Control: 10c5387d  Table: 7920804a  DAC: 00000051
> [   63.677401] Process ifconfig (pid: 375, stack limit = 0x(ptrval))
> [   63.683502] Stack: (0xe9201b50 to 0xe9202000)
> [   63.687867] 1b40:                                     80a62ccc
> ffffffff 00000000 e9201bc4
> [   63.696053] 1b60: 00000007 00000000 e8a3f000 00000001 e9201ba4
> e9201b80 8014ab0c 80a62cd8
> [   63.704238] 1b80: 8100a548 e8a3ca00 00000200 00000000 00000000
> e8a3f000 e9201bbc e9201ba8
> [   63.712424] 1ba0: 8014b15c 8014aac0 00000000 801543f4 e9201bec
> e9201bc0 80a6089c 8014b140
> [   63.720609] 1bc0: 8017dd48 e9201bf8 00000000 00000000 00000003
> ec6c7ddd 8100a548 e8a3ca00
> [   63.728794] 1be0: e9201c2c e9201bf0 80a61774 80a60830 8016e8a0
> 80286558 00000000 00000000
> [   63.736978] 1c00: 00000000 00000000 00000000 00000000 00000000
> ec6c7ddd 00004220 00010000
> [   63.745165] 1c20: e9201c4c e9201c30 80ae8b6c 80a616fc e8b778c0
> e8c44750 00000081 e8c44700
> [   63.753350] 1c40: e9201c8c e9201c50 80ae9374 80ae8b1c e9201c7c
> e9201c60 8017e9d4 e8a3f000
> [   63.761536] 1c60: e9201c84 ffffffee e8a3f000 00000001 00000000
> 8100a548 e9201c9c 00000000
> [   63.769721] 1c80: e9201cdc e9201c90 80aea10c 80ae9290 e8b59000
> 80a37860 81097500 e9201c9c
> [   63.777905] 1ca0: e9201c9c e9201cb0 80a37860 ec6c7ddd 00a377a8
> ffffffee 810a321c e9201d40
> [   63.786090] 1cc0: 00000001 00000000 00000000 00000000 e9201d04
> e9201ce0 8014ab0c 80ae9954
> [   63.794275] 1ce0: 00000001 e9201d40 00000001 e8a3f138 00000000
> 00000000 e9201d1c e9201d08
> [   63.802459] 1d00: 8014b15c 8014aac0 00000000 8086478c e9201d3c
> e9201d20 808647a4 8014b140
> [   63.810644] 1d20: 00000a20 00000000 8100a548 e8a3f000 e9201d6c
> e9201d40 8086ffc8 80864774
> [   63.818829] 1d40: e8a3f000 00000000 e8a3f000 ec6c7ddd 00000000
> e8a3f000 00000001 00000000
> [   63.827014] 1d60: e9201d94 e9201d70 808707ec 8086ff68 e9201e00
> 00000000 8100a548 e88bba0c
> [   63.835198] 1d80: 00008914 00000000 e9201dfc e9201d98 809abebc
> 808707a8 40070093 00000cc0
> [   63.843383] 1da0: e9201e00 ec6c7ddd ebf9b1e4 e88bba00 e8a3f000
> 00000014 00000cc0 00001043
> [   63.851568] 1dc0: 00000000 0007077c 000000ab ec6c7ddd 80154330
> 8100a548 7ee7ebc8 00008914
> [   63.859753] 1de0: 81097500 00000051 00000003 00000036 e9201e7c
> e9201e00 809ae278 809ab8f4
> [   63.867938] 1e00: 31687465 00000000 00000000 00000000 00001043
> 00000000 0007077c 000000ab
> [   63.876123] 1e20: 808678f4 8089e04c e8a3f000 808678ac e9201e54
> e9201e40 808678f4 8089e450
> [   63.884309] 1e40: e9201e8c 00000000 e9201e7c e9201e58 8089e450
> ec6c7ddd 00008914 8100a548
> [   63.892493] 1e60: 7ee7ebc8 81097500 e68ac020 00000003 e9201ed4
> e9201e80 808486f4 809ae070
> [   63.900678] 1e80: e8d8d300 00000001 e8da4080 31687465 00000000
> 00000000 00000000 00001002
> [   63.908863] 1ea0: 00000000 0007077c 000000ab ec6c7ddd 8100a548
> 7ee7ebc8 e8d8d300 00008914
> [   63.917049] 1ec0: e68ac020 00000003 e9201f6c e9201ed8 802b3d68
> 808484e4 8100a548 e8022b10
> [   63.925234] 1ee0: 80cad950 e68ac020 e9201f3c e9201ef8 802a0bd4
> 802a0a1c e8022b10 e67db440
> [   63.933419] 1f00: e9201f2c e9201f10 80154330 801752cc e8da2100
> 00000003 e8d8d300 802c0910
> [   63.941605] 1f20: e9201f4c e9201f30 802c0910 801543f4 00000000
> 8100a548 00000000 ec6c7ddd
> [   63.949790] 1f40: e9201f5c e8d8d300 e8d8d300 00000003 00008914
> 7ee7ebc8 e9200000 00000036
> [   63.957975] 1f60: e9201f94 e9201f70 802b45e4 802b3cb4 00065534
> 7ee7edac 7ee7eec7 00000036
> [   63.966160] 1f80: 80101204 e9200000 e9201fa4 e9201f98 802b4620
> 802b45ac 00000000 e9201fa8
> [   63.974345] 1fa0: 80101000 802b4614 00065534 7ee7edac 00000003
> 00008914 7ee7ebc8 00065534
> [   63.982530] 1fc0: 00065534 7ee7edac 7ee7eec7 00000036 7ee7edac
> 00000004 7ee7ebc8 00000003
> [   63.990714] 1fe0: 00083648 7ee7eb64 000183f5 76ea408c 20070010
> 00000003 00000000 00000000
> [   63.998892] Backtrace:
> [   64.001360] [<80a62ccc>] (dsa_switch_event) from [<8014ab0c>]
> (notifier_call_chain+0x58/0x94)
> [   64.009895]  r10:00000001 r9:e8a3f000 r8:00000000 r7:00000007
> r6:e9201bc4 r5:00000000
> [   64.017728]  r4:ffffffff r3:80a62ccc
> [   64.021315] [<8014aab4>] (notifier_call_chain) from [<8014b15c>]
> (raw_notifier_call_chain+0x28/0x30)
> [   64.030456]  r9:e8a3f000 r8:00000000 r7:00000000 r6:00000200
> r5:e8a3ca00 r4:8100a548
> [   64.038210] [<8014b134>] (raw_notifier_call_chain) from [<80a6089c>]
> (dsa_port_vlan_add+0x78/0xa8)
> [   64.047180] [<80a60824>] (dsa_port_vlan_add) from [<80a61774>]
> (dsa_slave_vlan_rx_add_vid+0x84/0xb0)
> [   64.056318]  r5:e8a3ca00 r4:8100a548
> [   64.059911] [<80a616f0>] (dsa_slave_vlan_rx_add_vid) from
> [<80ae8b6c>] (vlan_add_rx_filter_info+0x5c/0x84)
> [   64.069567]  r5:00010000 r4:00004220
> [   64.073155] [<80ae8b10>] (vlan_add_rx_filter_info) from [<80ae9374>]
> (vlan_vid_add+0xf0/0x1bc)
> [   64.081773]  r7:e8c44700 r6:00000081 r5:e8c44750 r4:e8b778c0
> [   64.087442] [<80ae9284>] (vlan_vid_add) from [<80aea10c>]
> (vlan_device_event+0x7c4/0x7c8)
> [   64.095626]  r10:00000000 r9:e9201c9c r8:8100a548 r7:00000000
> r6:00000001 r5:e8a3f000
> [   64.103459]  r4:ffffffee
> [   64.106001] [<80ae9948>] (vlan_device_event) from [<8014ab0c>]
> (notifier_call_chain+0x58/0x94)
> [   64.114620]  r10:00000000 r9:00000000 r8:00000000 r7:00000001
> r6:e9201d40 r5:810a321c
> [   64.122451]  r4:ffffffee
> [   64.124993] [<8014aab4>] (notifier_call_chain) from [<8014b15c>]
> (raw_notifier_call_chain+0x28/0x30)
> [   64.134133]  r9:00000000 r8:00000000 r7:e8a3f138 r6:00000001
> r5:e9201d40 r4:00000001
> [   64.141892] [<8014b134>] (raw_notifier_call_chain) from [<808647a4>]
> (call_netdevice_notifiers_info+0x3c/0x88)
> [   64.151904] [<80864768>] (call_netdevice_notifiers_info) from
> [<8086ffc8>] (__dev_notify_flags+0x6c/0xe4)
> [   64.161474]  r5:e8a3f000 r4:8100a548
> [   64.165058] [<8086ff5c>] (__dev_notify_flags) from [<808707ec>]
> (dev_change_flags+0x50/0x58)
> [   64.173502]  r6:00000000 r5:00000001 r4:e8a3f000
> [   64.178133] [<8087079c>] (dev_change_flags) from [<809abebc>]
> (devinet_ioctl+0x5d4/0x72c)
> [   64.186318]  r9:00000000 r8:00008914 r7:e88bba0c r6:8100a548
> r5:00000000 r4:e9201e00
> [   64.194071] [<809ab8e8>] (devinet_ioctl) from [<809ae278>]
> (inet_ioctl+0x214/0x3c0)
> [   64.201735]  r10:00000036 r9:00000003 r8:00000051 r7:81097500
> r6:00008914 r5:7ee7ebc8
> [   64.209567]  r4:8100a548
> [   64.212118] [<809ae064>] (inet_ioctl) from [<808486f4>]
> (sock_ioctl+0x21c/0x488)
> [   64.219521]  r9:00000003 r8:e68ac020 r7:81097500 r6:7ee7ebc8
> r5:8100a548 r4:00008914
> [   64.227276] [<808484d8>] (sock_ioctl) from [<802b3d68>]
> (do_vfs_ioctl+0xc0/0x8f8)
> [   64.234766]  r9:00000003 r8:e68ac020 r7:00008914 r6:e8d8d300
> r5:7ee7ebc8 r4:8100a548
> [   64.242517] [<802b3ca8>] (do_vfs_ioctl) from [<802b45e4>]
> (ksys_ioctl+0x44/0x68)
> [   64.249920]  r10:00000036 r9:e9200000 r8:7ee7ebc8 r7:00008914
> r6:00000003 r5:e8d8d300
> [   64.257752]  r4:e8d8d300
> [   64.260294] [<802b45a0>] (ksys_ioctl) from [<802b4620>]
> (sys_ioctl+0x18/0x1c)
> [   64.267437]  r9:e9200000 r8:80101204 r7:00000036 r6:7ee7eec7
> r5:7ee7edac r4:00065534
> [   64.275192] [<802b4608>] (sys_ioctl) from [<80101000>]
> (ret_fast_syscall+0x0/0x54)
> [   64.282765] Exception stack(0xe9201fa8 to 0xe9201ff0)
> [   64.287825] 1fa0:                   00065534 7ee7edac 00000003
> 00008914 7ee7ebc8 00065534
> [   64.296011] 1fc0: 00065534 7ee7edac 7ee7eec7 00000036 7ee7edac
> 00000004 7ee7ebc8 00000003
> [   64.304192] 1fe0: 00083648 7ee7eb64 000183f5 76ea408c
> [   64.309253] Code: bad PC value
> [   64.313351] ---[ end trace f18ee16759e4e878 ]---
> 
> I tracked it down to commit 061f6a505ac3 ("net: dsa: Add
> ndo_vlan_rx_{add, kill}_vid
> implementation") and this patch fixes it for me.
> 
> Thank you,
> Michal


-- 
Florian

Powered by blists - more mailing lists