Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jens Emil Schulz Østergaard" <jensemil.schulzostergaard@microchip.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
Subject: Re: [PATCH net-next v6 6/9] net: dsa: lan9645x: add vlan support
Date: Sat, 30 May 2026 00:50:13 +0000	[thread overview]
Message-ID: <20260530005014.503F01F0089A@smtp.kernel.org> (raw)
In-Reply-To: <20260527-dsa_lan9645x_switch_driver_base-v6-6-4d409ae64f3c@microchip.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] lan9645x_port_vlan_filtering clobbers the CPU (NPI) port's egress tagging configuration
--

commit 6fffecbf30e0b6af9ee79a8cd06c0280fc266991
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>

    net: dsa: lan9645x: add vlan support
    
    Add VLAN support for VLAN-aware and VLAN-unaware bridges.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 9d8f3a1b335bd..eb13dfadbd3cf 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -559,11 +563,52 @@ 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);

[Severity: High]
Does this code inadvertently clobber the CPU (NPI) port's egress tagging
configuration?

When VLAN filtering is toggled on a user bridge port, the DSA core
synchronizes this state to the CPU port by calling port_vlan_filtering().
Here, the driver unconditionally delegates to lan9645x_vlan_port_apply(),
which executes both ingress and egress VLAN application:

lan9645x_vlan_port_apply() {
        lan9645x_vlan_port_apply_ingress(p);
        lan9645x_vlan_port_apply_egress(p, NULL);
}

Because the CPU port is typically non-bridged,
lan9645x_vlan_port_apply_egress() will overwrite the REW_TAG_CFG register
with LAN9645X_TAG_NO_PVID_NO_UNAWARE:

lan9645x_vlan_port_apply_egress() {
        ...
        if (!lan9645x_port_is_bridged(p)) {
                tag_cfg = LAN9645X_TAG_NO_PVID_NO_UNAWARE;
        ...
}

The driver correctly avoids applying egress tagging to the CPU port in other
places. For example, lan9645x_port_setup() explicitly skips
lan9645x_vlan_set_hostmode() for the NPI port, and
lan9645x_vlan_cpu_add/del() only update ingress state.

Without a similar guard (such as checking p->chip_port != lan9645x->npi) in
this function, toggling VLAN filtering might push unexpected 802.1Q tags
onto frames extracted to the host, breaking DSA tag parsing and software
forwarding.

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-dsa_lan9645x_switch_driver_base-v6-0-4d409ae64f3c@microchip.com?part=6

  reply	other threads:[~2026-05-30  0:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27 14:49 [PATCH net-next v6 0/9] net: dsa: add DSA support for the LAN9645x switch chip family Jens Emil Schulz Østergaard
2026-05-27 14:49 ` [PATCH net-next v6 1/9] net: dsa: add tag driver for LAN9645X Jens Emil Schulz Østergaard
2026-05-27 15:39   ` Jonas Gorski
2026-05-28 10:45     ` Jens Emil Schulz Ostergaard
2026-05-30  0:50   ` sashiko-bot
2026-05-27 14:49 ` [PATCH net-next v6 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings Jens Emil Schulz Østergaard
2026-05-27 14:49 ` [PATCH net-next v6 3/9] net: dsa: lan9645x: add autogenerated register macros Jens Emil Schulz Østergaard
2026-05-27 14:49 ` [PATCH net-next v6 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X Jens Emil Schulz Østergaard
2026-05-30  0:50   ` sashiko-bot
2026-05-27 14:49 ` [PATCH net-next v6 5/9] net: dsa: lan9645x: add bridge support Jens Emil Schulz Østergaard
2026-05-30  0:50   ` sashiko-bot
2026-05-27 14:49 ` [PATCH net-next v6 6/9] net: dsa: lan9645x: add vlan support Jens Emil Schulz Østergaard
2026-05-30  0:50   ` sashiko-bot [this message]
2026-05-27 14:49 ` [PATCH net-next v6 7/9] net: dsa: lan9645x: add mac table integration Jens Emil Schulz Østergaard
2026-05-30  0:50   ` sashiko-bot
2026-05-27 14:49 ` [PATCH net-next v6 8/9] net: dsa: lan9645x: add mdb management Jens Emil Schulz Østergaard
2026-05-30  0:50   ` sashiko-bot
2026-05-27 14:49 ` [PATCH net-next v6 9/9] net: dsa: lan9645x: add port statistics Jens Emil Schulz Østergaard
2026-05-30  0:50   ` sashiko-bot

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=20260530005014.503F01F0089A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jensemil.schulzostergaard@microchip.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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