Date:   Tue, 18 Dec 2018 12:13:38 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Ido Schimmel <idosch@...lanox.com>
Cc:     "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 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.

> 
>>> 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?

> 
>> In case 1) or 2) if the desire is to have a VLAN aware network device
>> this can be either done through a VLAN device on top of the switch port,
>> or through a VLAN device on top of the bridge master itself, and in
>> either case, this amounts to doing about the same thing.
>>
>> Did I get this right?
> 
> I'm saying that a VLAN-aware bridge with VIDs 10-100 (for example) is
> equivalent to VLAN devices with VIDs 10-100 enslaved to br10-br100,
> respectively. It is up to you if you want to / can support both modes.
> We support both, but most / all users use the VLAN-aware bridge.
> 

Right, and it is also equivalent to have these two things:

- a VLAN aware bridge, adding VIDs 10-100 to sw0p1 through "bridge vlan
add vid .." and creating br0.10..br0.100 devices

and:

- a VLAN aware bridge, creating sw0p1.10..sw0p1.100 (VLAN devices) while
sw0p1 is a bridge port member
-- 
Florian

Powered by blists - more mailing lists