[PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Tobias reports that after the blamed patch, VLAN objects being added to
a bridge device are being added to all slave ports instead (swp2, swp3).

ip link add br0 type bridge vlan_filtering 1
ip link set swp2 master br0
ip link set swp3 master br0
bridge vlan add dev br0 vid 100 self

This is because the fix was too broad: we made dsa_port_offloads_netdev
say "yes, I offload the br0 bridge" for all slave ports, but we didn't
add the checks whether the switchdev object was in fact meant for the
physical port or for the bridge itself. So we are reacting on events in
a way in which we shouldn't.

The reason why the fix was too broad is because the question itself,
"does this DSA port offload this netdev", was too broad in the first
place. The solution is to disambiguate the question and separate it into
two different functions, one to be called for each switchdev attribute /
object that has an orig_dev == net_bridge (dsa_port_offloads_bridge),
and the other for orig_dev == net_bridge_port (*_offloads_bridge_port).

In the case of VLAN objects on the bridge interface, this solves the
problem because we know that VLAN objects are per bridge port and not
per bridge. And when orig_dev is equal to the net_bridge, we offload it
as a bridge, but not as a bridge port; that's how we are able to skip
reacting on those events. Note that this is compatible with future plans
to have explicit offloading of VLAN objects on the bridge interface as a
bridge port (in DSA, this signifies that we should add that VLAN towards
the CPU port).

Fixes: 99b8202b179f ("net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored")
Reported-by: Tobias Waldekranz <tobias@waldekranz.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
This is the logical v2 of Tobias' patches from here:
https://patchwork.kernel.org/project/netdevbpf/cover/20210306002455.1582593-1-tobias@waldekranz.com/

 net/dsa/dsa_priv.h | 25 +++++++++++---------
 net/dsa/slave.c    | 59 +++++++++++++++++++++++++++++++++-------------
 2 files changed, 57 insertions(+), 27 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2eeaa42f2e08..9d4b0e9b1aa1 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -230,8 +230,8 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr);
 void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
 
-static inline bool dsa_port_offloads_netdev(struct dsa_port *dp,
-					    struct net_device *dev)
+static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
+						 struct net_device *dev)
 {
 	/* Switchdev offloading can be configured on: */
 
@@ -241,12 +241,6 @@ static inline bool dsa_port_offloads_netdev(struct dsa_port *dp,
 		 */
 		return true;
 
-	if (dp->bridge_dev == dev)
-		/* DSA ports connected to a bridge, and event was emitted
-		 * for the bridge.
-		 */
-		return true;
-
 	if (dp->lag_dev == dev)
 		/* DSA ports connected to a bridge via a LAG */
 		return true;
@@ -254,14 +248,23 @@ static inline bool dsa_port_offloads_netdev(struct dsa_port *dp,
 	return false;
 }
 
+static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
+					    struct net_device *bridge_dev)
+{
+	/* DSA ports connected to a bridge, and event was emitted
+	 * for the bridge.
+	 */
+	return dp->bridge_dev == bridge_dev;
+}
+
 /* Returns true if any port of this tree offloads the given net_device */
-static inline bool dsa_tree_offloads_netdev(struct dsa_switch_tree *dst,
-					    struct net_device *dev)
+static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
+						 struct net_device *dev)
 {
 	struct dsa_port *dp;
 
 	list_for_each_entry(dp, &dst->ports, list)
-		if (dsa_port_offloads_netdev(dp, dev))
+		if (dsa_port_offloads_bridge_port(dp, dev))
 			return true;
 
 	return false;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 491e3761b5f4..992fcab4b552 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -278,28 +278,43 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int ret;
 
-	if (!dsa_port_offloads_netdev(dp, attr->orig_dev))
-		return -EOPNOTSUPP;
-
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
+		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
 		ret = dsa_port_set_state(dp, attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
+		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
 		ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
 					      extack);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
+		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
 		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
+		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
 		ret = dsa_port_pre_bridge_flags(dp, attr->u.brport_flags,
 						extack);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
+		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
 		ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, extack);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_MROUTER:
+		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
 		ret = dsa_port_mrouter(dp->cpu_dp, attr->u.mrouter, extack);
 		break;
 	default:
@@ -341,9 +356,6 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	struct switchdev_obj_port_vlan vlan;
 	int err;
 
-	if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
-		return -EOPNOTSUPP;
-
 	if (dsa_port_skip_vlan_configuration(dp)) {
 		NL_SET_ERR_MSG_MOD(extack, "skipping configuration of VLAN");
 		return 0;
@@ -391,27 +403,36 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
-		if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_port(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
+
 		err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_HOST_MDB:
+		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+			return -EOPNOTSUPP;
+
 		/* DSA can directly translate this to a normal MDB add,
 		 * but on the CPU port.
 		 */
 		err = dsa_port_mdb_add(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
+		if (!dsa_port_offloads_bridge_port(dp, obj->orig_dev))
+			return -EOPNOTSUPP;
+
 		err = dsa_slave_vlan_add(dev, obj, extack);
 		break;
 	case SWITCHDEV_OBJ_ID_MRP:
-		if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
+
 		err = dsa_port_mrp_add(dp, SWITCHDEV_OBJ_MRP(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
-		if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
+
 		err = dsa_port_mrp_add_ring_role(dp,
 						 SWITCHDEV_OBJ_RING_ROLE_MRP(obj));
 		break;
@@ -431,9 +452,6 @@ static int dsa_slave_vlan_del(struct net_device *dev,
 	struct switchdev_obj_port_vlan *vlan;
 	int err;
 
-	if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
-		return -EOPNOTSUPP;
-
 	if (dsa_port_skip_vlan_configuration(dp))
 		return 0;
 
@@ -459,27 +477,36 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
 
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
-		if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_port(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
+
 		err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_HOST_MDB:
+		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+			return -EOPNOTSUPP;
+
 		/* DSA can directly translate this to a normal MDB add,
 		 * but on the CPU port.
 		 */
 		err = dsa_port_mdb_del(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
+		if (!dsa_port_offloads_bridge_port(dp, obj->orig_dev))
+			return -EOPNOTSUPP;
+
 		err = dsa_slave_vlan_del(dev, obj);
 		break;
 	case SWITCHDEV_OBJ_ID_MRP:
-		if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
+
 		err = dsa_port_mrp_del(dp, SWITCHDEV_OBJ_MRP(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
-		if (!dsa_port_offloads_netdev(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
+
 		err = dsa_port_mrp_del_ring_role(dp,
 						 SWITCHDEV_OBJ_RING_ROLE_MRP(obj));
 		break;
@@ -2298,7 +2325,7 @@ static int dsa_slave_switchdev_event(struct notifier_block *unused,
 			 * other ports bridged with the LAG should be able to
 			 * autonomously forward towards it.
 			 */
-			if (dsa_tree_offloads_netdev(dp->ds->dst, dev))
+			if (dsa_tree_offloads_bridge_port(dp->ds->dst, dev))
 				return NOTIFY_DONE;
 		}
 
-- 
2.25.1

Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Tobias Waldekranz]

On Sun, Mar 07, 2021 at 12:21, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Tobias reports that after the blamed patch, VLAN objects being added to
> a bridge device are being added to all slave ports instead (swp2, swp3).
>
> ip link add br0 type bridge vlan_filtering 1
> ip link set swp2 master br0
> ip link set swp3 master br0
> bridge vlan add dev br0 vid 100 self
>
> This is because the fix was too broad: we made dsa_port_offloads_netdev
> say "yes, I offload the br0 bridge" for all slave ports, but we didn't
> add the checks whether the switchdev object was in fact meant for the
> physical port or for the bridge itself. So we are reacting on events in
> a way in which we shouldn't.
>
> The reason why the fix was too broad is because the question itself,
> "does this DSA port offload this netdev", was too broad in the first
> place. The solution is to disambiguate the question and separate it into
> two different functions, one to be called for each switchdev attribute /
> object that has an orig_dev == net_bridge (dsa_port_offloads_bridge),
> and the other for orig_dev == net_bridge_port (*_offloads_bridge_port).
>
> In the case of VLAN objects on the bridge interface, this solves the
> problem because we know that VLAN objects are per bridge port and not
> per bridge. And when orig_dev is equal to the net_bridge, we offload it
> as a bridge, but not as a bridge port; that's how we are able to skip
> reacting on those events. Note that this is compatible with future plans
> to have explicit offloading of VLAN objects on the bridge interface as a
> bridge port (in DSA, this signifies that we should add that VLAN towards
> the CPU port).
>
> Fixes: 99b8202b179f ("net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored")
> Reported-by: Tobias Waldekranz <tobias@waldekranz.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Please wait before applying.

I need to do some more testing later (possibly tomorrow). But I am
pretty sure that this patch does not work with the (admittedly somewhat
exotic) combination of:

- Non-offloaded LAG
- Bridge with VLAN filtering enabled.

When adding the LAG to the bridge, I get an error because mv88e6xxx
tries to add VLAN 1 to the ports (which it should not do as the LAG is
not offloaded).

Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Vladimir Oltean]

On Sun, Mar 07, 2021 at 04:17:14PM +0100, Tobias Waldekranz wrote:
> Please wait before applying.
> 
> I need to do some more testing later (possibly tomorrow). But I am
> pretty sure that this patch does not work with the (admittedly somewhat
> exotic) combination of:
> 
> - Non-offloaded LAG
> - Bridge with VLAN filtering enabled.
> 
> When adding the LAG to the bridge, I get an error because mv88e6xxx
> tries to add VLAN 1 to the ports (which it should not do as the LAG is
> not offloaded).

Weird, how are you testing, and why does it attempt to add VLAN 1? Is it
the mv88e6xxx driver itself that does this? Where from?

The following is my test procedure:

cat ./test_bond_no_offload.sh
#!/bin/bash

ip link del bond0
for eth in swp0 swp1 swp2; do ip link set $eth down; done
ip link add bond0 type bond mode broadcast
ip link add br0 type bridge vlan_filtering 1
ip link set swp0 master bond0
ip link set swp1 master bond0
ip link set swp2 master br0
ip link set bond0 master br0
for eth in swp0 swp1 swp2 bond0 br0; do ip link set $eth up; done

./test_bond_no_offload.sh
[   27.004206] bond0 (unregistering): Released all slaves
[   27.068440] mscc_felix 0000:00:00.5 swp0: configuring for inband/qsgmii link mode
[   27.077811] 8021q: adding VLAN 0 to HW filter on device swp0
[   27.083728] bond0: (slave swp0): Enslaving as an active interface with an up link
Warning: dsa_core: Offloading not supported.
[   27.095035] mscc_felix 0000:00:00.5 swp1: configuring for inband/qsgmii link mode
[   27.104073] 8021q: adding VLAN 0 to HW filter on device swp1
[   27.109948] bond0: (slave swp1): Enslaving as an active interface with an up link
Warning: dsa_core: Offloading not supported.
[   27.120214] br0: port 1(swp2) entered blocking state
[   27.125407] br0: port 1(swp2) entered disabled state
[   27.131738] mscc_felix 0000:00:00.5: dsa_port_vlan_filtering: port 2 vlan_filtering 1
[   27.139625] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 1
[   27.149223] br0: port 2(bond0) entered blocking state
[   27.154341] br0: port 2(bond0) entered disabled state
[   27.159600] device bond0 entered promiscuous mode
[   27.164340] device swp0 entered promiscuous mode
[   27.169028] device swp1 entered promiscuous mode
[   27.173718] device swp2 entered promiscuous mode
[   27.187698] mscc_felix 0000:00:00.5 swp2: configuring for inband/qsgmii link mode
[   27.196312] 8021q: adding VLAN 0 to HW filter on device swp2
[   27.207605] 8021q: adding VLAN 0 to HW filter on device bond0
[   28.060872] IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
[   28.067323] br0: port 2(bond0) entered blocking state
[   28.072406] br0: port 2(bond0) entered forwarding state
[   28.077751] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
# bridge link
8: swp2@eth1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 master br0 state disabled priority 32 cost 100
10: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 100
# bridge vlan add dev bond0 vid 100
# bridge vlan add dev swp2 vid 100
[   48.669422] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 100
# bridge vlan add dev br0 vid 100 self

Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Tobias Waldekranz]

On Sun, Mar 07, 2021 at 17:48, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Sun, Mar 07, 2021 at 04:17:14PM +0100, Tobias Waldekranz wrote:
>> Please wait before applying.
>> 
>> I need to do some more testing later (possibly tomorrow). But I am
>> pretty sure that this patch does not work with the (admittedly somewhat
>> exotic) combination of:
>> 
>> - Non-offloaded LAG
>> - Bridge with VLAN filtering enabled.
>> 
>> When adding the LAG to the bridge, I get an error because mv88e6xxx
>> tries to add VLAN 1 to the ports (which it should not do as the LAG is
>> not offloaded).
>
> Weird, how are you testing, and why does it attempt to add VLAN 1? Is it
> the mv88e6xxx driver itself that does this? Where from?
>
> The following is my test procedure:
>
> cat ./test_bond_no_offload.sh
> #!/bin/bash
>
> ip link del bond0
> for eth in swp0 swp1 swp2; do ip link set $eth down; done
> ip link add bond0 type bond mode broadcast
> ip link add br0 type bridge vlan_filtering 1
> ip link set swp0 master bond0
> ip link set swp1 master bond0
> ip link set swp2 master br0
> ip link set bond0 master br0
> for eth in swp0 swp1 swp2 bond0 br0; do ip link set $eth up; done
>
> ./test_bond_no_offload.sh
> [   27.004206] bond0 (unregistering): Released all slaves
> [   27.068440] mscc_felix 0000:00:00.5 swp0: configuring for inband/qsgmii link mode
> [   27.077811] 8021q: adding VLAN 0 to HW filter on device swp0
> [   27.083728] bond0: (slave swp0): Enslaving as an active interface with an up link
> Warning: dsa_core: Offloading not supported.
> [   27.095035] mscc_felix 0000:00:00.5 swp1: configuring for inband/qsgmii link mode
> [   27.104073] 8021q: adding VLAN 0 to HW filter on device swp1
> [   27.109948] bond0: (slave swp1): Enslaving as an active interface with an up link
> Warning: dsa_core: Offloading not supported.
> [   27.120214] br0: port 1(swp2) entered blocking state
> [   27.125407] br0: port 1(swp2) entered disabled state
> [   27.131738] mscc_felix 0000:00:00.5: dsa_port_vlan_filtering: port 2 vlan_filtering 1
> [   27.139625] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 1
> [   27.149223] br0: port 2(bond0) entered blocking state
> [   27.154341] br0: port 2(bond0) entered disabled state
> [   27.159600] device bond0 entered promiscuous mode
> [   27.164340] device swp0 entered promiscuous mode
> [   27.169028] device swp1 entered promiscuous mode
> [   27.173718] device swp2 entered promiscuous mode
> [   27.187698] mscc_felix 0000:00:00.5 swp2: configuring for inband/qsgmii link mode
> [   27.196312] 8021q: adding VLAN 0 to HW filter on device swp2
> [   27.207605] 8021q: adding VLAN 0 to HW filter on device bond0
> [   28.060872] IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
> [   28.067323] br0: port 2(bond0) entered blocking state
> [   28.072406] br0: port 2(bond0) entered forwarding state
> [   28.077751] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
> # bridge link
> 8: swp2@eth1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 master br0 state disabled priority 32 cost 100
> 10: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 100
> # bridge vlan add dev bond0 vid 100
> # bridge vlan add dev swp2 vid 100
> [   48.669422] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 100
> # bridge vlan add dev br0 vid 100 self

I ran the same test on my box (s/swp/eth/g just because that is what the
ports are called on my board):

root@envoy:~# dmesg -c
root@envoy:~# ./test_bond_no_offload.sh
Warning: dsa_core: Offloading not supported.
Warning: dsa_core: Offloading not supported.
RTNETLINK answers: Operation not supported
root@envoy:~# dmesg -c
[   40.392113] device eth3 left promiscuous mode
[   40.392233] br0: port 1(eth3) entered disabled state
[   40.468035] bond0 (unregistering): (slave eth1): Releasing backup interface
[   40.480821] device eth1 left promiscuous mode
[   40.487626] bond0 (unregistering): (slave eth2): Releasing backup interface
[   40.508856] device eth2 left promiscuous mode
[   40.508870] device chan0 left promiscuous mode
[   40.515602] bond0 (unregistering): Released all slaves
[   40.571520] mv88e6085 30be0000.ethernet-1:04 eth1: configuring for inband/2500base-x link mode
[   40.574803] 8021q: adding VLAN 0 to HW filter on device eth1       
[   40.576595] bond0: (slave eth1): Enslaving as an active interface with an up link    
[   40.583908] mv88e6085 30be0000.ethernet-1:04 eth2: configuring for inband/sgmii link mode
[   40.587225] 8021q: adding VLAN 0 to HW filter on device eth2 
[   40.589014] bond0: (slave eth2): Enslaving as an active interface with an up link
[   40.591622] br0: port 1(eth3) entered blocking state
[   40.591642] br0: port 1(eth3) entered disabled state
[   40.602894] br0: port 2(bond0) entered blocking state
[   40.602931] br0: port 2(bond0) entered disabled state
[   40.603172] device bond0 entered promiscuous mode
[   40.603179] device eth1 entered promiscuous mode
[   40.603183] device chan0 entered promiscuous mode
[   40.603229] device eth2 entered promiscuous mode
[   40.603284] device eth3 entered promiscuous mode
[   40.605250] mv88e6085 30be0000.ethernet-1:04: p10: hw VLAN 1 already used by port 8 in br0
[   40.605268] CPU: 0 PID: 1734 Comm: ip Not tainted 5.11.0 #197
[   40.605276] Hardware name: lynx-2510 (DT)
[   40.605281] Call trace: 
[   40.605284]  dump_backtrace+0x0/0x1b0
[   40.605301]  show_stack+0x20/0x70
[   40.605310]  dump_stack+0xd0/0x12c
[   40.605320]  mv88e6xxx_port_vlan_add+0x79c/0x810
[   40.605333]  dsa_switch_event+0x600/0xc70
[   40.605343]  raw_notifier_call_chain+0x5c/0x80
[   40.605351]  dsa_tree_notify+0x1c/0x40
[   40.605358]  dsa_port_vlan_add+0x58/0x80
[   40.605365]  dsa_slave_vlan_rx_add_vid+0x80/0x130
[   40.605372]  vlan_add_rx_filter_info+0x60/0x90
[   40.605380]  vlan_vid_add+0xf4/0x1b0
[   40.605386]  bond_vlan_rx_add_vid+0x78/0x110
[   40.605394]  vlan_add_rx_filter_info+0x60/0x90
[   40.605400]  vlan_vid_add+0xf4/0x1b0
[   40.605406]  __vlan_add+0x6c8/0x840
[   40.605415]  nbp_vlan_add+0xfc/0x180
[   40.605423]  nbp_vlan_init+0x140/0x190
[   40.605433]  br_add_if+0x558/0x740
[   40.605440]  br_add_slave+0x1c/0x30

(I added the dump_stack() just for demonstration purposes)

So we are coming in from everyones favorite ndo: ndo_vlan_add_rx_vid!

mv88e6xxx complains (rightly IMHO) that the hardware cannot offload VLAN
1 to two different bridges. It sees that eth3 is connected to br0, and
the current port is trying to add the same VID to a different
bridge. The second bridge in this case is in fact NULL.

One could argue that mv88e6xxx could just skip config if dp->bridge_dev
is not set. OTOH, the DSA layer manages all the intricacies of that in
all other scenarios.

Should we return early from the ndo if dp->bridge_dev is NULL? But then
why do we implement those ndos at all?

Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Tobias Waldekranz]

On Sun, Mar 07, 2021 at 21:02, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> On Sun, Mar 07, 2021 at 17:48, Vladimir Oltean <olteanv@gmail.com> wrote:
>> On Sun, Mar 07, 2021 at 04:17:14PM +0100, Tobias Waldekranz wrote:
>>> Please wait before applying.
>>> 
>>> I need to do some more testing later (possibly tomorrow). But I am
>>> pretty sure that this patch does not work with the (admittedly somewhat
>>> exotic) combination of:
>>> 
>>> - Non-offloaded LAG
>>> - Bridge with VLAN filtering enabled.
>>> 
>>> When adding the LAG to the bridge, I get an error because mv88e6xxx
>>> tries to add VLAN 1 to the ports (which it should not do as the LAG is
>>> not offloaded).
>>
>> Weird, how are you testing, and why does it attempt to add VLAN 1? Is it
>> the mv88e6xxx driver itself that does this? Where from?
>>
>> The following is my test procedure:
>>
>> cat ./test_bond_no_offload.sh
>> #!/bin/bash
>>
>> ip link del bond0
>> for eth in swp0 swp1 swp2; do ip link set $eth down; done
>> ip link add bond0 type bond mode broadcast
>> ip link add br0 type bridge vlan_filtering 1
>> ip link set swp0 master bond0
>> ip link set swp1 master bond0
>> ip link set swp2 master br0
>> ip link set bond0 master br0
>> for eth in swp0 swp1 swp2 bond0 br0; do ip link set $eth up; done
>>
>> ./test_bond_no_offload.sh
>> [   27.004206] bond0 (unregistering): Released all slaves
>> [   27.068440] mscc_felix 0000:00:00.5 swp0: configuring for inband/qsgmii link mode
>> [   27.077811] 8021q: adding VLAN 0 to HW filter on device swp0
>> [   27.083728] bond0: (slave swp0): Enslaving as an active interface with an up link
>> Warning: dsa_core: Offloading not supported.
>> [   27.095035] mscc_felix 0000:00:00.5 swp1: configuring for inband/qsgmii link mode
>> [   27.104073] 8021q: adding VLAN 0 to HW filter on device swp1
>> [   27.109948] bond0: (slave swp1): Enslaving as an active interface with an up link
>> Warning: dsa_core: Offloading not supported.
>> [   27.120214] br0: port 1(swp2) entered blocking state
>> [   27.125407] br0: port 1(swp2) entered disabled state
>> [   27.131738] mscc_felix 0000:00:00.5: dsa_port_vlan_filtering: port 2 vlan_filtering 1
>> [   27.139625] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 1
>> [   27.149223] br0: port 2(bond0) entered blocking state
>> [   27.154341] br0: port 2(bond0) entered disabled state
>> [   27.159600] device bond0 entered promiscuous mode
>> [   27.164340] device swp0 entered promiscuous mode
>> [   27.169028] device swp1 entered promiscuous mode
>> [   27.173718] device swp2 entered promiscuous mode
>> [   27.187698] mscc_felix 0000:00:00.5 swp2: configuring for inband/qsgmii link mode
>> [   27.196312] 8021q: adding VLAN 0 to HW filter on device swp2
>> [   27.207605] 8021q: adding VLAN 0 to HW filter on device bond0
>> [   28.060872] IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
>> [   28.067323] br0: port 2(bond0) entered blocking state
>> [   28.072406] br0: port 2(bond0) entered forwarding state
>> [   28.077751] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
>> # bridge link
>> 8: swp2@eth1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 master br0 state disabled priority 32 cost 100
>> 10: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 100
>> # bridge vlan add dev bond0 vid 100
>> # bridge vlan add dev swp2 vid 100
>> [   48.669422] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 100
>> # bridge vlan add dev br0 vid 100 self
>
> I ran the same test on my box (s/swp/eth/g just because that is what the
> ports are called on my board):
>
> root@envoy:~# dmesg -c
> root@envoy:~# ./test_bond_no_offload.sh
> Warning: dsa_core: Offloading not supported.
> Warning: dsa_core: Offloading not supported.
> RTNETLINK answers: Operation not supported
> root@envoy:~# dmesg -c
> [   40.392113] device eth3 left promiscuous mode
> [   40.392233] br0: port 1(eth3) entered disabled state
> [   40.468035] bond0 (unregistering): (slave eth1): Releasing backup interface
> [   40.480821] device eth1 left promiscuous mode
> [   40.487626] bond0 (unregistering): (slave eth2): Releasing backup interface
> [   40.508856] device eth2 left promiscuous mode
> [   40.508870] device chan0 left promiscuous mode
> [   40.515602] bond0 (unregistering): Released all slaves
> [   40.571520] mv88e6085 30be0000.ethernet-1:04 eth1: configuring for inband/2500base-x link mode
> [   40.574803] 8021q: adding VLAN 0 to HW filter on device eth1       
> [   40.576595] bond0: (slave eth1): Enslaving as an active interface with an up link    
> [   40.583908] mv88e6085 30be0000.ethernet-1:04 eth2: configuring for inband/sgmii link mode
> [   40.587225] 8021q: adding VLAN 0 to HW filter on device eth2 
> [   40.589014] bond0: (slave eth2): Enslaving as an active interface with an up link
> [   40.591622] br0: port 1(eth3) entered blocking state
> [   40.591642] br0: port 1(eth3) entered disabled state
> [   40.602894] br0: port 2(bond0) entered blocking state
> [   40.602931] br0: port 2(bond0) entered disabled state
> [   40.603172] device bond0 entered promiscuous mode
> [   40.603179] device eth1 entered promiscuous mode
> [   40.603183] device chan0 entered promiscuous mode
> [   40.603229] device eth2 entered promiscuous mode
> [   40.603284] device eth3 entered promiscuous mode
> [   40.605250] mv88e6085 30be0000.ethernet-1:04: p10: hw VLAN 1 already used by port 8 in br0
> [   40.605268] CPU: 0 PID: 1734 Comm: ip Not tainted 5.11.0 #197
> [   40.605276] Hardware name: lynx-2510 (DT)
> [   40.605281] Call trace: 
> [   40.605284]  dump_backtrace+0x0/0x1b0
> [   40.605301]  show_stack+0x20/0x70
> [   40.605310]  dump_stack+0xd0/0x12c
> [   40.605320]  mv88e6xxx_port_vlan_add+0x79c/0x810
> [   40.605333]  dsa_switch_event+0x600/0xc70
> [   40.605343]  raw_notifier_call_chain+0x5c/0x80
> [   40.605351]  dsa_tree_notify+0x1c/0x40
> [   40.605358]  dsa_port_vlan_add+0x58/0x80
> [   40.605365]  dsa_slave_vlan_rx_add_vid+0x80/0x130
> [   40.605372]  vlan_add_rx_filter_info+0x60/0x90
> [   40.605380]  vlan_vid_add+0xf4/0x1b0
> [   40.605386]  bond_vlan_rx_add_vid+0x78/0x110
> [   40.605394]  vlan_add_rx_filter_info+0x60/0x90
> [   40.605400]  vlan_vid_add+0xf4/0x1b0
> [   40.605406]  __vlan_add+0x6c8/0x840
> [   40.605415]  nbp_vlan_add+0xfc/0x180
> [   40.605423]  nbp_vlan_init+0x140/0x190
> [   40.605433]  br_add_if+0x558/0x740
> [   40.605440]  br_add_slave+0x1c/0x30
>
> (I added the dump_stack() just for demonstration purposes)
>
> So we are coming in from everyones favorite ndo: ndo_vlan_add_rx_vid!
>
> mv88e6xxx complains (rightly IMHO) that the hardware cannot offload VLAN
> 1 to two different bridges. It sees that eth3 is connected to br0, and
> the current port is trying to add the same VID to a different
> bridge. The second bridge in this case is in fact NULL.
>
> One could argue that mv88e6xxx could just skip config if dp->bridge_dev
> is not set. OTOH, the DSA layer manages all the intricacies of that in
> all other scenarios.
>
> Should we return early from the ndo if dp->bridge_dev is NULL? But then
> why do we implement those ndos at all?

If I understand Florian's original message (061f6a505ac3) correctly,
this was originally done to support HW that cannot control VLAN
filtering per port. I.e to support this setup:

  vlan1
    |
   br0     vlan2
  /   \      |
swp0  swp1  swp2

Where swp2 cannot be configured to ignore 1Q tags at the same time as
VLAN filtering is enabled on swp0 and swp1.

Florian, do I have that right?

If so, I think we can safely just `return 0` on these in mv88e6xxx (and
any other drivers where the HW can control this per port).

Adding a guard against configuring VLANs on unbridged user ports in
mv88e6xxx_port_vlan_add does seem to do the trick.

Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Florian Fainelli]

On 3/7/2021 2:49 PM, Tobias Waldekranz wrote:
> On Sun, Mar 07, 2021 at 21:02, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>> On Sun, Mar 07, 2021 at 17:48, Vladimir Oltean <olteanv@gmail.com> wrote:
>>> On Sun, Mar 07, 2021 at 04:17:14PM +0100, Tobias Waldekranz wrote:
>>>> Please wait before applying.
>>>>
>>>> I need to do some more testing later (possibly tomorrow). But I am
>>>> pretty sure that this patch does not work with the (admittedly somewhat
>>>> exotic) combination of:
>>>>
>>>> - Non-offloaded LAG
>>>> - Bridge with VLAN filtering enabled.
>>>>
>>>> When adding the LAG to the bridge, I get an error because mv88e6xxx
>>>> tries to add VLAN 1 to the ports (which it should not do as the LAG is
>>>> not offloaded).
>>>
>>> Weird, how are you testing, and why does it attempt to add VLAN 1? Is it
>>> the mv88e6xxx driver itself that does this? Where from?
>>>
>>> The following is my test procedure:
>>>
>>> cat ./test_bond_no_offload.sh
>>> #!/bin/bash
>>>
>>> ip link del bond0
>>> for eth in swp0 swp1 swp2; do ip link set $eth down; done
>>> ip link add bond0 type bond mode broadcast
>>> ip link add br0 type bridge vlan_filtering 1
>>> ip link set swp0 master bond0
>>> ip link set swp1 master bond0
>>> ip link set swp2 master br0
>>> ip link set bond0 master br0
>>> for eth in swp0 swp1 swp2 bond0 br0; do ip link set $eth up; done
>>>
>>> ./test_bond_no_offload.sh
>>> [   27.004206] bond0 (unregistering): Released all slaves
>>> [   27.068440] mscc_felix 0000:00:00.5 swp0: configuring for inband/qsgmii link mode
>>> [   27.077811] 8021q: adding VLAN 0 to HW filter on device swp0
>>> [   27.083728] bond0: (slave swp0): Enslaving as an active interface with an up link
>>> Warning: dsa_core: Offloading not supported.
>>> [   27.095035] mscc_felix 0000:00:00.5 swp1: configuring for inband/qsgmii link mode
>>> [   27.104073] 8021q: adding VLAN 0 to HW filter on device swp1
>>> [   27.109948] bond0: (slave swp1): Enslaving as an active interface with an up link
>>> Warning: dsa_core: Offloading not supported.
>>> [   27.120214] br0: port 1(swp2) entered blocking state
>>> [   27.125407] br0: port 1(swp2) entered disabled state
>>> [   27.131738] mscc_felix 0000:00:00.5: dsa_port_vlan_filtering: port 2 vlan_filtering 1
>>> [   27.139625] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 1
>>> [   27.149223] br0: port 2(bond0) entered blocking state
>>> [   27.154341] br0: port 2(bond0) entered disabled state
>>> [   27.159600] device bond0 entered promiscuous mode
>>> [   27.164340] device swp0 entered promiscuous mode
>>> [   27.169028] device swp1 entered promiscuous mode
>>> [   27.173718] device swp2 entered promiscuous mode
>>> [   27.187698] mscc_felix 0000:00:00.5 swp2: configuring for inband/qsgmii link mode
>>> [   27.196312] 8021q: adding VLAN 0 to HW filter on device swp2
>>> [   27.207605] 8021q: adding VLAN 0 to HW filter on device bond0
>>> [   28.060872] IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
>>> [   28.067323] br0: port 2(bond0) entered blocking state
>>> [   28.072406] br0: port 2(bond0) entered forwarding state
>>> [   28.077751] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
>>> # bridge link
>>> 8: swp2@eth1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 master br0 state disabled priority 32 cost 100
>>> 10: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 100
>>> # bridge vlan add dev bond0 vid 100
>>> # bridge vlan add dev swp2 vid 100
>>> [   48.669422] mscc_felix 0000:00:00.5 swp2: dsa_slave_vlan_add: vid 100
>>> # bridge vlan add dev br0 vid 100 self
>>
>> I ran the same test on my box (s/swp/eth/g just because that is what the
>> ports are called on my board):
>>
>> root@envoy:~# dmesg -c
>> root@envoy:~# ./test_bond_no_offload.sh
>> Warning: dsa_core: Offloading not supported.
>> Warning: dsa_core: Offloading not supported.
>> RTNETLINK answers: Operation not supported
>> root@envoy:~# dmesg -c
>> [   40.392113] device eth3 left promiscuous mode
>> [   40.392233] br0: port 1(eth3) entered disabled state
>> [   40.468035] bond0 (unregistering): (slave eth1): Releasing backup interface
>> [   40.480821] device eth1 left promiscuous mode
>> [   40.487626] bond0 (unregistering): (slave eth2): Releasing backup interface
>> [   40.508856] device eth2 left promiscuous mode
>> [   40.508870] device chan0 left promiscuous mode
>> [   40.515602] bond0 (unregistering): Released all slaves
>> [   40.571520] mv88e6085 30be0000.ethernet-1:04 eth1: configuring for inband/2500base-x link mode
>> [   40.574803] 8021q: adding VLAN 0 to HW filter on device eth1       
>> [   40.576595] bond0: (slave eth1): Enslaving as an active interface with an up link    
>> [   40.583908] mv88e6085 30be0000.ethernet-1:04 eth2: configuring for inband/sgmii link mode
>> [   40.587225] 8021q: adding VLAN 0 to HW filter on device eth2 
>> [   40.589014] bond0: (slave eth2): Enslaving as an active interface with an up link
>> [   40.591622] br0: port 1(eth3) entered blocking state
>> [   40.591642] br0: port 1(eth3) entered disabled state
>> [   40.602894] br0: port 2(bond0) entered blocking state
>> [   40.602931] br0: port 2(bond0) entered disabled state
>> [   40.603172] device bond0 entered promiscuous mode
>> [   40.603179] device eth1 entered promiscuous mode
>> [   40.603183] device chan0 entered promiscuous mode
>> [   40.603229] device eth2 entered promiscuous mode
>> [   40.603284] device eth3 entered promiscuous mode
>> [   40.605250] mv88e6085 30be0000.ethernet-1:04: p10: hw VLAN 1 already used by port 8 in br0
>> [   40.605268] CPU: 0 PID: 1734 Comm: ip Not tainted 5.11.0 #197
>> [   40.605276] Hardware name: lynx-2510 (DT)
>> [   40.605281] Call trace: 
>> [   40.605284]  dump_backtrace+0x0/0x1b0
>> [   40.605301]  show_stack+0x20/0x70
>> [   40.605310]  dump_stack+0xd0/0x12c
>> [   40.605320]  mv88e6xxx_port_vlan_add+0x79c/0x810
>> [   40.605333]  dsa_switch_event+0x600/0xc70
>> [   40.605343]  raw_notifier_call_chain+0x5c/0x80
>> [   40.605351]  dsa_tree_notify+0x1c/0x40
>> [   40.605358]  dsa_port_vlan_add+0x58/0x80
>> [   40.605365]  dsa_slave_vlan_rx_add_vid+0x80/0x130
>> [   40.605372]  vlan_add_rx_filter_info+0x60/0x90
>> [   40.605380]  vlan_vid_add+0xf4/0x1b0
>> [   40.605386]  bond_vlan_rx_add_vid+0x78/0x110
>> [   40.605394]  vlan_add_rx_filter_info+0x60/0x90
>> [   40.605400]  vlan_vid_add+0xf4/0x1b0
>> [   40.605406]  __vlan_add+0x6c8/0x840
>> [   40.605415]  nbp_vlan_add+0xfc/0x180
>> [   40.605423]  nbp_vlan_init+0x140/0x190
>> [   40.605433]  br_add_if+0x558/0x740
>> [   40.605440]  br_add_slave+0x1c/0x30
>>
>> (I added the dump_stack() just for demonstration purposes)
>>
>> So we are coming in from everyones favorite ndo: ndo_vlan_add_rx_vid!
>>
>> mv88e6xxx complains (rightly IMHO) that the hardware cannot offload VLAN
>> 1 to two different bridges. It sees that eth3 is connected to br0, and
>> the current port is trying to add the same VID to a different
>> bridge. The second bridge in this case is in fact NULL.
>>
>> One could argue that mv88e6xxx could just skip config if dp->bridge_dev
>> is not set. OTOH, the DSA layer manages all the intricacies of that in
>> all other scenarios.
>>
>> Should we return early from the ndo if dp->bridge_dev is NULL? But then
>> why do we implement those ndos at all?
> 
> If I understand Florian's original message (061f6a505ac3) correctly,
> this was originally done to support HW that cannot control VLAN
> filtering per port. I.e to support this setup:
> 
>   vlan1
>     |
>    br0     vlan2
>   /   \      |
> swp0  swp1  swp2
> 
> Where swp2 cannot be configured to ignore 1Q tags at the same time as
> VLAN filtering is enabled on swp0 and swp1.
> 
> Florian, do I have that right?

Yes, this change was intended to support switches that have global VLAN
filtering attributes (like b53, bcm_sf2) and where standalone ports that
get a VLAN upper require us to program an appropriate VLAN table entry
for these uppers to keep working.

> 
> If so, I think we can safely just `return 0` on these in mv88e6xxx (and
> any other drivers where the HW can control this per port).
> 
> Adding a guard against configuring VLANs on unbridged user ports in
> mv88e6xxx_port_vlan_add does seem to do the trick.
> 

OK.
-- 
Florian

Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[Tobias Waldekranz]

On Sun, Mar 07, 2021 at 12:21, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Tobias reports that after the blamed patch, VLAN objects being added to
> a bridge device are being added to all slave ports instead (swp2, swp3).
>
> ip link add br0 type bridge vlan_filtering 1
> ip link set swp2 master br0
> ip link set swp3 master br0
> bridge vlan add dev br0 vid 100 self
>
> This is because the fix was too broad: we made dsa_port_offloads_netdev
> say "yes, I offload the br0 bridge" for all slave ports, but we didn't
> add the checks whether the switchdev object was in fact meant for the
> physical port or for the bridge itself. So we are reacting on events in
> a way in which we shouldn't.
>
> The reason why the fix was too broad is because the question itself,
> "does this DSA port offload this netdev", was too broad in the first
> place. The solution is to disambiguate the question and separate it into
> two different functions, one to be called for each switchdev attribute /
> object that has an orig_dev == net_bridge (dsa_port_offloads_bridge),
> and the other for orig_dev == net_bridge_port (*_offloads_bridge_port).
>
> In the case of VLAN objects on the bridge interface, this solves the
> problem because we know that VLAN objects are per bridge port and not
> per bridge. And when orig_dev is equal to the net_bridge, we offload it
> as a bridge, but not as a bridge port; that's how we are able to skip
> reacting on those events. Note that this is compatible with future plans
> to have explicit offloading of VLAN objects on the bridge interface as a
> bridge port (in DSA, this signifies that we should add that VLAN towards
> the CPU port).
>
> Fixes: 99b8202b179f ("net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored")
> Reported-by: Tobias Waldekranz <tobias@waldekranz.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> This is the logical v2 of Tobias' patches from here:
> https://patchwork.kernel.org/project/netdevbpf/cover/20210306002455.1582593-1-tobias@waldekranz.com/

The issue related to the combo of software lagged ports in a VLAN
filtering bridge is a separate one, so I think this is fine the way it
is. I will address that issue in a separate patch.

Reviewed-by: Tobias Waldekranz <tobias@waldekranz.com>
Tested-by: Tobias Waldekranz <tobias@waldekranz.com>

Re: [PATCH v2 net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Sun,  7 Mar 2021 12:21:56 +0200 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Tobias reports that after the blamed patch, VLAN objects being added to
> a bridge device are being added to all slave ports instead (swp2, swp3).
> 
> ip link add br0 type bridge vlan_filtering 1
> ip link set swp2 master br0
> ip link set swp3 master br0
> bridge vlan add dev br0 vid 100 self
> 
> [...]

Here is the summary with links:
  - [v2,net] net: dsa: fix switchdev objects on bridge master mistakenly being applied on ports
    https://git.kernel.org/netdev/net/c/03cbb87054c1

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html