[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <358a0759-e2be-52e7-eabd-928c678cb06d@cumulusnetworks.com>
Date: Tue, 2 Apr 2019 23:15:44 +0300
From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To: Mike Manning <mmanning@...tta.att-mail.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/4] vlan: do not transfer link state in vlan
bridge binding mode
On 02/04/2019 18:35, Mike Manning wrote:
> In vlan bridge binding mode, the link state is no longer transferred
> from the lower device. Instead it is set by the bridge module according
> to the state of bridge ports that are members of the vlan.
>
> Signed-off-by: Mike Manning <mmanning@...tta.att-mail.com>
> ---
> net/8021q/vlan.c | 18 ++++++++++++++----
> net/8021q/vlan_dev.c | 19 ++++++++++++-------
> 2 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index dc4411165e43..1f99678751df 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -75,6 +75,14 @@ static int vlan_group_prealloc_vid(struct vlan_group *vg,
> return 0;
> }
>
> +static void vlan_stacked_transfer_operstate(const struct net_device *rootdev,
> + struct net_device *dev,
> + struct vlan_dev_priv *vlan)
> +{
> + if (!(vlan->flags & VLAN_FLAG_BRIDGE_BINDING))
> + netif_stacked_transfer_operstate(rootdev, dev);
> +}
I think this may be problematic with STP since STP can set netif_carrier_off() to
the bridge device even with up ports (but not forwarding) and it will not be propagated
to the vlan
> +
> void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
> {
> struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> @@ -180,7 +188,7 @@ int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack)
> /* Account for reference in struct vlan_dev_priv */
> dev_hold(real_dev);
>
> - netif_stacked_transfer_operstate(real_dev, dev);
> + vlan_stacked_transfer_operstate(real_dev, dev, vlan);
> linkwatch_fire_event(dev); /* _MUST_ call rfc2863_policy() */
>
> /* So, got the sucker initialized, now lets place
> @@ -399,7 +407,8 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
> case NETDEV_CHANGE:
> /* Propagate real device state to vlan devices */
> vlan_group_for_each_dev(grp, i, vlandev)
> - netif_stacked_transfer_operstate(dev, vlandev);
> + vlan_stacked_transfer_operstate(dev, vlandev,
> + vlan_dev_priv(vlandev));
> break;
>
> case NETDEV_CHANGEADDR:
> @@ -446,7 +455,8 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
> dev_close_many(&close_list, false);
>
> list_for_each_entry_safe(vlandev, tmp, &close_list, close_list) {
> - netif_stacked_transfer_operstate(dev, vlandev);
> + vlan_stacked_transfer_operstate(dev, vlandev,
> + vlan_dev_priv(vlandev));
> list_del_init(&vlandev->close_list);
> }
> list_del(&close_list);
> @@ -463,7 +473,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
> if (!(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
> dev_change_flags(vlandev, flgs | IFF_UP,
> extack);
> - netif_stacked_transfer_operstate(dev, vlandev);
> + vlan_stacked_transfer_operstate(dev, vlandev, vlan);
> }
> break;
>
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 86b38bb87f9a..270df9f0dfea 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -297,7 +297,8 @@ static int vlan_dev_open(struct net_device *dev)
> if (vlan->flags & VLAN_FLAG_MVRP)
> vlan_mvrp_request_join(dev);
>
> - if (netif_carrier_ok(real_dev))
> + if (netif_carrier_ok(real_dev) &&
> + !(vlan->flags & VLAN_FLAG_BRIDGE_BINDING))
> netif_carrier_on(dev);
> return 0;
>
> @@ -327,7 +328,8 @@ static int vlan_dev_stop(struct net_device *dev)
> if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
> dev_uc_del(real_dev, dev->dev_addr);
>
> - netif_carrier_off(dev);
> + if (!(vlan->flags & VLAN_FLAG_BRIDGE_BINDING))
> + netif_carrier_off(dev);
> return 0;
> }
>
> @@ -549,7 +551,8 @@ static const struct net_device_ops vlan_netdev_ops;
>
> static int vlan_dev_init(struct net_device *dev)
> {
> - struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
> + struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> + struct net_device *real_dev = vlan->real_dev;
>
> netif_carrier_off(dev);
>
> @@ -560,6 +563,9 @@ static int vlan_dev_init(struct net_device *dev)
> (1<<__LINK_STATE_DORMANT))) |
> (1<<__LINK_STATE_PRESENT);
>
> + if (vlan->flags & VLAN_FLAG_BRIDGE_BINDING)
> + dev->state |= (1 << __LINK_STATE_NOCARRIER);
> +
> dev->hw_features = NETIF_F_HW_CSUM | NETIF_F_SG |
> NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE |
> NETIF_F_GSO_ENCAP_ALL |
> @@ -590,8 +596,7 @@ static int vlan_dev_init(struct net_device *dev)
> #endif
>
> dev->needed_headroom = real_dev->needed_headroom;
> - if (vlan_hw_offload_capable(real_dev->features,
> - vlan_dev_priv(dev)->vlan_proto)) {
> + if (vlan_hw_offload_capable(real_dev->features, vlan->vlan_proto)) {
> dev->header_ops = &vlan_passthru_header_ops;
> dev->hard_header_len = real_dev->hard_header_len;
> } else {
> @@ -605,8 +610,8 @@ static int vlan_dev_init(struct net_device *dev)
>
> vlan_dev_set_lockdep_class(dev, vlan_dev_get_lock_subclass(dev));
>
> - vlan_dev_priv(dev)->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
> - if (!vlan_dev_priv(dev)->vlan_pcpu_stats)
> + vlan->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
> + if (!vlan->vlan_pcpu_stats)
> return -ENOMEM;
>
> return 0;
>
Powered by blists - more mailing lists