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

Vladimir Oltean
2020-07-16T08:29:41Z

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

Thanks,
-Vladimir