| Message ID | 20210913144300.1265143-5-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | RTL8366(RB) cleanups part 1 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | CHECK: Please don't use multiple blank lines |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Mon, Sep 13, 2021 at 04:42:56PM +0200, Linus Walleij wrote: > VLAN 0 shall always be treated as untagged, as per example > from other drivers (I guess from the spec). > > Cc: Vladimir Oltean <olteanv@gmail.com> > Cc: Mauri Sandberg <sandberg@mailfence.com> > Cc: Alvin Šipraga <alsi@bang-olufsen.dk> > Cc: Florian Fainelli <f.fainelli@gmail.com> > Cc: DENG Qingfang <dqfext@gmail.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v4: > - New patch after noting that other drivers always sets VLAN 0 > as untagged. > --- "Other drivers" are not always a good example. Technically speaking, IEEE 802.1Q-2018 wants switches to _preserve_ the VID 0 found inside packets when forwarding them, but treat them the same as untagged packets otherwise (aka classify them to the port's PVID, and forward them according to the forwarding domain of the $(PVID) VLAN in that bridge). "Preserve" the VID 0 tag means "mark it as egress-tagged", so the opposite of the change you are making. Now, I know all too well it is not always possible to satisfy that expectation, and we have had some back-and-forth on other drivers about this, and ended up accepting the fact that the processing of VID 0 is more or less broken. User space deals with that the best it can (read as: sometimes it can't): https://sourceforge.net/p/linuxptp/mailman/message/37318312/ But the justification given here to make VID 0 egress-untagged is pretty weak as it is.
diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c index 0672dd56c698..fae14c448fe4 100644 --- a/drivers/net/dsa/rtl8366.c +++ b/drivers/net/dsa/rtl8366.c @@ -308,6 +308,11 @@ int rtl8366_vlan_add(struct dsa_switch *ds, int port, return -EINVAL; } + /* Note that VLAN 0 shall always be treated as untagged */ + if (vlan->vid == 0) + untagged = true; + + /* Enable VLAN in the hardware * FIXME: what's with this 4k business? * Just rtl8366_enable_vlan() seems inconclusive.
VLAN 0 shall always be treated as untagged, as per example from other drivers (I guess from the spec). Cc: Vladimir Oltean <olteanv@gmail.com> Cc: Mauri Sandberg <sandberg@mailfence.com> Cc: Alvin Šipraga <alsi@bang-olufsen.dk> Cc: Florian Fainelli <f.fainelli@gmail.com> Cc: DENG Qingfang <dqfext@gmail.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v4: - New patch after noting that other drivers always sets VLAN 0 as untagged. --- drivers/net/dsa/rtl8366.c | 5 +++++ 1 file changed, 5 insertions(+)