netdev - Re: [PATCH net-next] Documentation: networking: Clarify switchdev devices behavior

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20181222202912.GA20191@splinter>
Date:   Sat, 22 Dec 2018 22:29:12 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Ido Schimmel <idosch@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "andrew@...n.ch" <andrew@...n.ch>, Jiri Pirko <jiri@...lanox.com>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "nikolay@...ulusnetworks.com" <nikolay@...ulusnetworks.com>,
        "roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
        "bridge@...ts.linux-foundation.org" 
        <bridge@...ts.linux-foundation.org>,
        "cphealy@...il.com" <cphealy@...il.com>
Subject: Re: [PATCH net-next] Documentation: networking: Clarify switchdev
 devices behavior

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()