Add support for vlanaware bridge. We reserve vid 4095 for standalone
mode, to implement fdb-isolation. A vlan-unaware bridge uses vid 0.
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
---
Changes in v2:
- redesign based on selftests which rely on changing vlan_default_pvid.
Our HW limitations were too forward. Following Vladimirs changes to
ocelot VLAN implementation, we now dynamically change egress tag
configuration, allowing more states.
- selftests are passing, except an expected failure w.r.t ctag/stag
conformance, which is a hw limitation.
---
drivers/net/dsa/microchip/lan9645x/Makefile | 1 +
drivers/net/dsa/microchip/lan9645x/lan9645x_main.c | 49 +++
drivers/net/dsa/microchip/lan9645x/lan9645x_main.h | 28 ++
drivers/net/dsa/microchip/lan9645x/lan9645x_port.c | 3 +
drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c | 353 +++++++++++++++++++++
5 files changed, 434 insertions(+)
diff --git a/drivers/net/dsa/microchip/lan9645x/Makefile b/drivers/net/dsa/microchip/lan9645x/Makefile
index 7cc0ae0ada40..e049114b3563 100644
--- a/drivers/net/dsa/microchip/lan9645x/Makefile
+++ b/drivers/net/dsa/microchip/lan9645x/Makefile
@@ -6,3 +6,4 @@ mchp-lan9645x-objs := \
lan9645x_npi.o \
lan9645x_phylink.o \
lan9645x_port.o \
+ lan9645x_vlan.o \
diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
index 599e589c4ec3..046c95a72242 100644
--- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
@@ -156,6 +156,7 @@ static int lan9645x_setup(struct dsa_switch *ds)
}
mutex_init(&lan9645x->fwd_domain_lock);
+ lan9645x_vlan_init(lan9645x);
/* Link Aggregation Mode: NETDEV_LAG_HASH_L2 */
lan_wr(ANA_AGGR_CFG_AC_SMAC_ENA |
@@ -535,11 +536,54 @@ static void lan9645x_port_bridge_leave(struct dsa_switch *ds, int port,
if (!lan9645x->bridge_mask)
lan9645x->bridge = NULL;
+ lan9645x_vlan_set_hostmode(p);
lan9645x_update_fwd_mask(lan9645x);
mutex_unlock(&lan9645x->fwd_domain_lock);
}
+static int lan9645x_port_vlan_filtering(struct dsa_switch *ds, int port,
+ bool enabled,
+ struct netlink_ext_ack *extack)
+{
+ struct lan9645x *lan9645x = ds->priv;
+ struct lan9645x_port *p;
+
+ p = lan9645x_to_port(lan9645x, port);
+ p->vlan_aware = enabled;
+ lan9645x_vlan_port_apply(p);
+
+ return 0;
+}
+
+static int lan9645x_port_vlan_add(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan,
+ struct netlink_ext_ack *extack)
+{
+ struct lan9645x *lan9645x = ds->priv;
+ struct lan9645x_port *p;
+ bool pvid, untagged;
+
+ p = lan9645x_to_port(lan9645x, port);
+ pvid = !!(vlan->flags & BRIDGE_VLAN_INFO_PVID);
+ untagged = !!(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
+
+ return lan9645x_vlan_port_add_vlan(p, vlan->vid, pvid, untagged,
+ extack);
+}
+
+static int lan9645x_port_vlan_del(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
+{
+ struct lan9645x *lan9645x = ds->priv;
+ struct lan9645x_port *p;
+
+ p = lan9645x_to_port(lan9645x, port);
+ lan9645x_vlan_port_del_vlan(p, vlan->vid);
+
+ return 0;
+}
+
static const struct dsa_switch_ops lan9645x_switch_ops = {
.get_tag_protocol = lan9645x_get_tag_protocol,
@@ -562,6 +606,11 @@ static const struct dsa_switch_ops lan9645x_switch_ops = {
.port_bridge_leave = lan9645x_port_bridge_leave,
.port_stp_state_set = lan9645x_port_bridge_stp_state_set,
.port_set_host_flood = lan9645x_port_set_host_flood,
+
+ /* VLAN integration */
+ .port_vlan_filtering = lan9645x_port_vlan_filtering,
+ .port_vlan_add = lan9645x_port_vlan_add,
+ .port_vlan_del = lan9645x_port_vlan_del,
};
static int lan9645x_request_target_regmaps(struct lan9645x *lan9645x)
diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.h b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.h
index 42048e16bb47..b2b2a88083c3 100644
--- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.h
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.h
@@ -7,6 +7,7 @@
#include <linux/dsa/lan9645x.h>
#include <linux/if_bridge.h>
+#include <linux/if_vlan.h>
#include <linux/regmap.h>
#include <net/dsa.h>
@@ -150,6 +151,17 @@ enum lan9645x_vlan_port_tag {
LAN9645X_TAG_ALL = 3,
};
+struct lan9645x_vlan {
+ u32 portmask: 10, /* ports 0-8 + CPU_PORT */
+ untagged: 9, /* ports 0-8 */
+ src_chk: 1,
+ mir: 1,
+ lrn_dis: 1,
+ prv_vlan: 1,
+ fld_dis: 1,
+ s_fwd_ena: 1;
+};
+
struct lan9645x {
struct device *dev;
struct dsa_switch *ds;
@@ -174,6 +186,9 @@ struct lan9645x {
u16 bridge_fwd_mask; /* Mask for forwarding bridged ports */
struct mutex fwd_domain_lock; /* lock forwarding configuration */
+ /* VLAN entries */
+ struct lan9645x_vlan vlans[VLAN_N_VID];
+
int num_port_dis;
bool dd_dis;
bool tsn_dis;
@@ -186,6 +201,9 @@ struct lan9645x_port {
u8 stp_state;
bool learn_ena;
+ bool vlan_aware;
+ u16 pvid;
+
bool rx_internal_delay;
bool tx_internal_delay;
};
@@ -349,4 +367,14 @@ void lan9645x_phylink_get_caps(struct lan9645x *lan9645x, int port,
struct phylink_config *c);
void lan9645x_phylink_port_down(struct lan9645x *lan9645x, int port);
+/* VLAN lan9645x_vlan.c */
+void lan9645x_vlan_init(struct lan9645x *lan9645x);
+u16 lan9645x_vlan_unaware_pvid(bool is_bridged);
+void lan9645x_vlan_port_apply(struct lan9645x_port *p);
+int lan9645x_vlan_port_add_vlan(struct lan9645x_port *p, u16 vid, bool pvid,
+ bool untagged,
+ struct netlink_ext_ack *extack);
+void lan9645x_vlan_port_del_vlan(struct lan9645x_port *p, u16 vid);
+void lan9645x_vlan_set_hostmode(struct lan9645x_port *p);
+
#endif /* __LAN9645X_MAIN_H__ */
diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
index ba3cbb8d75b3..3fe8de0ce902 100644
--- a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
@@ -175,5 +175,8 @@ int lan9645x_port_setup(struct dsa_switch *ds, int port)
ANA_PORT_CFG_PORTID_VAL,
lan9645x, ANA_PORT_CFG(p->chip_port));
+ if (p->chip_port != lan9645x->npi)
+ lan9645x_vlan_set_hostmode(p);
+
return 0;
}
diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
new file mode 100644
index 000000000000..0459932a626c
--- /dev/null
+++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
@@ -0,0 +1,353 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2026 Microchip Technology Inc.
+ */
+
+#include "lan9645x_main.h"
+
+#define VLANACCESS_CMD_IDLE 0
+#define VLANACCESS_CMD_READ 1
+#define VLANACCESS_CMD_WRITE 2
+#define VLANACCESS_CMD_INIT 3
+
+struct lan9645x_vlan_port_info {
+ int untagged;
+ int tagged;
+ u16 untagged_vid;
+};
+
+/* Calculate VLAN state of a port, across all VLANS. */
+static void lan9645x_vlan_port_get_info(struct lan9645x *lan9645x, int port,
+ struct lan9645x_vlan_port_info *info)
+{
+ u16 vid;
+
+ info->untagged = 0;
+ info->tagged = 0;
+ info->untagged_vid = 0;
+
+ for (vid = 1; vid <= VLAN_MAX; vid++) {
+ struct lan9645x_vlan *v = &lan9645x->vlans[vid];
+
+ if (!(v->portmask & BIT(port)))
+ continue;
+
+ if (v->untagged & BIT(port)) {
+ info->untagged++;
+ info->untagged_vid = vid;
+ } else {
+ info->tagged++;
+ }
+
+ /* VLAN composition is invalid, so break early. */
+ if (info->untagged > 1 && info->tagged)
+ break;
+ }
+}
+
+static int lan9645x_vlan_wait_for_completion(struct lan9645x *lan9645x)
+{
+ u32 val;
+
+ return lan9645x_rd_poll_timeout(lan9645x, ANA_VLANACCESS, val,
+ ANA_VLANACCESS_VLAN_TBL_CMD_GET(val) ==
+ VLANACCESS_CMD_IDLE);
+}
+
+static void lan9645x_vlan_hw_wr(struct lan9645x *lan9645x, u16 vid)
+{
+ struct lan9645x_vlan *v = &lan9645x->vlans[vid];
+ bool cpu_dis = !(v->portmask & BIT(CPU_PORT));
+ u32 val;
+
+ val = ANA_VLANTIDX_VLAN_PGID_CPU_DIS_SET(cpu_dis) |
+ ANA_VLANTIDX_V_INDEX_SET(vid) |
+ ANA_VLANTIDX_VLAN_SEC_FWD_ENA_SET(v->s_fwd_ena) |
+ ANA_VLANTIDX_VLAN_FLOOD_DIS_SET(v->fld_dis) |
+ ANA_VLANTIDX_VLAN_PRIV_VLAN_SET(v->prv_vlan) |
+ ANA_VLANTIDX_VLAN_LEARN_DISABLED_SET(v->lrn_dis) |
+ ANA_VLANTIDX_VLAN_MIRROR_SET(v->mir) |
+ ANA_VLANTIDX_VLAN_SRC_CHK_SET(v->src_chk);
+
+ lan_wr(val, lan9645x, ANA_VLANTIDX);
+ lan_wr(v->portmask, lan9645x, ANA_VLAN_PORT_MASK);
+ lan_wr(VLANACCESS_CMD_WRITE, lan9645x, ANA_VLANACCESS);
+
+ if (lan9645x_vlan_wait_for_completion(lan9645x))
+ dev_err(lan9645x->dev, "Vlan set mask failed\n");
+}
+
+u16 lan9645x_vlan_unaware_pvid(bool is_bridged)
+{
+ return is_bridged ? UNAWARE_PVID : HOST_PVID;
+}
+
+static u16 lan9645x_vlan_port_get_pvid(struct lan9645x_port *port)
+{
+ bool is_bridged = lan9645x_port_is_bridged(port);
+
+ if (is_bridged && port->vlan_aware)
+ return port->pvid;
+ else
+ return lan9645x_vlan_unaware_pvid(is_bridged);
+}
+
+/* Dynamically choose the egress tagging mode based on the port vlan state:
+ *
+ * Standalone:
+ * TAG_NO_PVID_NO_UNAWARE with PORT_VID=HOST_PVID. This avoids leaking the
+ * internal HOST_PVID tag on ingress mirrored frames while leaving normal
+ * egress frames untagged.
+ *
+ * Bridged, VLAN-aware:
+ * - N untagged, 0 tagged: TAG_DISABLED
+ * - 1 untagged, N tagged: TAG_NO_PVID_NO_UNAWARE
+ * - 0 untagged, N tagged: TAG_ALL
+ *
+ * Bridged, VLAN-unaware:
+ * TAG_DISABLED
+ */
+static void
+lan9645x_vlan_port_apply_egress(struct lan9645x_port *p,
+ struct lan9645x_vlan_port_info *info)
+{
+ struct lan9645x *lan9645x = p->lan9645x;
+ enum lan9645x_vlan_port_tag tag_cfg;
+ u16 port_vid = UNAWARE_PVID;
+
+ if (!lan9645x_port_is_bridged(p)) {
+ tag_cfg = LAN9645X_TAG_NO_PVID_NO_UNAWARE;
+ port_vid = HOST_PVID;
+ } else if (p->vlan_aware) {
+ struct lan9645x_vlan_port_info _info;
+
+ if (!info) {
+ lan9645x_vlan_port_get_info(lan9645x, p->chip_port,
+ &_info);
+ info = &_info;
+ }
+
+ if (info->untagged == 1 && info->tagged) {
+ tag_cfg = LAN9645X_TAG_NO_PVID_NO_UNAWARE;
+ port_vid = info->untagged_vid;
+ } else if (info->untagged) {
+ tag_cfg = LAN9645X_TAG_DISABLED;
+ } else {
+ tag_cfg = LAN9645X_TAG_ALL;
+ }
+ } else {
+ tag_cfg = LAN9645X_TAG_DISABLED;
+ }
+
+ /* TAG_TPID_CFG encoding:
+ *
+ * 0: Use 0x8100.
+ * 1: Use 0x88A8.
+ * 2: Use custom value from PORT_VLAN_CFG.PORT_TPID.
+ * 3: Use PORT_VLAN_CFG.PORT_TPID, unless ingress tag was a C-tag
+ * (EtherType = 0x8100)
+ *
+ * Use 3 and PORT_VLAN_CFG.PORT_TPID=0x88a8 to ensure stags are not
+ * rewritten to ctags on egress.
+ */
+ lan_rmw(REW_TAG_CFG_TAG_TPID_CFG_SET(3) |
+ REW_TAG_CFG_TAG_CFG_SET(tag_cfg),
+ REW_TAG_CFG_TAG_TPID_CFG |
+ REW_TAG_CFG_TAG_CFG,
+ lan9645x, REW_TAG_CFG(p->chip_port));
+
+ lan_rmw(REW_PORT_VLAN_CFG_PORT_TPID_SET(ETH_P_8021AD) |
+ REW_PORT_VLAN_CFG_PORT_VID_SET(port_vid),
+ REW_PORT_VLAN_CFG_PORT_TPID |
+ REW_PORT_VLAN_CFG_PORT_VID,
+ lan9645x, REW_PORT_VLAN_CFG(p->chip_port));
+}
+
+static void lan9645x_vlan_port_apply_ingress(struct lan9645x_port *p)
+{
+ struct lan9645x *lan9645x = p->lan9645x;
+ u16 pvid;
+ u32 val;
+
+ pvid = lan9645x_vlan_port_get_pvid(p);
+
+ /* Default vlan to classify for untagged frames (may be zero) */
+ val = ANA_VLAN_CFG_VLAN_VID_SET(pvid);
+ if (p->vlan_aware)
+ val |= ANA_VLAN_CFG_VLAN_AWARE_ENA_SET(1) |
+ ANA_VLAN_CFG_VLAN_POP_CNT_SET(1);
+
+ lan_rmw(val,
+ ANA_VLAN_CFG_VLAN_VID |
+ ANA_VLAN_CFG_VLAN_AWARE_ENA |
+ ANA_VLAN_CFG_VLAN_POP_CNT,
+ lan9645x, ANA_VLAN_CFG(p->chip_port));
+
+ lan_rmw(DEV_MAC_TAGS_CFG_VLAN_AWR_ENA_SET(p->vlan_aware) |
+ DEV_MAC_TAGS_CFG_PB_ENA_SET(p->vlan_aware),
+ DEV_MAC_TAGS_CFG_VLAN_AWR_ENA |
+ DEV_MAC_TAGS_CFG_PB_ENA,
+ lan9645x, DEV_MAC_TAGS_CFG(p->chip_port));
+
+ /* Drop frames with multicast source address */
+ val = ANA_DROP_CFG_DROP_MC_SMAC_ENA_SET(1);
+ if (p->vlan_aware && !pvid)
+ /* If port is vlan-aware and tagged, drop untagged and priority
+ * tagged frames.
+ */
+ val |= ANA_DROP_CFG_DROP_UNTAGGED_ENA_SET(1) |
+ ANA_DROP_CFG_DROP_PRIO_S_TAGGED_ENA_SET(1) |
+ ANA_DROP_CFG_DROP_PRIO_C_TAGGED_ENA_SET(1);
+
+ lan_wr(val, lan9645x, ANA_DROP_CFG(p->chip_port));
+}
+
+void lan9645x_vlan_port_apply(struct lan9645x_port *p)
+{
+ lan9645x_vlan_port_apply_ingress(p);
+ lan9645x_vlan_port_apply_egress(p, NULL);
+}
+
+static struct lan9645x_vlan *lan9645x_vlan_port_modify(struct lan9645x_port *p,
+ u16 vid, bool pvid,
+ bool untagged)
+{
+ struct lan9645x_vlan *v = &p->lan9645x->vlans[vid];
+
+ if (untagged)
+ v->untagged |= BIT(p->chip_port);
+ else
+ v->untagged &= ~BIT(p->chip_port);
+
+ if (pvid)
+ p->pvid = vid;
+ else if (p->pvid == vid)
+ p->pvid = 0;
+
+ return v;
+}
+
+static int lan9645x_vlan_cpu_add(struct lan9645x_port *p, u16 vid, bool pvid,
+ bool untagged)
+{
+ struct lan9645x_vlan *v;
+
+ v = lan9645x_vlan_port_modify(p, vid, pvid, untagged);
+ v->portmask |= BIT(CPU_PORT) | BIT(p->chip_port);
+ lan9645x_vlan_hw_wr(p->lan9645x, vid);
+ lan9645x_vlan_port_apply_ingress(p);
+
+ return 0;
+}
+
+int lan9645x_vlan_port_add_vlan(struct lan9645x_port *p, u16 vid, bool pvid,
+ bool untagged, struct netlink_ext_ack *extack)
+{
+ struct lan9645x *lan9645x = p->lan9645x;
+ struct lan9645x_vlan_port_info info;
+ struct lan9645x_vlan old_vlan;
+ struct lan9645x_vlan *v;
+ u16 old_pvid;
+
+ /* Kernel VLAN core adds vid 0, which collides with our UNAWARE_PVID.
+ * We handle priority tagged frames by other means.
+ */
+ if (!vid)
+ return 0;
+
+ if (p->chip_port == lan9645x->npi)
+ return lan9645x_vlan_cpu_add(p, vid, pvid, untagged);
+
+ if (vid > VLAN_MAX) {
+ NL_SET_ERR_MSG_MOD(extack, "VLAN range 4094-4095 reserved.");
+ return -EBUSY;
+ }
+
+ old_vlan = lan9645x->vlans[vid];
+ old_pvid = p->pvid;
+
+ v = lan9645x_vlan_port_modify(p, vid, pvid, untagged);
+ v->portmask |= BIT(p->chip_port);
+
+ lan9645x_vlan_port_get_info(lan9645x, p->chip_port, &info);
+
+ if (info.untagged > 1 && info.tagged) {
+ *v = old_vlan;
+ p->pvid = old_pvid;
+ NL_SET_ERR_MSG_MOD(extack, "Only support 1 untagged port VLAN");
+ return -EBUSY;
+ }
+
+ lan9645x_vlan_hw_wr(lan9645x, vid);
+ lan9645x_vlan_port_apply_ingress(p);
+ lan9645x_vlan_port_apply_egress(p, &info);
+
+ return 0;
+}
+
+void lan9645x_vlan_port_del_vlan(struct lan9645x_port *p, u16 vid)
+{
+ struct lan9645x *lan9645x = p->lan9645x;
+ struct lan9645x_vlan *v;
+
+ if (!vid)
+ return;
+
+ v = lan9645x_vlan_port_modify(p, vid, false, false);
+ v->portmask &= ~BIT(p->chip_port);
+
+ if (p->chip_port == lan9645x->npi)
+ v->portmask &= ~BIT(CPU_PORT);
+
+ lan9645x_vlan_hw_wr(lan9645x, vid);
+
+ if (p->chip_port == lan9645x->npi)
+ lan9645x_vlan_port_apply_ingress(p);
+ else
+ lan9645x_vlan_port_apply(p);
+}
+
+void lan9645x_vlan_set_hostmode(struct lan9645x_port *p)
+{
+ p->vlan_aware = false;
+ lan9645x_vlan_port_apply(p);
+}
+
+void lan9645x_vlan_init(struct lan9645x *lan9645x)
+{
+ u32 all_phys_ports, all_ports;
+ u16 port, vid;
+
+ all_phys_ports = GENMASK(lan9645x->num_phys_ports - 1, 0);
+ all_ports = all_phys_ports | BIT(CPU_PORT);
+
+ /* Clear VLAN table, by default all ports are members of all VLANS */
+ lan_wr(ANA_VLANACCESS_VLAN_TBL_CMD_SET(VLANACCESS_CMD_INIT),
+ lan9645x, ANA_VLANACCESS);
+
+ if (lan9645x_vlan_wait_for_completion(lan9645x))
+ dev_err(lan9645x->dev, "Vlan clear table failed\n");
+
+ for (vid = 1; vid < VLAN_N_VID; vid++)
+ lan9645x_vlan_hw_wr(lan9645x, vid);
+
+ /* Set all the ports + cpu to be part of HOST_PVID and UNAWARE_PVID */
+ lan9645x->vlans[HOST_PVID].portmask = all_ports;
+ lan9645x_vlan_hw_wr(lan9645x, HOST_PVID);
+
+ lan9645x->vlans[UNAWARE_PVID].portmask = all_ports;
+ lan9645x_vlan_hw_wr(lan9645x, UNAWARE_PVID);
+
+ /* Configure the CPU port to be vlan aware */
+ lan_wr(ANA_VLAN_CFG_VLAN_VID_SET(UNAWARE_PVID) |
+ ANA_VLAN_CFG_VLAN_AWARE_ENA_SET(1) |
+ ANA_VLAN_CFG_VLAN_POP_CNT_SET(1),
+ lan9645x, ANA_VLAN_CFG(CPU_PORT));
+
+ /* Set vlan ingress filter mask to all ports */
+ lan_wr(all_ports, lan9645x, ANA_VLANMASK);
+
+ for (port = 0; port < lan9645x->num_phys_ports; port++) {
+ lan_wr(0, lan9645x, REW_PORT_VLAN_CFG(port));
+ lan_wr(0, lan9645x, REW_TAG_CFG(port));
+ }
+}
--
2.52.0
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add vlan support
Add support for vlanaware bridge. We reserve vid 4095 for standalone
mode, to implement fdb-isolation. A vlan-unaware bridge uses vid 0.
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 599e589c4ec3..046c95a72242 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -535,11 +536,54 @@ static void lan9645x_port_bridge_leave(struct dsa_switch *ds, int port,
[ ... ]
> +static int lan9645x_port_vlan_filtering(struct dsa_switch *ds, int port,
> + bool enabled,
> + struct netlink_ext_ack *extack)
> +{
> + struct lan9645x *lan9645x = ds->priv;
> + struct lan9645x_port *p;
> +
> + p = lan9645x_to_port(lan9645x, port);
> + p->vlan_aware = enabled;
> + lan9645x_vlan_port_apply(p);
In other places, the driver avoids calling egress functions on the npi
port because it conflicts with cpu port header extraction. Since this
unconditionally applies the vlan configuration to both ingress and egress,
could this overwrite the npi port's configuration when the state is
synchronized?
> +
> + return 0;
> +}
> +
> +static int lan9645x_port_vlan_add(struct dsa_switch *ds, int port,
[ ... ]
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
> new file mode 100644
> index 000000000000..0459932a626c
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
[ ... ]
> +static void lan9645x_vlan_hw_wr(struct lan9645x *lan9645x, u16 vid)
> +{
> + struct lan9645x_vlan *v = &lan9645x->vlans[vid];
> + bool cpu_dis = !(v->portmask & BIT(CPU_PORT));
> + u32 val;
> +
> + val = ANA_VLANTIDX_VLAN_PGID_CPU_DIS_SET(cpu_dis) |
> + ANA_VLANTIDX_V_INDEX_SET(vid) |
> + ANA_VLANTIDX_VLAN_SEC_FWD_ENA_SET(v->s_fwd_ena) |
> + ANA_VLANTIDX_VLAN_FLOOD_DIS_SET(v->fld_dis) |
> + ANA_VLANTIDX_VLAN_PRIV_VLAN_SET(v->prv_vlan) |
> + ANA_VLANTIDX_VLAN_LEARN_DISABLED_SET(v->lrn_dis) |
> + ANA_VLANTIDX_VLAN_MIRROR_SET(v->mir) |
> + ANA_VLANTIDX_VLAN_SRC_CHK_SET(v->src_chk);
> +
> + lan_wr(val, lan9645x, ANA_VLANTIDX);
> + lan_wr(v->portmask, lan9645x, ANA_VLAN_PORT_MASK);
> + lan_wr(VLANACCESS_CMD_WRITE, lan9645x, ANA_VLANACCESS);
Are these registers written with raw values rather than using their respective
set macros? This circumvents the register abstraction and might risk failures
if the hardware layout changes.
> +
> + if (lan9645x_vlan_wait_for_completion(lan9645x))
> + dev_err(lan9645x->dev, "Vlan set mask failed\n");
> +}
[ ... ]
> +static void
> +lan9645x_vlan_port_apply_egress(struct lan9645x_port *p,
> + struct lan9645x_vlan_port_info *info)
> +{
[ ... ]
> + lan_rmw(REW_TAG_CFG_TAG_TPID_CFG_SET(3) |
> + REW_TAG_CFG_TAG_CFG_SET(tag_cfg),
> + REW_TAG_CFG_TAG_TPID_CFG |
> + REW_TAG_CFG_TAG_CFG,
> + lan9645x, REW_TAG_CFG(p->chip_port));
> +
> + lan_rmw(REW_PORT_VLAN_CFG_PORT_TPID_SET(ETH_P_8021AD) |
> + REW_PORT_VLAN_CFG_PORT_VID_SET(port_vid),
> + REW_PORT_VLAN_CFG_PORT_TPID |
> + REW_PORT_VLAN_CFG_PORT_VID,
> + lan9645x, REW_PORT_VLAN_CFG(p->chip_port));
> +}
Setting the port tpid to 0x88a8 causes untagged ingress frames to egress trunk
ports with 802.1ad tags. Could this break interoperability with standard
networks that expect 0x8100 tags?
> +static void lan9645x_vlan_port_apply_ingress(struct lan9645x_port *p)
> +{
[ ... ]
> + /* Drop frames with multicast source address */
> + val = ANA_DROP_CFG_DROP_MC_SMAC_ENA_SET(1);
> + if (p->vlan_aware && !pvid)
> + /* If port is vlan-aware and tagged, drop untagged and priority
> + * tagged frames.
> + */
> + val |= ANA_DROP_CFG_DROP_UNTAGGED_ENA_SET(1) |
> + ANA_DROP_CFG_DROP_PRIO_S_TAGGED_ENA_SET(1) |
> + ANA_DROP_CFG_DROP_PRIO_C_TAGGED_ENA_SET(1);
> +
> + lan_wr(val, lan9645x, ANA_DROP_CFG(p->chip_port));
> +}
Does this overwrite the entire 32-bit register and silently clear other
initialized bitfields? Would it be safer to use a read-modify-write here
instead?
[ ... ]
> +static int lan9645x_vlan_cpu_add(struct lan9645x_port *p, u16 vid, bool pvid,
> + bool untagged)
> +{
> + struct lan9645x_vlan *v;
> +
> + v = lan9645x_vlan_port_modify(p, vid, pvid, untagged);
Is it intentional that this bypasses the bounds check on the vlan id?
> + v->portmask |= BIT(CPU_PORT) | BIT(p->chip_port);
> + lan9645x_vlan_hw_wr(p->lan9645x, vid);
> + lan9645x_vlan_port_apply_ingress(p);
> +
> + return 0;
> +}
> +
> +int lan9645x_vlan_port_add_vlan(struct lan9645x_port *p, u16 vid, bool pvid,
> + bool untagged, struct netlink_ext_ack *extack)
> +{
[ ... ]
> +void lan9645x_vlan_port_del_vlan(struct lan9645x_port *p, u16 vid)
> +{
> + struct lan9645x *lan9645x = p->lan9645x;
> + struct lan9645x_vlan *v;
> +
> + if (!vid)
> + return;
> +
> + v = lan9645x_vlan_port_modify(p, vid, false, false);
There is a check for the maximum vlan id when adding a vlan, but no bounds
check when deleting one. Could userspace use this to delete the reserved
vid 4095 and break standalone forwarding?
> + v->portmask &= ~BIT(p->chip_port);
> +
> + if (p->chip_port == lan9645x->npi)
> + v->portmask &= ~BIT(CPU_PORT);
> +
> + lan9645x_vlan_hw_wr(lan9645x, vid);
[ ... ]
> +void lan9645x_vlan_init(struct lan9645x *lan9645x)
> +{
[ ... ]
> + for (vid = 1; vid < VLAN_N_VID; vid++)
> + lan9645x_vlan_hw_wr(lan9645x, vid);
If the indirect table access times out, the write function prints an error
but does not return a status code. Will this cause the initialization loop
to stall for the full timeout duration 4095 consecutive times if the
hardware is unresponsive?
> +
> + /* Set all the ports + cpu to be part of HOST_PVID and UNAWARE_PVID */
> + lan9645x->vlans[HOST_PVID].portmask = all_ports;
On Sun, 2026-03-29 at 12:56 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: dsa: lan9645x: add vlan support
>
> Add support for vlanaware bridge. We reserve vid 4095 for standalone
> mode, to implement fdb-isolation. A vlan-unaware bridge uses vid 0.
>
> > diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> > index 599e589c4ec3..046c95a72242 100644
> > --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> > +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> [ ... ]
> > @@ -535,11 +536,54 @@ static void lan9645x_port_bridge_leave(struct dsa_switch *ds, int port,
> [ ... ]
> > +static int lan9645x_port_vlan_filtering(struct dsa_switch *ds, int port,
> > + bool enabled,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct lan9645x *lan9645x = ds->priv;
> > + struct lan9645x_port *p;
> > +
> > + p = lan9645x_to_port(lan9645x, port);
> > + p->vlan_aware = enabled;
> > + lan9645x_vlan_port_apply(p);
>
> In other places, the driver avoids calling egress functions on the npi
> port because it conflicts with cpu port header extraction. Since this
> unconditionally applies the vlan configuration to both ingress and egress,
> could this overwrite the npi port's configuration when the state is
> synchronized?
>
It is true we avoid this, but the vlan_filtering is not called by DSA core for
the cpu port, only user ports.
> > +
> > + return 0;
> > +}
> > +
> > +static int lan9645x_port_vlan_add(struct dsa_switch *ds, int port,
> [ ... ]
> > diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
> > new file mode 100644
> > index 000000000000..0459932a626c
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
> [ ... ]
> > +static void lan9645x_vlan_hw_wr(struct lan9645x *lan9645x, u16 vid)
> > +{
> > + struct lan9645x_vlan *v = &lan9645x->vlans[vid];
> > + bool cpu_dis = !(v->portmask & BIT(CPU_PORT));
> > + u32 val;
> > +
> > + val = ANA_VLANTIDX_VLAN_PGID_CPU_DIS_SET(cpu_dis) |
> > + ANA_VLANTIDX_V_INDEX_SET(vid) |
> > + ANA_VLANTIDX_VLAN_SEC_FWD_ENA_SET(v->s_fwd_ena) |
> > + ANA_VLANTIDX_VLAN_FLOOD_DIS_SET(v->fld_dis) |
> > + ANA_VLANTIDX_VLAN_PRIV_VLAN_SET(v->prv_vlan) |
> > + ANA_VLANTIDX_VLAN_LEARN_DISABLED_SET(v->lrn_dis) |
> > + ANA_VLANTIDX_VLAN_MIRROR_SET(v->mir) |
> > + ANA_VLANTIDX_VLAN_SRC_CHK_SET(v->src_chk);
> > +
> > + lan_wr(val, lan9645x, ANA_VLANTIDX);
> > + lan_wr(v->portmask, lan9645x, ANA_VLAN_PORT_MASK);
> > + lan_wr(VLANACCESS_CMD_WRITE, lan9645x, ANA_VLANACCESS);
>
> Are these registers written with raw values rather than using their respective
> set macros? This circumvents the register abstraction and might risk failures
> if the hardware layout changes.
>
Yes, they are single field registers. I can add the macro.
> > +
> > + if (lan9645x_vlan_wait_for_completion(lan9645x))
> > + dev_err(lan9645x->dev, "Vlan set mask failed\n");
> > +}
> [ ... ]
> > +static void
> > +lan9645x_vlan_port_apply_egress(struct lan9645x_port *p,
> > + struct lan9645x_vlan_port_info *info)
> > +{
> [ ... ]
> > + lan_rmw(REW_TAG_CFG_TAG_TPID_CFG_SET(3) |
> > + REW_TAG_CFG_TAG_CFG_SET(tag_cfg),
> > + REW_TAG_CFG_TAG_TPID_CFG |
> > + REW_TAG_CFG_TAG_CFG,
> > + lan9645x, REW_TAG_CFG(p->chip_port));
> > +
> > + lan_rmw(REW_PORT_VLAN_CFG_PORT_TPID_SET(ETH_P_8021AD) |
> > + REW_PORT_VLAN_CFG_PORT_VID_SET(port_vid),
> > + REW_PORT_VLAN_CFG_PORT_TPID |
> > + REW_PORT_VLAN_CFG_PORT_VID,
> > + lan9645x, REW_PORT_VLAN_CFG(p->chip_port));
> > +}
>
> Setting the port tpid to 0x88a8 causes untagged ingress frames to egress trunk
> ports with 802.1ad tags. Could this break interoperability with standard
> networks that expect 0x8100 tags?
>
No that is not how it works. I think the comment which is left out explains this
clearly. 0x8100 tags are unaffected by this, but it means an stag is not rewritten
to a ctag.
> > +static void lan9645x_vlan_port_apply_ingress(struct lan9645x_port *p)
> > +{
> [ ... ]
> > + /* Drop frames with multicast source address */
> > + val = ANA_DROP_CFG_DROP_MC_SMAC_ENA_SET(1);
> > + if (p->vlan_aware && !pvid)
> > + /* If port is vlan-aware and tagged, drop untagged and priority
> > + * tagged frames.
> > + */
> > + val |= ANA_DROP_CFG_DROP_UNTAGGED_ENA_SET(1) |
> > + ANA_DROP_CFG_DROP_PRIO_S_TAGGED_ENA_SET(1) |
> > + ANA_DROP_CFG_DROP_PRIO_C_TAGGED_ENA_SET(1);
> > +
> > + lan_wr(val, lan9645x, ANA_DROP_CFG(p->chip_port));
> > +}
>
> Does this overwrite the entire 32-bit register and silently clear other
> initialized bitfields? Would it be safer to use a read-modify-write here
> instead?
>
Yes this is intentional. I prefer this over writing all fields with rmw which is
slower.
> [ ... ]
> > +static int lan9645x_vlan_cpu_add(struct lan9645x_port *p, u16 vid, bool pvid,
> > + bool untagged)
> > +{
> > + struct lan9645x_vlan *v;
> > +
> > + v = lan9645x_vlan_port_modify(p, vid, pvid, untagged);
>
> Is it intentional that this bypasses the bounds check on the vlan id?
>
No, I will move the cpu action below the bounds check.
> > + v->portmask |= BIT(CPU_PORT) | BIT(p->chip_port);
> > + lan9645x_vlan_hw_wr(p->lan9645x, vid);
> > + lan9645x_vlan_port_apply_ingress(p);
> > +
> > + return 0;
> > +}
> > +
> > +int lan9645x_vlan_port_add_vlan(struct lan9645x_port *p, u16 vid, bool pvid,
> > + bool untagged, struct netlink_ext_ack *extack)
> > +{
> [ ... ]
> > +void lan9645x_vlan_port_del_vlan(struct lan9645x_port *p, u16 vid)
> > +{
> > + struct lan9645x *lan9645x = p->lan9645x;
> > + struct lan9645x_vlan *v;
> > +
> > + if (!vid)
> > + return;
> > +
> > + v = lan9645x_vlan_port_modify(p, vid, false, false);
>
> There is a check for the maximum vlan id when adding a vlan, but no bounds
> check when deleting one. Could userspace use this to delete the reserved
> vid 4095 and break standalone forwarding?
>
I will add the bounds check for delete.
> > + v->portmask &= ~BIT(p->chip_port);
> > +
> > + if (p->chip_port == lan9645x->npi)
> > + v->portmask &= ~BIT(CPU_PORT);
> > +
> > + lan9645x_vlan_hw_wr(lan9645x, vid);
> [ ... ]
> > +void lan9645x_vlan_init(struct lan9645x *lan9645x)
> > +{
> [ ... ]
> > + for (vid = 1; vid < VLAN_N_VID; vid++)
> > + lan9645x_vlan_hw_wr(lan9645x, vid);
>
> If the indirect table access times out, the write function prints an error
> but does not return a status code. Will this cause the initialization loop
> to stall for the full timeout duration 4095 consecutive times if the
> hardware is unresponsive?
>
Yes, I can return the err from vlan_hw_wr and return it early from vlan_init.
> > +
> > + /* Set all the ports + cpu to be part of HOST_PVID and UNAWARE_PVID */
> > + lan9645x->vlans[HOST_PVID].portmask = all_ports;
© 2016 - 2026 Red Hat, Inc.