| 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: <6209405e-7100-43f9-b415-3be8fbcc6352@blackwall.org>
Date: Mon, 14 Oct 2024 09:22:07 +0300
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Eric Woudstra <ericwouds@...il.com>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Pablo Neira Ayuso <pablo@...filter.org>,
Jozsef Kadlecsik <kadlec@...filter.org>, Roopa Prabhu <roopa@...dia.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Jiri Pirko <jiri@...nulli.us>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Lorenzo Bianconi <lorenzo@...nel.org>,
Frank Wunderlich <frank-w@...lic-files.de>,
Daniel Golle <daniel@...rotopia.org>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
netfilter-devel@...r.kernel.org, coreteam@...filter.org,
bridge@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH RFC v1 net-next 11/12] bridge:
br_vlan_fill_forward_path_mode no _UNTAG_HW for dsa
On 14/10/2024 09:18, Nikolay Aleksandrov wrote:
> 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@...il.com>
>> ---
>> 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.
>
you can use an internal bridge opt (check br_private.h) with a private opt
that's set when such device is added as a port, no need for a full counter
obviously
>> + 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;
>
Powered by blists - more mailing lists