All of lore.kernel.org
 help / color / mirror / Atom feed
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:08:08 +0200	[thread overview]
Message-ID: <8635gyvrpz.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,

I could check if f->dst is NULL, but in general this should be able to
work on a per port basis, so do you have an idea of how to keep a per
port counter of added locked fdb entries?

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:08:08 +0200	[thread overview]
Message-ID: <8635gyvrpz.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,

I could check if f->dst is NULL, but in general this should be able to
work on a per port basis, so do you have an idea of how to keep a per
port counter of added locked fdb entries?

Best,
Hans

  reply	other threads:[~2022-05-24 16:08 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     ` Hans Schultz [this message]
2022-05-24 16:08       ` Hans Schultz
2022-05-24 16:21     ` [Bridge] " Hans Schultz
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=8635gyvrpz.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.