All of lore.kernel.org
 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 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.