From: Hans Schultz <schultz.hans@gmail.com>
To: Nikolay Aleksandrov <razor@blackwall.org>,
Hans Schultz <schultz.hans@gmail.com>,
davem@davemloft.net, kuba@kernel.org
Cc: Ivan Vecera <ivecera@redhat.com>, Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Jiri Pirko <jiri@resnulli.us>,
Daniel Borkmann <daniel@iogearbox.net>,
netdev@vger.kernel.org, Ido Schimmel <idosch@nvidia.com>,
bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
Eric Dumazet <edumazet@google.com>,
linux-kselftest@vger.kernel.org, Roopa Prabhu <roopa@nvidia.com>,
Paolo Abeni <pabeni@redhat.com>,
Vladimir Oltean <olteanv@gmail.com>,
Shuah Khan <shuah@kernel.org>,
Vivien Didelot <vivien.didelot@gmail.com>
Subject: Re: [Bridge] [PATCH V3 net-next 1/4] net: bridge: add fdb flag to extent locked port feature
Date: Tue, 24 May 2022 18:21:02 +0200 [thread overview]
Message-ID: <86zgj6oqa9.fsf@gmail.com> (raw)
In-Reply-To: <01e6e35c-f5c9-9776-1263-058f84014ed9@blackwall.org>
>
> Hi Hans,
> So this approach has a fundamental problem, f->dst is changed without any synchronization
> you cannot rely on it and thus you cannot account for these entries properly. We must be very
> careful if we try to add any new synchronization not to affect performance as well.
> More below...
>
>> @@ -319,6 +326,9 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f,
>> if (test_bit(BR_FDB_STATIC, &f->flags))
>> fdb_del_hw_addr(br, f->key.addr.addr);
>>
>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &f->flags) && !test_bit(BR_FDB_OFFLOADED, &f->flags))
>> + atomic_dec(&f->dst->locked_entry_cnt);
>
> Sorry but you cannot do this for multiple reasons:
> - f->dst can be NULL
> - f->dst changes without any synchronization
> - there is no synchronization between fdb's flags and its ->dst
>
> Cheers,
> Nik
Hi Nik,
if a port is decoupled from the bridge, the locked entries would of
course be invalid, so maybe if adding and removing a port is accounted
for wrt locked entries and the count of locked entries, would that not
work?
Best,
Hans
WARNING: multiple messages have this Message-ID (diff)
From: Hans Schultz <schultz.hans@gmail.com>
To: Nikolay Aleksandrov <razor@blackwall.org>,
Hans Schultz <schultz.hans@gmail.com>,
davem@davemloft.net, kuba@kernel.org
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>, Jiri Pirko <jiri@resnulli.us>,
Ivan Vecera <ivecera@redhat.com>, Roopa Prabhu <roopa@nvidia.com>,
Shuah Khan <shuah@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Ido Schimmel <idosch@nvidia.com>,
linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH V3 net-next 1/4] net: bridge: add fdb flag to extent locked port feature
Date: Tue, 24 May 2022 18:21:02 +0200 [thread overview]
Message-ID: <86zgj6oqa9.fsf@gmail.com> (raw)
In-Reply-To: <01e6e35c-f5c9-9776-1263-058f84014ed9@blackwall.org>
>
> Hi Hans,
> So this approach has a fundamental problem, f->dst is changed without any synchronization
> you cannot rely on it and thus you cannot account for these entries properly. We must be very
> careful if we try to add any new synchronization not to affect performance as well.
> More below...
>
>> @@ -319,6 +326,9 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f,
>> if (test_bit(BR_FDB_STATIC, &f->flags))
>> fdb_del_hw_addr(br, f->key.addr.addr);
>>
>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &f->flags) && !test_bit(BR_FDB_OFFLOADED, &f->flags))
>> + atomic_dec(&f->dst->locked_entry_cnt);
>
> Sorry but you cannot do this for multiple reasons:
> - f->dst can be NULL
> - f->dst changes without any synchronization
> - there is no synchronization between fdb's flags and its ->dst
>
> Cheers,
> Nik
Hi Nik,
if a port is decoupled from the bridge, the locked entries would of
course be invalid, so maybe if adding and removing a port is accounted
for wrt locked entries and the count of locked entries, would that not
work?
Best,
Hans
next prev parent reply other threads:[~2022-05-24 16:21 UTC|newest]
Thread overview: 108+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-24 15:21 [Bridge] [PATCH V3 net-next 0/4] Extend locked port feature with FDB locked flag (MAC-Auth/MAB) Hans Schultz
2022-05-24 15:21 ` Hans Schultz
2022-05-24 15:21 ` [Bridge] [PATCH V3 net-next 1/4] net: bridge: add fdb flag to extent locked port feature Hans Schultz
2022-05-24 15:21 ` Hans Schultz
2022-05-24 15:39 ` [Bridge] " Nikolay Aleksandrov
2022-05-24 15:39 ` Nikolay Aleksandrov
2022-05-24 16:08 ` [Bridge] " Hans Schultz
2022-05-24 16:08 ` Hans Schultz
2022-05-24 16:21 ` Hans Schultz [this message]
2022-05-24 16:21 ` Hans Schultz
2022-05-25 8:06 ` [Bridge] " Nikolay Aleksandrov
2022-05-25 8:06 ` Nikolay Aleksandrov
2022-05-25 8:34 ` [Bridge] " Hans Schultz
2022-05-25 8:34 ` Hans Schultz
2022-05-25 8:38 ` [Bridge] " Nikolay Aleksandrov
2022-05-25 8:38 ` Nikolay Aleksandrov
2022-05-25 9:11 ` [Bridge] " Hans Schultz
2022-05-25 9:11 ` Hans Schultz
2022-05-25 10:18 ` [Bridge] " Nikolay Aleksandrov
2022-05-25 10:18 ` Nikolay Aleksandrov
2022-07-06 18:13 ` [Bridge] " Vladimir Oltean
2022-07-06 18:13 ` Vladimir Oltean
2022-07-06 19:38 ` [Bridge] " Nikolay Aleksandrov
2022-07-06 19:38 ` Nikolay Aleksandrov
2022-07-06 20:21 ` [Bridge] " Vladimir Oltean
2022-07-06 20:21 ` Vladimir Oltean
2022-07-06 21:01 ` [Bridge] " Nikolay Aleksandrov
2022-07-06 21:01 ` Nikolay Aleksandrov
2022-07-07 14:08 ` [Bridge] " Nikolay Aleksandrov
2022-07-07 14:08 ` Nikolay Aleksandrov
2022-07-07 17:15 ` [Bridge] " Vladimir Oltean
2022-07-07 17:15 ` Vladimir Oltean
2022-07-07 17:26 ` [Bridge] " Nikolay Aleksandrov
2022-07-07 17:26 ` Nikolay Aleksandrov
2022-07-08 6:38 ` [Bridge] " Hans S
2022-07-08 6:38 ` Hans S
2022-05-26 14:13 ` [Bridge] " Ido Schimmel
2022-05-26 14:13 ` Ido Schimmel
2022-05-27 8:52 ` [Bridge] " Hans Schultz
2022-05-27 8:52 ` Hans Schultz
2022-05-27 9:58 ` [Bridge] " Ido Schimmel
2022-05-27 9:58 ` Ido Schimmel
2022-05-27 16:00 ` [Bridge] " Hans Schultz
2022-05-27 16:00 ` Hans Schultz
2022-05-31 9:34 ` [Bridge] " Hans Schultz
2022-05-31 9:34 ` Hans Schultz
2022-05-31 14:23 ` [Bridge] " Ido Schimmel
2022-05-31 14:23 ` Ido Schimmel
2022-05-31 15:49 ` [Bridge] " Hans Schultz
2022-05-31 15:49 ` Hans Schultz
2022-06-02 9:17 ` [Bridge] " Hans Schultz
2022-06-02 9:17 ` Hans Schultz
2022-06-02 9:33 ` [Bridge] " Nikolay Aleksandrov
2022-06-02 9:33 ` Nikolay Aleksandrov
2022-06-02 10:17 ` [Bridge] " Hans Schultz
2022-06-02 10:17 ` Hans Schultz
2022-06-02 10:30 ` [Bridge] " Nikolay Aleksandrov
2022-06-02 10:30 ` Nikolay Aleksandrov
2022-06-02 10:39 ` [Bridge] " Ido Schimmel
2022-06-02 10:39 ` Ido Schimmel
2022-06-02 11:36 ` [Bridge] " Hans Schultz
2022-06-02 11:36 ` Hans Schultz
2022-06-02 11:55 ` [Bridge] " Ido Schimmel
2022-06-02 11:55 ` Ido Schimmel
2022-06-02 12:08 ` [Bridge] " Hans Schultz
2022-06-02 12:08 ` Hans Schultz
2022-06-02 12:18 ` [Bridge] " Ido Schimmel
2022-06-02 12:18 ` Ido Schimmel
2022-06-02 12:53 ` [Bridge] " Hans S
2022-06-02 13:27 ` Hans S
2022-06-02 13:27 ` Hans S
2022-05-24 15:21 ` [Bridge] [PATCH V3 net-next 2/4] net: switchdev: add support for offloading of fdb locked flag Hans Schultz
2022-05-24 15:21 ` Hans Schultz
2022-06-27 16:06 ` [Bridge] " Vladimir Oltean
2022-06-27 16:06 ` Vladimir Oltean
2022-05-24 15:21 ` [Bridge] [PATCH V3 net-next 3/4] net: dsa: mv88e6xxx: mac-auth/MAB implementation Hans Schultz
2022-05-24 15:21 ` Hans Schultz
2022-05-24 21:36 ` kernel test robot
2022-06-27 12:58 ` [Bridge] " Hans S
2022-06-27 12:58 ` Hans S
2022-06-27 18:05 ` [Bridge] " Vladimir Oltean
2022-06-27 18:05 ` Vladimir Oltean
2022-06-28 12:26 ` [Bridge] " Hans S
2022-06-28 12:26 ` Hans S
2022-07-05 15:05 ` [Bridge] " Hans S
2022-07-05 15:05 ` Hans S
2022-07-06 13:28 ` [Bridge] " Vladimir Oltean
2022-07-06 13:28 ` Vladimir Oltean
2022-07-06 13:48 ` [Bridge] " Hans S
2022-07-06 13:48 ` Hans S
2022-07-06 8:55 ` [Bridge] " Vladimir Oltean
2022-07-06 8:55 ` Vladimir Oltean
2022-07-06 10:12 ` [Bridge] " Hans S
2022-07-06 10:12 ` Hans S
2022-07-06 14:23 ` [Bridge] " Hans S
2022-07-06 14:23 ` Hans S
2022-07-06 14:33 ` [Bridge] " Vladimir Oltean
2022-07-06 14:33 ` Vladimir Oltean
2022-07-06 15:38 ` [Bridge] " Hans S
2022-07-06 15:38 ` Hans S
2022-07-07 6:54 ` [Bridge] " Hans S
2022-07-07 6:54 ` Hans S
2022-05-24 15:21 ` [Bridge] [PATCH V3 net-next 4/4] selftests: forwarding: add test of MAC-Auth Bypass to locked port tests Hans Schultz
2022-05-24 15:21 ` Hans Schultz
2022-05-26 14:27 ` [Bridge] " Ido Schimmel
2022-05-26 14:27 ` Ido Schimmel
2022-05-27 9:07 ` [Bridge] " Hans Schultz
2022-05-27 9:07 ` Hans Schultz
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=86zgj6oqa9.fsf@gmail.com \
--to=schultz.hans@gmail.com \
--cc=andrew@lunn.ch \
--cc=bridge@lists.linux-foundation.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=idosch@nvidia.com \
--cc=ivecera@redhat.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=razor@blackwall.org \
--cc=roopa@nvidia.com \
--cc=shuah@kernel.org \
--cc=vivien.didelot@gmail.com \
/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.