netdev - Re: [RFC PATCH 5/9] net: dsa: refuse configuration in prepare phase of dsa_port_vlan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 19 Sep 2020 19:38:54 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org,
        davem@...emloft.net
Cc:     andrew@...n.ch, vivien.didelot@...il.com, idosch@...sch.org,
        jiri@...nulli.us, kurt.kanzenbach@...utronix.de, kuba@...nel.org
Subject: Re: [RFC PATCH 5/9] net: dsa: refuse configuration in prepare phase
 of dsa_port_vlan_filtering()



On 9/19/2020 6:47 PM, Vladimir Oltean wrote:
> The current logic beats me a little bit. The comment that "bridge skips
> -EOPNOTSUPP, so skip the prepare phase" was introduced in commit
> fb2dabad69f0 ("net: dsa: support VLAN filtering switchdev attr").
> 
> I'm not sure:
> (a) ok, the bridge skips -EOPNOTSUPP, but, so what, where are we
>      returning -EOPNOTSUPP?
> (b) even if we are, and I'm just not seeing it, what is the causality
>      relationship between the bridge skipping -EOPNOTSUPP and DSA
>      skipping the prepare phase, and just returning zero?
> 
> One thing is certain beyond doubt though, and that is that DSA currently
> refuses VLAN filtering from the "commit" phase instead of "prepare", and
> that this is not a good thing:
> 
> ip link add br0 type bridge
> ip link add br1 type bridge vlan_filtering 1
> ip link set swp2 master br0
> ip link set swp3 master br1
> [ 3790.379389] 001: sja1105 spi0.1: VLAN filtering is a global setting
> [ 3790.379399] 001: ------------[ cut here ]------------
> [ 3790.379403] 001: WARNING: CPU: 1 PID: 515 at net/switchdev/switchdev.c:157 switchdev_port_attr_set_now+0x9c/0xa4
> [ 3790.379420] 001: swp3: Commit of attribute (id=6) failed.
> [ 3790.379533] 001: [<c11ff588>] (switchdev_port_attr_set_now) from [<c11b62e4>] (nbp_vlan_init+0x84/0x148)
> [ 3790.379544] 001: [<c11b62e4>] (nbp_vlan_init) from [<c11a2ff0>] (br_add_if+0x514/0x670)
> [ 3790.379554] 001: [<c11a2ff0>] (br_add_if) from [<c1031b5c>] (do_setlink+0x38c/0xab0)
> [ 3790.379565] 001: [<c1031b5c>] (do_setlink) from [<c1036fe8>] (__rtnl_newlink+0x44c/0x748)
> [ 3790.379573] 001: [<c1036fe8>] (__rtnl_newlink) from [<c1037328>] (rtnl_newlink+0x44/0x60)
> [ 3790.379580] 001: [<c1037328>] (rtnl_newlink) from [<c10315fc>] (rtnetlink_rcv_msg+0x124/0x2f8)
> [ 3790.379590] 001: [<c10315fc>] (rtnetlink_rcv_msg) from [<c10926b8>] (netlink_rcv_skb+0xb8/0x110)
> [ 3790.379806] 001: ---[ end trace 0000000000000002 ]---
> [ 3790.379819] 001: sja1105 spi0.1 swp3: failed to initialize vlan filtering on this port
> 
> So move the current logic that may fail (except ds->ops->port_vlan_filtering,
> that is way harder) into the prepare stage of the switchdev transaction.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian