| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250527103749.66505756@2a02-8440-d111-2026-8d50-1f4f-0da2-e170.rev.sfr.net>
Date: Tue, 27 May 2025 10:37:49 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Horatiu Vultur <horatiu.vultur@...rochip.com>
Cc: <UNGLinuxDriver@...rochip.com>, <andrew+netdev@...n.ch>,
<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] net: lan966x: Make sure to insert the vlan tags
also in host mode
Hello Horatiu,
On Tue, 27 May 2025 09:08:50 +0200
Horatiu Vultur <horatiu.vultur@...rochip.com> wrote:
> When running these commands on DUT (and similar at the other end)
> ip link set dev eth0 up
> ip link add link eth0 name eth0.10 type vlan id 10
> ip addr add 10.0.0.1/24 dev eth0.10
> ip link set dev eth0.10 up
> ping 10.0.0.2/24
>
> The ping will fail.
>
> The reason why is failing is because, the network interfaces for lan966x
> have a flag saying that the HW can insert the vlan tags into the
> frames(NETIF_F_HW_VLAN_CTAG_TX). Meaning that the frames that are
> transmitted don't have the vlan tag inside the skb data, but they have
> it inside the skb. We already get that vlan tag and put it in the IFH
> but the problem is that we don't configure the HW to rewrite the frame
> when the interface is in host mode.
> The fix consists in actually configuring the HW to insert the vlan tag
> if it is different than 0.
>
> Fixes: 6d2c186afa5d ("net: lan966x: Add vlan support.")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> ---
> .../ethernet/microchip/lan966x/lan966x_main.c | 1 +
> .../ethernet/microchip/lan966x/lan966x_main.h | 1 +
> .../microchip/lan966x/lan966x_switchdev.c | 1 +
> .../ethernet/microchip/lan966x/lan966x_vlan.c | 21 +++++++++++++++++++
> 4 files changed, 24 insertions(+)
>
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index 427bdc0e4908c..7001584f1b7a6 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> @@ -879,6 +879,7 @@ static int lan966x_probe_port(struct lan966x *lan966x, u32 p,
> lan966x_vlan_port_set_vlan_aware(port, 0);
> lan966x_vlan_port_set_vid(port, HOST_PVID, false, false);
> lan966x_vlan_port_apply(port);
> + lan966x_vlan_port_rew_host(port);
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> index 1f9df67f05044..4f75f06883693 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> @@ -497,6 +497,7 @@ void lan966x_vlan_port_apply(struct lan966x_port *port);
> bool lan966x_vlan_cpu_member_cpu_vlan_mask(struct lan966x *lan966x, u16 vid);
> void lan966x_vlan_port_set_vlan_aware(struct lan966x_port *port,
> bool vlan_aware);
> +void lan966x_vlan_port_rew_host(struct lan966x_port *port);
> int lan966x_vlan_port_set_vid(struct lan966x_port *port,
> u16 vid,
> bool pvid,
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> index 1c88120eb291a..bcb4db76b75cd 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> @@ -297,6 +297,7 @@ static void lan966x_port_bridge_leave(struct lan966x_port *port,
> lan966x_vlan_port_set_vlan_aware(port, false);
> lan966x_vlan_port_set_vid(port, HOST_PVID, false, false);
> lan966x_vlan_port_apply(port);
> + lan966x_vlan_port_rew_host(port);
> }
>
> int lan966x_port_changeupper(struct net_device *dev,
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> index fa34a739c748e..f158ec6ab10cc 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> @@ -149,6 +149,27 @@ void lan966x_vlan_port_set_vlan_aware(struct lan966x_port *port,
> port->vlan_aware = vlan_aware;
> }
>
> +/* When the interface is in host mode, the interface should not be vlan aware
> + * but it should insert all the tags that it gets from the network stack.
> + * The tags are no in the data of the frame but actually in the skb and the ifh
not
> + * is confiured already to get this tag. So what we need to do is to update the
configured
> + * rewriter to insert the vlan tag for all frames which have a vlan tag
> + * different than 0.
Just to be extra clear, the doc seems to say that
REW_TAG_CFG_TAG_CFG_SET(1);
means "Tag all frames, except when VID=PORT_VLAN_CFG.PORT_VID or
VID=0."
Another setting for these bits are "Tag all frames except when VID=0",
which is what you document in the above comment.
In this case, is there any chance that it would make a difference ?
> + */
> +void lan966x_vlan_port_rew_host(struct lan966x_port *port)
> +{
> + struct lan966x *lan966x = port->lan966x;
> + u32 val;
> +
> + /* Tag all frames except when VID == DEFAULT_VLAN */
> + val = REW_TAG_CFG_TAG_CFG_SET(1);
> +
> + /* Update only some bits in the register */
> + lan_rmw(val,
> + REW_TAG_CFG_TAG_CFG,
> + lan966x, REW_TAG_CFG(port->chip_port));
> +}
> +
> void lan966x_vlan_port_apply(struct lan966x_port *port)
> {
> struct lan966x *lan966x = port->lan966x;
Sorry for the typo nitpicking, but with that :
Reviewed-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
Maxime
Powered by blists - more mailing lists