On Tue, Dec 18, 2018 at 12:13:38PM -0800, Florian Fainelli wrote:
> On 12/17/18 11:01 PM, Ido Schimmel wrote:
> > On Sun, Dec 16, 2018 at 09:14:09AM -0800, Florian Fainelli wrote:
> >> Le 12/16/18 à 12:25 AM, Ido Schimmel a écrit :
> >>> On Wed, Dec 12, 2018 at 03:09:43PM -0800, Florian Fainelli wrote:
> >>>> +Non-bridged network ports of the same switch fabric must not be disturbed in any
> >>>> +way, shape or form by the enabling of VLAN filtering.
> >>>> +
> >>>> +VLAN devices configured on top of a switchdev network device (e.g: sw0p1.100)
> >>>> +which is a bridge port member must also observe the following behavior:
> >>>> +
> >>>> +- with VLAN filtering turned off, these VLAN devices must be fully functional
> >>>> + since the hardware is allowed VID frames
> >>>> +
> >>>> +- with VLAN filtering turned on, these VLAN devices are not going to be
> >>>> + functional unless the bridge's VLAN database is also configured to have that
> >>>> + VID enabled for the underlying network device/port
> >>>> + (e.g: bridge vlan add vid 100 dev sw0p1)
> >>>
> >>> mlxsw forbids the enslavement of VLAN devices to VLAN-aware bridges. It
> >>> doesn't really make sense to enable VLAN filtering when all the packets
> >>> are untagged.
> >>
> >> Did you mean VLAN-unaware here, otherwise that would contradict the
> >> statement that VLAN-aware bridges mean everything untagged, or am I
> >> incorrectly understanding things here?
> >
> > I meant VLAN-aware... In a VLAN-unaware bridge the VLAN is meaningless.
> > For example, there is no filtering based on VLAN at ingress/egress and
> > FDB entries are only searched based on MAC (VLAN is always 0). This is
> > in contrast to a VLAN-aware bridge.
> >
> > When you enslave VLAN netdevs to a bridge, the bridge sees untagged
> > packets. The VLAN tag is pulled from the packet in Rx path and then the
> > packet is injected to the bridge via the Rx handler configured on the
> > VLAN netdev. Therefore, there is point in enslaving these device to a
> > VLAN-aware bridge.
>
> I see what you describe and that is not quite what I was talking about,
> see below.
>
> >
> > Also, mlxsw only supports a single VLAN-aware bridge. You can however,
> > configure 1K VLAN-unaware bridges.
>
> OK, how do you enforce that in the driver? I was going to do something
> as basic as: loop around all ports that are not the one being changed by
> VLAN filtering attribute, if bridge device associated is non-NULL and
> br_vlan_enabled() returns true for that bridge and we want to turn off
> VLAN filtering, then this is not possible since that would break the
> other bridge devices we have which are VLAN filtering enabled.
See mlxsw_sp_bridge_device_create(). We basically keep a list of bridges
we care about. If one is already VLAN aware, then we fail the creation
of another bridge.
>
> >
> >>> But I disagree with the comment about the underlying port. When you
> >>> configured the VLAN device, it should have enabled the VLAN filters on
> >>> the real device via ndo_vlan_rx_add_vid().
> >>
> >> That is really why I submitted this patch, because right now I have a
> >> patch (yet to be submitted) which adds ndo_vlan_rx_{add,kill}_vid() and
> >> if the underlying device is enslaved into a bridge, I just do nothing
> >> and let the bridge control the VLAN membership, hence my comment and
> >> example here.
> >>
> >> What you are saying is that we should have these two cases:
> >>
> >> 1) VLAN devices on top of VLAN unaware bridge: allow the VLAN device and
> >> program VLAN filter on the underlying switch port to permit VLAN tagging
> >
> > When you say "on top" you mean enslaved to?
>
> I meant to write: a VLAN device created on (top of) a switch port, and
> this switch port being a bridge member. The VLAN device would not be
> added as a bridge member (did not really think about it).
>
> >
> >> 2) VLAN devices on top of a VLAN aware bridge: deny the VLAN device
> >> creation and let the bridge, which is VLAN aware manage the port VLAN
> >> membership
> >
> > mlxsw does not forbid the creation of the VLAN device. It only forbids
> > its enslavement to a VLAN-aware bridge.
>
> That's done in mlxsw_sp_netdevice_port_vlan_event() right?
Almost :) See mlxsw_sp_bridge_8021q_port_join()
