Re: [RFC PATCH] net: dsa: qca8k: Add 802.1q VLAN support

Florian Fainelli
2020-07-22T22:36:51Z

On 7/22/20 12:38 PM, Jonathan McDowell wrote:
> On Tue, Jul 21, 2020 at 10:26:07AM -0700, Florian Fainelli wrote:
>> On 7/21/20 10:16 AM, Jonathan McDowell wrote:
>>> This adds full 802.1q VLAN support to the qca8k, allowing the use of
>>> vlan_filtering and more complicated bridging setups than allowed by
>>> basic port VLAN support.
>>>
>>> Tested with a number of untagged ports with separate VLANs and then a
>>> trunk port with all the VLANs tagged on it.
>>
>> This looks good to me at first glance, at least not seeing obvious
>> issue, however below are a few questions:
>
> Thanks for the comments.
>
>> - vid == 0 appears to be unsupported according to your port_vlan_prepare
>> callback, is this really the case, or is it more a case of VID 0 should
>> be pvid untagged, which is what dsa_slave_vlan_rx_add_vid() would
>> attempt to program
>
> I don't quite follow you here. VID 0 doesn't appear to be supported by
> the hardware (and several other DSA drivers don't seem to like it
> either), hence the check; I'm not clear if there's something alternate I
> should be doing in that case instead?

Is it really that the hardware does not support it, or is it that it was
not tried or poorly handled before? If the switch supports programming
the VID 0 entry as PVID egress untagged, which is what
dsa_slave_vlan_rx_add_vid() requests, then this is great, because you
have almost nothing to do.

If not, what you are doing is definitively okay, because you have a
port_bridge_join that ensures that the port matrix gets configured
correctly for bridging, if that was not supported we would be in trouble.

>
>> - since we have a qca8k_port_bridge_join() callback which programs the
>> port lookup control register, putting all ports by default in VID==1
>> does not break per-port isolation between non-bridged and bridged ports,
>> right?
>
> VLAN_MODE_NONE (set when we don't have VLAN filtering enabled)
> configures us for port filtering, ignoring the VLAN info, so I think
> we're good here.

OK, good, so just to be sure, there is no cross talk between non-bridged
ports and bridged ports even when VLAN filtering is not enabled on the
bridge, right?
--
Florian