From: "Alvin Šipraga" <alvin.sipraga@analog.com>
To: Stijn Cleynhens <stijncleynhens@gmail.com>
Cc: netdev@vger.kernel.org, Linus Walleij <linusw@kernel.org>,
Luiz Angelo Daros de Luca <luizluca@gmail.com>,
Andrew Lunn <andrew@lunn.ch>, Vladimir Oltean <olteanv@gmail.com>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@kernel.org>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Mieczyslaw Nalewaj <namiltd@yahoo.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net 1/2] net: dsa: realtek: rtl8365mb: use IVL in VLAN 0
Date: Wed, 4 Nov 2026 16:22:58 +0100 [thread overview]
Message-ID: <autLlP81HFeMyICc@analog.com> (raw)
In-Reply-To: <20261104082514.2139278-2-stijncleynhens@gmail.com>
On Wed, Nov 04, 2026 at 09:25:13AM +0100, Stijn Cleynhens wrote:
> DSA installs the FDB and MDB entries of standalone ports and of
> VLAN-unaware bridges with VID 0, and the driver writes them with
> Independent VLAN Learning. No bridge VLAN sets up the VLAN4k entry of
> VID 0 though, and it defaults to Shared VLAN Learning, so the switch
> looks up and learns frames classified to VID 0 by FID. Lookups never hit
> the IVL-keyed static entries, and a learned entry for the same MAC sits
> alongside the static one.
>
> Set up VID 0 with IVL, like every other VLAN the driver programs.
>
> On its own this has no visible effect: standalone ports do not learn and
> the driver does not control host flooding, so the unused static entries
> only meant that traffic was flooded to the CPU. The next patch, which
> moves VLAN-unaware bridges onto VID 0, depends on it.
Yes, all this makes sense. I actually remember doing basically the same
thing back when I was working on this, but it seems not to have survived
the final work by Luiz. It would be good to get his input on why. I
think he asked me at some point, but I couldn't remember, and so he
rightfully dropped it. But I could be wrong.
>
> Fixes: 336e3e4a1ab3 ("net: dsa: realtek: rtl8365mb: add FDB support")
> Signed-off-by: Stijn Cleynhens <stijncleynhens@gmail.com>
> ---
> drivers/net/dsa/realtek/rtl8365mb_main.c | 8 ++++++
> drivers/net/dsa/realtek/rtl8365mb_vlan.c | 31 ++++++++++++++++++++++++
> drivers/net/dsa/realtek/rtl8365mb_vlan.h | 1 +
> 3 files changed, 40 insertions(+)
>
> diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
> index da06f952a..b90e46c16 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb_main.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
> @@ -2232,6 +2232,14 @@ static int rtl8365mb_vlan_setup(struct dsa_switch *ds)
> }
> }
>
> + mutex_lock(&priv->vlan_lock);
> + ret = rtl8365mb_vlan_4k_setup_neutral(priv);
> + mutex_unlock(&priv->vlan_lock);
So I'm not sure I like this function being added to the vlan.c
library. Semantically it makes more sense to me for the main.c driver to
actually call rtl8365mb_vlan4k_port_add() for each port, so as to
explicitly say: "all ports are, one way or another, members of this
neutral VLAN". Then you would get IVL automatically.
But that API uses a struct switchdev_obj, which would be a bit crude to
construct here... So I see why you did it like this.
If you could give it a second thought and see if there's a neater way,
that would be nice. But either way I think this is fine, so:
Reviewed-by: Alvin Šipraga <alvin.sipraga@analog.com>
> + if (ret) {
> + dev_err(priv->dev, "Failed to set up VLAN 0\n");
> + return ret;
> + }
> +
> /* VLAN is always enabled. */
> ret = regmap_update_bits(priv->map, RTL8365MB_VLAN_CTRL_REG,
> RTL8365MB_VLAN_CTRL_EN_MASK,
> diff --git a/drivers/net/dsa/realtek/rtl8365mb_vlan.c b/drivers/net/dsa/realtek/rtl8365mb_vlan.c
> index 8d49ffadc..58948e76f 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb_vlan.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb_vlan.c
> @@ -322,6 +322,37 @@ rtl8365mb_vlan_4k_port_set(struct dsa_switch *ds, int port,
> return 0;
> }
>
> +/*
> + * rtl8365mb_vlan_4k_setup_neutral() - Use IVL in the neutral VLAN
> + * @priv: realtek_priv pointer
> + *
> + * Untagged frames of standalone ports and of ports in a VLAN-unaware bridge
> + * are classified to VID 0, the VID of the neutral PVID. No bridge VLAN sets
> + * up its VLAN4k entry, which by default uses Shared VLAN Learning, so the
> + * switch would learn these frames by FID while the driver installs its FDB
> + * and MDB entries for VID 0 with Independent VLAN Learning. Use IVL in VID 0
> + * as in every other VLAN.
> + *
> + * Context: Can sleep. Must be called with &priv->vlan_lock held.
> + * Takes and releases &priv->map_lock.
> + * Return: 0 on success, or a negative error code on failure.
> + */
> +int rtl8365mb_vlan_4k_setup_neutral(struct realtek_priv *priv)
> +{
> + struct rtl8365mb_vlan4k vlan4k = {0};
> + int ret;
> +
> + lockdep_assert_held(&priv->vlan_lock);
> +
> + ret = rtl8365mb_vlan_4k_read(priv, 0, &vlan4k);
> + if (ret)
> + return ret;
> +
> + vlan4k.ivl_en = true;
> +
> + return rtl8365mb_vlan_4k_write(priv, &vlan4k);
> +}
> +
> /*
> * rtl8365mb_vlan_4k_port_add() - Add a port to a VLAN 4K table entry
> * @ds: dsa switch instance
> diff --git a/drivers/net/dsa/realtek/rtl8365mb_vlan.h b/drivers/net/dsa/realtek/rtl8365mb_vlan.h
> index 27a526b61..809034009 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb_vlan.h
> +++ b/drivers/net/dsa/realtek/rtl8365mb_vlan.h
> @@ -28,6 +28,7 @@ int
> rtl8365mb_vlan_port_set_framefilter(struct realtek_priv *priv,
> int port,
> enum rtl8365mb_frame_ingress frame_type);
> +int rtl8365mb_vlan_4k_setup_neutral(struct realtek_priv *priv);
> int rtl8365mb_vlan_4k_port_add(struct dsa_switch *ds, int port,
> const struct switchdev_obj_port_vlan *vlan,
> struct netlink_ext_ack *extack);
> --
> 2.56.0
>
next prev parent reply other threads:[~2026-11-04 15:23 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 8:25 [PATCH net 0/2] net: dsa: realtek: rtl8365mb: fix FDB lookups in VLAN-unaware bridges Stijn Cleynhens
2026-11-04 8:25 ` [PATCH net 1/2] net: dsa: realtek: rtl8365mb: use IVL in VLAN 0 Stijn Cleynhens
2026-11-04 15:22 ` Alvin Šipraga [this message]
2026-11-04 8:25 ` [PATCH net 2/2] net: dsa: realtek: rtl8365mb: keep the neutral PVID in VLAN-unaware bridges Stijn Cleynhens
2026-11-04 15:57 ` Alvin Šipraga
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=autLlP81HFeMyICc@analog.com \
--to=alvin.sipraga@analog.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@kernel.org \
--cc=kuba@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luizluca@gmail.com \
--cc=namiltd@yahoo.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=stijncleynhens@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.