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

[Kurt Kanzenbach <kurt@linutronix.de>]

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

On Thu Jul 16 2020, Vladimir Oltean wrote:
> On Mon, Jul 13, 2020 at 08:30:25AM +0200, Kurt Kanzenbach wrote:
>> On Sat Jul 11 2020, Florian Fainelli wrote:
>> > On 7/10/2020 4:36 AM, Kurt Kanzenbach wrote:
>> > [snip]
>> >
>> >> +
>> >> +/* Default setup for DSA:
>> >> + *  VLAN 2: CPU and Port 1 egress untagged.
>> >> + *  VLAN 3: CPU and Port 2 egress untagged.
>> >
>> > Can you use any of the DSA_TAG_8021Q services to help you with that?
>> 
>> Maybe dsa_port_setup_8021q_tagging() could be used. It does distinguish
>> between RX and TX, but I assume it'd also work. Needs to be tested.
>> 
>
> The fundamental role of DSA_TAG_8021Q is not to give you port
> separation, but port identification, when there is no hardware tagging
> support, or it cannot be used.

OK. Then it shouldn't be used.

> So in fact, it is quite the contrary:
> tag_8021q assumes that port separation will be achieved by external
> means. Most switches support a "port forwarding matrix" of sorts (i.e.
> "is port i allowed to forward to port j?"), and that is what is used, in
> tag_8021q setups, to isolate one port from another on RX (in standalone
> mode). I'm not sure what's the status with hellcreek hardware design, it
> seems very odd to me to not include any sort of port forwarding matrix,
> and to have to rely on port membership on each port's pvid to achieve
> that in the .port_bridge_join method.

As far as I know there is no port forwarding matrix. Traffic is
forwarded between the ports when they're members of the same
vlan. That's why I created them by default.

> By the way, this brings up another topic: any 'bridge vlan add' for a
> pvid will break your setup. You should avoid that somehow.

So usually for vlan configurations the bridge is created with
vlan_filtering set. In that case the default setting is reverted (see
hellcreek_vlan_filtering()).

> Please try to set ds->configure_vlan_while_not_filtering = true; in your
> .setup callback.  We're trying to make all switches behave uniformly and
> be able to deal with VLANs added straight away by the bridge. Don't be
> confused by the fact that it's an option - there's nothing really
> optional about it, it is just there to avoid breakage in drivers which
> haven't been converted.  Since yours is a new driver, it should enable
> this option from day 1.

OK.

Thanks,
Kurt

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