Re: [PATCH v5 2/7] net: dsa: Add DSA driver for Hirschmann Hellcreek switches

[Kurt Kanzenbach <kurt@linutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3122 bytes --]

On Mon Sep 07 2020, Vladimir Oltean wrote:
> On Mon, Sep 07, 2020 at 02:49:03PM +0200, Kurt Kanzenbach wrote:
>> On Mon Sep 07 2020, Vladimir Oltean wrote:
>> > On Mon, Sep 07, 2020 at 08:05:25AM +0200, Kurt Kanzenbach wrote:
>> >> Well, that depends on whether hellcreek_vlan_add() is called for
>> >> creating that vlan interfaces. In general: As soon as both ports are
>> >> members of the same vlan that traffic is switched.
>> >
>> > That's indeed what I would expect.
>> > Not only that, but with your pvid-based setup, you only ensure port
>> > separation for untagged traffic anyway.
>>
>> Why? Tagged traffic is dropped unless the vlan is configured somehow. By
>> default, I've configured vlan 2 and 3 to reflect the port separation for
>> DSA. At reset the ports aren't members of any vlan.
>>
>
> Wait, so what is the out-of-reset state of "ptcfg & HR_PTCFG_INGRESSFLT"?

No, ingress filtering is not set by default. But, still the ports are by
default not members of any vlan. So, I thought the traffic will be
dropped as well. I'll check that. 

> If it is filtering by default (and even if it isn't, but you can make
> it), then I suppose you can keep it like that, and try to model your
> ports something like this:
>
> - force "ethtool -k swpN | grep rx-vlan-filter" to return "on (fixed)".
> - enforce a check that in standalone mode, you can't have an 8021q upper
>   interface with the same VLAN ID on more than 1 port at the same time.
>   This will be the only way in which you can terminate VLAN traffic on
>   standalone ports.
>
> If you do this, I think you should be compliant with the stack.

OK, great. I'll look into it.

>> OK. when a new driver should set the flag, then I'll set it. So, all
>> vlan requests programming requests should be "buffered" and executed
>> when vlan filtering is enabled? What is it good for?
>
> It is good for correct functionality of the hardware, I don't get the
> question? If your driver makes private use of VLAN tags beyond what the
> upper layers ask for, then it should keep track of them.

OK.

> DSA has, in the past, ignored VLAN switchdev operations from the
> bridge when not in vlan_filtering mode, for unknown reasons. This is
> known to break some command sequences (see below), so the consensus at
> the time was to stop doing that, and introduce this temporary
> compatibility flag.
>
> Some tests to make sure you're passing are:
>
> 1. Statically creating an 802.1Q bridge:
>
> ip link add br0 type bridge vlan_filtering 1
> ip link set swp1 master br0
> ip link set swp2 master br0
>
> 2. Dynamically turning an 802.1D bridge into an 802.1Q bridge:
>
> ip link add br0 type bridge
> ip link set swp1 master br0
> ip link set swp2 master br0
> ip link set br0 type bridge vlan_filtering 1
> # at this moment in time, if you don't have
> # configure_vlan_while_not_filtering = true, then the VLAN tables of
> # swp1 and swp2 will be missing the default_pvid (by default 1) of br0.

Yes, OK. I've observed this behavior myself and was confused about it.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]