Message ID | 20190108164732.4024-9-idosch@mellanox.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | mlxsw: Various fixes | expand |
On 08/01/2019 18:48, Ido Schimmel wrote: > When adding / deleting VLANs to / from a bridge port, the bridge driver > first tries to propagate the information via switchdev and falls back to > the 8021q driver in case the underlying driver does not support > switchdev. This can result in a memory leak [1] when VXLAN and mlxsw > ports are enslaved to the bridge: > > $ ip link set dev vxlan0 master br0 > # No mlxsw ports are enslaved to 'br0', so mlxsw ignores the switchdev > # notification and the bridge driver adds the VLAN on 'vxlan0' via the > # 8021q driver > $ bridge vlan add vid 10 dev vxlan0 pvid untagged > # mlxsw port is enslaved to the bridge > $ ip link set dev swp1 master br0 > # mlxsw processes the switchdev notification and the 8021q driver is > # skipped > $ bridge vlan del vid 10 dev vxlan0 > > This results in 'struct vlan_info' and 'struct vlan_vid_info' being > leaked, as they were allocated by the 8021q driver during VLAN addition, > but never freed as the 8021q driver was skipped during deletion. > > Fix this by introducing a new VLAN private flag that indicates whether > the VLAN was added on the port by switchdev or the 8021q driver. If the > VLAN was added by the 8021q driver, then we make sure to delete it via > the 8021q driver as well. > > [1] > unreferenced object 0xffff88822d20b1e8 (size 256): > comm "bridge", pid 2532, jiffies 4295216998 (age 1188.830s) > hex dump (first 32 bytes): > e0 42 97 ce 81 88 ff ff 00 00 00 00 00 00 00 00 .B.............. > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330 > [<00000000e0178b02>] vlan_vid_add+0x661/0x920 > [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00 > [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90 > [<000000003535392c>] br_vlan_info+0x132/0x410 > [<00000000aedaa9dc>] br_afspec+0x75c/0x870 > [<00000000f5716133>] br_setlink+0x3dc/0x6d0 > [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30 > [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80 > [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0 > [<000000008be8d614>] rtnetlink_rcv+0x21/0x30 > [<000000009ab2ca25>] netlink_unicast+0x52f/0x740 > [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50 > [<000000005d1e2050>] sock_sendmsg+0xbe/0x120 > [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0 > [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270 > unreferenced object 0xffff888227454308 (size 32): > comm "bridge", pid 2532, jiffies 4295216998 (age 1188.882s) > hex dump (first 32 bytes): > 88 b2 20 2d 82 88 ff ff 88 b2 20 2d 82 88 ff ff .. -...... -.... > 81 00 0a 00 01 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330 > [<0000000018050631>] vlan_vid_add+0x3e6/0x920 > [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00 > [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90 > [<000000003535392c>] br_vlan_info+0x132/0x410 > [<00000000aedaa9dc>] br_afspec+0x75c/0x870 > [<00000000f5716133>] br_setlink+0x3dc/0x6d0 > [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30 > [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80 > [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0 > [<000000008be8d614>] rtnetlink_rcv+0x21/0x30 > [<000000009ab2ca25>] netlink_unicast+0x52f/0x740 > [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50 > [<000000005d1e2050>] sock_sendmsg+0xbe/0x120 > [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0 > [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270 > > Fixes: d70e42b22dd4 ("mlxsw: spectrum: Enable VxLAN enslavement to VLAN-aware bridges") > Signed-off-by: Ido Schimmel <idosch@mellanox.com> > Reviewed-by: Petr Machata <petrm@mellanox.com> > Cc: Roopa Prabhu <roopa@cumulusnetworks.com> > Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > Cc: bridge@lists.linux-foundation.org > --- > net/bridge/br_private.h | 1 + > net/bridge/br_vlan.c | 26 +++++++++++++------------- > 2 files changed, 14 insertions(+), 13 deletions(-) Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index d240b3e7919f..eabf8bf28a3f 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -107,6 +107,7 @@ struct br_tunnel_info { /* private vlan flags */ enum { BR_VLFLAG_PER_PORT_STATS = BIT(0), + BR_VLFLAG_ADDED_BY_SWITCHDEV = BIT(1), }; /** diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 4a2f31157ef5..96abf8feb9dc 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -80,16 +80,18 @@ static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags) } static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br, - u16 vid, u16 flags, struct netlink_ext_ack *extack) + struct net_bridge_vlan *v, u16 flags, + struct netlink_ext_ack *extack) { int err; /* Try switchdev op first. In case it is not supported, fallback to * 8021q add. */ - err = br_switchdev_port_vlan_add(dev, vid, flags, extack); + err = br_switchdev_port_vlan_add(dev, v->vid, flags, extack); if (err == -EOPNOTSUPP) - return vlan_vid_add(dev, br->vlan_proto, vid); + return vlan_vid_add(dev, br->vlan_proto, v->vid); + v->priv_flags |= BR_VLFLAG_ADDED_BY_SWITCHDEV; return err; } @@ -121,19 +123,17 @@ static void __vlan_del_list(struct net_bridge_vlan *v) } static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br, - u16 vid) + const struct net_bridge_vlan *v) { int err; /* Try switchdev op first. In case it is not supported, fallback to * 8021q del. */ - err = br_switchdev_port_vlan_del(dev, vid); - if (err == -EOPNOTSUPP) { - vlan_vid_del(dev, br->vlan_proto, vid); - return 0; - } - return err; + err = br_switchdev_port_vlan_del(dev, v->vid); + if (!(v->priv_flags & BR_VLFLAG_ADDED_BY_SWITCHDEV)) + vlan_vid_del(dev, br->vlan_proto, v->vid); + return err == -EOPNOTSUPP ? 0 : err; } /* Returns a master vlan, if it didn't exist it gets created. In all cases a @@ -242,7 +242,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags, * This ensures tagged traffic enters the bridge when * promiscuous mode is disabled by br_manage_promisc(). */ - err = __vlan_vid_add(dev, br, v->vid, flags, extack); + err = __vlan_vid_add(dev, br, v, flags, extack); if (err) goto out; @@ -305,7 +305,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags, out_filt: if (p) { - __vlan_vid_del(dev, br, v->vid); + __vlan_vid_del(dev, br, v); if (masterv) { if (v->stats && masterv->stats != v->stats) free_percpu(v->stats); @@ -338,7 +338,7 @@ static int __vlan_del(struct net_bridge_vlan *v) __vlan_delete_pvid(vg, v->vid); if (p) { - err = __vlan_vid_del(p->dev, p->br, v->vid); + err = __vlan_vid_del(p->dev, p->br, v); if (err) goto out; } else {