On 13/10/2024 21:55, Eric Woudstra wrote:
> In network setup as below:
>
> fastpath bypass
> .----------------------------------------.
> / \
> | IP - forwarding |
> | / \ v
> | / wan ...
> | /
> | |
> | |
> | brlan.1
> | |
> | +-------------------------------+
> | | vlan 1 |
> | | |
> | | brlan (vlan-filtering) |
> | | +---------------+
> | | | DSA-SWITCH |
> | | vlan 1 | |
> | | to | |
> | | untagged 1 vlan 1 |
> | +---------------+---------------+
> . / \
> ----->wlan1 lan0
> . .
> . ^
> ^ vlan 1 tagged packets
> untagged packets
>
> Now that DEV_PATH_MTK_WDMA is added to nft_dev_path_info() the forward
> path is filled also when ending with the mediatek wlan1, info.indev not
> NULL now in nft_dev_forward_path(). This results in a direct transmit
> instead of a neighbor transmit. This is how it should be, But this fails.
>
> br_vlan_fill_forward_path_mode() sets DEV_PATH_BR_VLAN_UNTAG_HW when
> filling in from brlan.1 towards wlan1. But it should be set to
> DEV_PATH_BR_VLAN_UNTAG in this case. Using BR_VLFLAG_ADDED_BY_SWITCHDEV
> is not correct. The dsa switchdev adds it as a foreign port.
>
> Use BR_VLFLAG_TAGGING_BY_SWITCHDEV to make sure DEV_PATH_BR_VLAN_UNTAG is
> set when there is a dsa-switch inside the bridge.
>
> Signed-off-by: Eric Woudstra <ericwouds@xxxxxxxxx>
> ---
> net/bridge/br_private.h | 1 +
> net/bridge/br_vlan.c | 18 +++++++++++++++++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 8da7798f9368..7d427214cc7c 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -180,6 +180,7 @@ enum {
> BR_VLFLAG_MCAST_ENABLED = BIT(2),
> BR_VLFLAG_GLOBAL_MCAST_ENABLED = BIT(3),
> BR_VLFLAG_NEIGH_SUPPRESS_ENABLED = BIT(4),
> + BR_VLFLAG_TAGGING_BY_SWITCHDEV = BIT(5),
> };
>
> /**
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 1830d7d617cd..b7877724b969 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -3,6 +3,7 @@
> #include <linux/netdevice.h>
> #include <linux/rtnetlink.h>
> #include <linux/slab.h>
> +#include <net/dsa.h>
> #include <net/switchdev.h>
>
> #include "br_private.h"
> @@ -100,6 +101,19 @@ static void __vlan_flags_commit(struct net_bridge_vlan *v, u16 flags)
> __vlan_flags_update(v, flags, true);
> }
>
> +static inline bool br_vlan_tagging_by_switchdev(struct net_bridge *br)
no inline in .c files and also constify br
> +{
> +#if IS_ENABLED(CONFIG_NET_DSA)
> + struct net_bridge_port *p;
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + if (dsa_user_dev_check(p->dev))
I don't think this can change at runtime, so please keep a counter in
the bridge and don't walk the port list on every vlan add.
> + return false;
> + }
> +#endif
> + return true;
> +}
> +
> static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
> struct net_bridge_vlan *v, u16 flags,
> struct netlink_ext_ack *extack)
> @@ -113,6 +127,8 @@ static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
> if (err == -EOPNOTSUPP)
> return vlan_vid_add(dev, br->vlan_proto, v->vid);
> v->priv_flags |= BR_VLFLAG_ADDED_BY_SWITCHDEV;
> + if (br_vlan_tagging_by_switchdev(br))
> + v->priv_flags |= BR_VLFLAG_TAGGING_BY_SWITCHDEV;
> return err;
> }
>
> @@ -1491,7 +1507,7 @@ int br_vlan_fill_forward_path_mode(struct net_bridge *br,
>
> if (path->bridge.vlan_mode == DEV_PATH_BR_VLAN_TAG)
> path->bridge.vlan_mode = DEV_PATH_BR_VLAN_KEEP;
> - else if (v->priv_flags & BR_VLFLAG_ADDED_BY_SWITCHDEV)
> + else if (v->priv_flags & BR_VLFLAG_TAGGING_BY_SWITCHDEV)
> path->bridge.vlan_mode = DEV_PATH_BR_VLAN_UNTAG_HW;
> else
> path->bridge.vlan_mode = DEV_PATH_BR_VLAN_UNTAG;
