| Message ID | ab3fe6b46683a34bfe5088b94621335558727b92.1525702327.git.petrm@mellanox.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Ido Schimmel |
| Headers | show |
On Mon, May 07, 2018 at 04:29:39PM +0200, Petr Machata wrote: > When mirroring to a gretap or ip6gretap device, allow the underlay > packet path to include VLAN devices. The following configurations are > supported in underlay: > > - vlan over phys > - vlan-unaware bridge where the egress device is vlan over phys > - vlan over vlan-aware bridge where the egress device is phys > > Signed-off-by: Petr Machata <petrm@mellanox.com> > --- > drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c > index f6b3ca9..ac70a71 100644 > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c > @@ -176,9 +176,9 @@ mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev, > { > struct bridge_vlan_info vinfo; > struct net_device *edev; > - u16 pvid; > + u16 pvid = *p_vid; IIUC, in case we got here from a VLAN device configured on top of a VLAN-aware bridge, then 'p_vid' is actually the VID of the VLAN device. If so, then calling it PVID is incorrect. PVID is only used for untagged traffic. Also, I'm missing the check where packets with this VLAN are even allowed to ingress the bridge through the bridge device. Note that we don't get notified about changes to the bridge device's VLAN filter, so this is something that needs to be added as well. > > - if (WARN_ON(br_vlan_get_pvid(br_dev, &pvid))) > + if (!pvid && WARN_ON(br_vlan_get_pvid(br_dev, &pvid))) > return NULL; > if (!pvid) > return NULL; > @@ -208,13 +208,13 @@ mlxsw_sp_span_entry_bridge(const struct net_device *br_dev, > { > struct mlxsw_sp_bridge_port *bridge_port; > enum mlxsw_reg_spms_state spms_state; > + struct net_device *dev = NULL; > struct mlxsw_sp_port *port; > - struct net_device *dev; > u8 stp_state; > > if (br_vlan_enabled(br_dev)) > dev = mlxsw_sp_span_entry_bridge_8021q(br_dev, dmac, p_vid); > - else > + else if (!*p_vid) > dev = mlxsw_sp_span_entry_bridge_8021d(br_dev, dmac); > if (!dev) > return NULL; > @@ -261,12 +261,21 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev, > if (!l3edev || mlxsw_sp_span_dmac(tbl, &gw, l3edev, dmac)) > goto unoffloadable; > > + if (is_vlan_dev(l3edev)) > + l3edev = mlxsw_sp_span_entry_vlan(l3edev, &vid); > + > if (netif_is_bridge_master(l3edev)) { > l3edev = mlxsw_sp_span_entry_bridge(l3edev, dmac, &vid); > if (!l3edev) > goto unoffloadable; > } > > + if (is_vlan_dev(l3edev)) { > + if (vid || !(l3edev->flags & IFF_UP)) > + goto unoffloadable; > + l3edev = mlxsw_sp_span_entry_vlan(l3edev, &vid); > + } > + > if (!mlxsw_sp_port_dev_check(l3edev)) > goto unoffloadable; > > -- > 2.4.11 >
Ido Schimmel <idosch@mellanox.com> writes: > On Mon, May 07, 2018 at 04:29:39PM +0200, Petr Machata wrote: >> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c >> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c >> @@ -176,9 +176,9 @@ mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev, >> { >> struct bridge_vlan_info vinfo; >> struct net_device *edev; >> - u16 pvid; >> + u16 pvid = *p_vid; > > IIUC, in case we got here from a VLAN device configured on top of a > VLAN-aware bridge, then 'p_vid' is actually the VID of the VLAN device. > If so, then calling it PVID is incorrect. PVID is only used for untagged > traffic. Yep, I felt like the meaning is sufficiently close that I can keep the name, but I'll fix this up. We have a similar problem with l3_edev, which has become a misnomer since the bridge patches. I was reluctant to change it, because such "rename-only" patch seems like it doesn't fit anywhere. But I'll add a patch for this to v2. > Also, I'm missing the check where packets with this VLAN are even > allowed to ingress the bridge through the bridge device. Good point, I forgot about that. > Note that we don't get notified about changes to the bridge device's > VLAN filter, so this is something that needs to be added as well. Yeah, the same issue as with bridge pvid change notification. I'd like to fix this separately. I added a scrum task for this, we definitely need to take care of this in May. Thanks, Petr
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c index f6b3ca9..ac70a71 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c @@ -176,9 +176,9 @@ mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev, { struct bridge_vlan_info vinfo; struct net_device *edev; - u16 pvid; + u16 pvid = *p_vid; - if (WARN_ON(br_vlan_get_pvid(br_dev, &pvid))) + if (!pvid && WARN_ON(br_vlan_get_pvid(br_dev, &pvid))) return NULL; if (!pvid) return NULL; @@ -208,13 +208,13 @@ mlxsw_sp_span_entry_bridge(const struct net_device *br_dev, { struct mlxsw_sp_bridge_port *bridge_port; enum mlxsw_reg_spms_state spms_state; + struct net_device *dev = NULL; struct mlxsw_sp_port *port; - struct net_device *dev; u8 stp_state; if (br_vlan_enabled(br_dev)) dev = mlxsw_sp_span_entry_bridge_8021q(br_dev, dmac, p_vid); - else + else if (!*p_vid) dev = mlxsw_sp_span_entry_bridge_8021d(br_dev, dmac); if (!dev) return NULL; @@ -261,12 +261,21 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev, if (!l3edev || mlxsw_sp_span_dmac(tbl, &gw, l3edev, dmac)) goto unoffloadable; + if (is_vlan_dev(l3edev)) + l3edev = mlxsw_sp_span_entry_vlan(l3edev, &vid); + if (netif_is_bridge_master(l3edev)) { l3edev = mlxsw_sp_span_entry_bridge(l3edev, dmac, &vid); if (!l3edev) goto unoffloadable; } + if (is_vlan_dev(l3edev)) { + if (vid || !(l3edev->flags & IFF_UP)) + goto unoffloadable; + l3edev = mlxsw_sp_span_entry_vlan(l3edev, &vid); + } + if (!mlxsw_sp_port_dev_check(l3edev)) goto unoffloadable;
When mirroring to a gretap or ip6gretap device, allow the underlay packet path to include VLAN devices. The following configurations are supported in underlay: - vlan over phys - vlan-unaware bridge where the egress device is vlan over phys - vlan over vlan-aware bridge where the egress device is phys Signed-off-by: Petr Machata <petrm@mellanox.com> --- drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)