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

From: Kurt Kanzenbach <kurt@linutronix.de>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Richard Cochran <richardcochran@gmail.com>,
	Kamil Alkhouri <kamil.alkhouri@hs-offenburg.de>,
	ilias.apalodimas@linaro.org
Subject: Re: [PATCH net-next v6 2/7] net: dsa: Add DSA driver for Hirschmann Hellcreek switches
Date: Tue, 06 Oct 2020 16:05:11 +0200	[thread overview]
Message-ID: <87o8lf78mg.fsf@kurt> (raw)
In-Reply-To: <20201006134247.7uwchuo6s7lmyzeq@skbuf>

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

On Tue Oct 06 2020, Vladimir Oltean wrote:
> On Tue, Oct 06, 2020 at 03:23:36PM +0200, Kurt Kanzenbach wrote:
>> On Tue Oct 06 2020, Vladimir Oltean wrote:
>> Does this mean that tagged traffic is forwarded no matter what?
>
> Precisely. The bridge VLAN table should be irrelevant to the acceptance
> or forwarding decision of the packet if vlan_filtering is 0.

I see.

>
>> That doesn't work with the current implementation, because the VLAN
>> tags are interpreted by default. There's a global flag to put the
>> switch in VLAN unaware mode. But it's global and not per bridge or
>> port.
>
> Oh, there is? Maybe you can use it then.
>
> JUST FOR CONTEXT, for sja1105 and felix/ocelot, this is the mode that
> they're operating in, when a bridge with vlan_filtering=0 is configured
> as an upper.
>
> In sja1105, I don't even have the VLAN awareness flag that you have. So
> I need to change the VLAN TPID from 0x8100 to 0xdadb, and the switch
> will think that VLAN-tagged frames aren't VLAN. So all frames are tagged
> internally by the switch with the port-based VLAN ID and PCP, when in
> vlan_filtering=0.
> And because my knob is global and not per bridge either, I just set
> ds->vlan_filtering_is_global = true and let DSA handle the rest.

What's that flag doing? ...

	/* Disallow bridge core from requesting different VLAN awareness
	 * settings on ports if not hardware-supported
	 */
	bool			vlan_filtering_is_global;

OK, that's what I need for the bridging part.

>
> As for ocelot/felix, those switches have 2 knobs:
> - VLAN awareness: does the ingress port derive the classified VLAN from
>   the packet's 802.1Q header? If yes, the VLAN ID and PCP are taken from
>   the packet. If not, they are taken from the port-based default.
> - VLAN ingress filtering: does the ingress port drop a VLAN-tagged frame
>   if the classified VLAN is not installed in its ingress filter?
>
> As you may guess, even for ocelot/felix, when we have a bridge with
> vlan_filtering=0, we are still configuring it as:
> VLAN awareness = disabled
> VLAN ingress filtering = enabled
>
> Because the classified VLAN is not derived from the packet, it will
> always be equal to the pvid of the port, which is installed privately by
> the driver. So no packet drops due to VLAN, regardless of VLAN ID.
>
>> So you're saying private VLANs can be used but the user or the other
>> kernel modules shouldn't be allowed to use them to simplify the
>> implementation?  Makes sense to me.
>
> Yes.
> And because the user is more likely to install VLAN 2 and 3 than 4095
> and 4094, maybe you could use private VLANs from the end of the range,
> just to make this restriction less obvious (or maybe not at all).
>
>> The egress port has to member to that VLAN.
>
> Same as ocelot/felix. This is the reason why we make it VLAN-unaware.
> There's no point in disabling just VLAN ingress filtering if the end
> result is still going to be a drop, albeit due to a different reason (no
> destinations).

OK.

Thanks,
Kurt

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