| Message ID | 20190402153543.6277-3-mmanning@vyatta.att-mail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: support binding vlan dev link state to vlan member bridge ports | expand |
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@vyatta.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; >
On 02/04/2019 21:15, Nikolay Aleksandrov wrote: > 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@vyatta.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 > Thanks for catching this issue, I will need to work on a solution for this before submitting a v1 series (if at all possible). We do not have this concern with the specific topology for which these changes are required, but carrier changes due to STP must propagate even with this option.
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); +} + 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;
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@vyatta.att-mail.com> --- net/8021q/vlan.c | 18 ++++++++++++++---- net/8021q/vlan_dev.c | 19 ++++++++++++------- 2 files changed, 26 insertions(+), 11 deletions(-)