From: Florian Fainelli <f.fainelli@gmail.com>
To: Jonathan McDowell <noodles@earth.li>
Cc: Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
David Miller <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Russell King - ARM Linux admin <linux@armlinux.org.uk>,
Matthew Hagan <mnhagan88@gmail.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] net: dsa: qca8k: Add 802.1q VLAN support
Date: Wed, 22 Jul 2020 15:36:38 -0700 [thread overview]
Message-ID: <77c136d0-c183-ebb5-5954-647e08c8ec18@gmail.com> (raw)
In-Reply-To: <20200722193850.GM23489@earth.li>
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
next prev parent reply other threads:[~2020-07-22 22:36 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-21 17:16 [RFC PATCH] net: dsa: qca8k: Add 802.1q VLAN support Jonathan McDowell
2020-07-21 17:26 ` Florian Fainelli
2020-07-22 19:38 ` Jonathan McDowell
2020-07-22 22:36 ` Florian Fainelli [this message]
2020-07-22 22:58 ` Vladimir Oltean
2020-07-25 17:35 ` Jonathan McDowell
2020-07-21 20:48 ` Russell King - ARM Linux admin
2020-07-22 19:33 ` Jonathan McDowell
2020-07-26 14:56 ` [PATCH net-next v2] " Jonathan McDowell
2020-07-28 16:34 ` Vladimir Oltean
2020-07-30 10:40 ` Jonathan McDowell
2020-07-30 21:10 ` Vladimir Oltean
2020-08-01 17:05 ` [PATCH net-next v3 1/2] net: dsa: qca8k: Add define for port VID Jonathan McDowell
2020-08-01 20:48 ` Florian Fainelli
2020-08-02 13:21 ` Vladimir Oltean
2020-08-03 22:45 ` David Miller
2020-08-01 17:06 ` [PATCH net-next v3 2/2] net: dsa: qca8k: Add 802.1q VLAN support Jonathan McDowell
2020-08-01 20:50 ` Florian Fainelli
2020-08-02 13:21 ` Vladimir Oltean
2020-08-03 22:46 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=77c136d0-c183-ebb5-5954-647e08c8ec18@gmail.com \
--to=f.fainelli@gmail.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mnhagan88@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=noodles@earth.li \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).