[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3f659e4c-fe27-bc0e-9f13-06f808b8d3b7@nvidia.com>
Date: Tue, 23 Mar 2021 16:06:07 +0200
From: Nikolay Aleksandrov <nikolay@...dia.com>
To: Vladimir Oltean <olteanv@...il.com>,
Jakub Kicinski <kuba@...nel.org>,
"David S. Miller" <davem@...emloft.net>
Cc: Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
Tobias Waldekranz <tobias@...dekranz.com>,
Claudiu Manoil <claudiu.manoil@....com>,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
Roopa Prabhu <roopa@...dia.com>, Jiri Pirko <jiri@...nulli.us>,
Ido Schimmel <idosch@...sch.org>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
UNGLinuxDriver@...rochip.com, Ivan Vecera <ivecera@...hat.com>,
linux-omap@...r.kernel.org,
Vladimir Oltean <vladimir.oltean@....com>
Subject: Re: [PATCH v4 net-next 05/11] net: bridge: add helper to replay VLANs
installed on port
On 23/03/2021 01:51, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@....com>
>
> Currently this simple setup with DSA:
>
> ip link add br0 type bridge vlan_filtering 1
> ip link add bond0 type bond
> ip link set bond0 master br0
> ip link set swp0 master bond0
>
> will not work because the bridge has created the PVID in br_add_if ->
> nbp_vlan_init, and it has notified switchdev of the existence of VLAN 1,
> but that was too early, since swp0 was not yet a lower of bond0, so it
> had no reason to act upon that notification.
>
> We need a helper in the bridge to replay the switchdev VLAN objects that
> were notified since the bridge port creation, because some of them may
> have been missed.
>
> As opposed to the br_mdb_replay function, the vg->vlan_list write side
> protection is offered by the rtnl_mutex which is sleepable, so we don't
> need to queue up the objects in atomic context, we can replay them right
> away.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
> include/linux/if_bridge.h | 10 ++++++
> net/bridge/br_vlan.c | 73 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 83 insertions(+)
>
Same comments about the const qualifiers as the other patches.
The code looks good to me otherwise.
Acked-by: Nikolay Aleksandrov <nikolay@...dia.com>
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index b564c4486a45..2cc35038a8ca 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -111,6 +111,8 @@ int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid);
> int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto);
> int br_vlan_get_info(const struct net_device *dev, u16 vid,
> struct bridge_vlan_info *p_vinfo);
> +int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
> + struct notifier_block *nb, struct netlink_ext_ack *extack);
> #else
> static inline bool br_vlan_enabled(const struct net_device *dev)
> {
> @@ -137,6 +139,14 @@ static inline int br_vlan_get_info(const struct net_device *dev, u16 vid,
> {
> return -EINVAL;
> }
> +
> +static inline int br_vlan_replay(struct net_device *br_dev,
> + struct net_device *dev,
> + struct notifier_block *nb,
> + struct netlink_ext_ack *extack)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
>
> #if IS_ENABLED(CONFIG_BRIDGE)
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 8829f621b8ec..ca8daccff217 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -1751,6 +1751,79 @@ void br_vlan_notify(const struct net_bridge *br,
> kfree_skb(skb);
> }
>
> +static int br_vlan_replay_one(struct notifier_block *nb,
> + struct net_device *dev,
> + struct switchdev_obj_port_vlan *vlan,
> + struct netlink_ext_ack *extack)
> +{
> + struct switchdev_notifier_port_obj_info obj_info = {
> + .info = {
> + .dev = dev,
> + .extack = extack,
> + },
> + .obj = &vlan->obj,
> + };
> + int err;
> +
> + err = nb->notifier_call(nb, SWITCHDEV_PORT_OBJ_ADD, &obj_info);
> + return notifier_to_errno(err);
> +}
> +
> +int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
> + struct notifier_block *nb, struct netlink_ext_ack *extack)
> +{
> + struct net_bridge_vlan_group *vg;
> + struct net_bridge_vlan *v;
> + struct net_bridge_port *p;
> + struct net_bridge *br;
> + int err = 0;
> + u16 pvid;
> +
> + ASSERT_RTNL();
> +
> + if (!netif_is_bridge_master(br_dev))
> + return -EINVAL;
> +
> + if (!netif_is_bridge_master(dev) && !netif_is_bridge_port(dev))
> + return -EINVAL;
> +
> + if (netif_is_bridge_master(dev)) {
> + br = netdev_priv(dev);
> + vg = br_vlan_group(br);
> + p = NULL;
> + } else {
> + p = br_port_get_rtnl(dev);
> + if (WARN_ON(!p))
> + return -EINVAL;
> + vg = nbp_vlan_group(p);
> + br = p->br;
> + }
> +
> + if (!vg)
> + return 0;
> +
> + pvid = br_get_pvid(vg);
> +
> + list_for_each_entry(v, &vg->vlan_list, vlist) {
> + struct switchdev_obj_port_vlan vlan = {
> + .obj.orig_dev = dev,
> + .obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
> + .flags = br_vlan_flags(v, pvid),
> + .vid = v->vid,
> + };
> +
> + if (!br_vlan_should_use(v))
> + continue;
> +
> + br_vlan_replay_one(nb, dev, &vlan, extack);
> + if (err)
> + return err;
> + }
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(br_vlan_replay);
> +
> /* check if v_curr can enter a range ending in range_end */
> bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
> const struct net_bridge_vlan *range_end)
>
Powered by blists - more mailing lists