From: Nikolay Aleksandrov <razor@blackwall.org> To: Hans Schultz <schultz.hans@gmail.com>, Ido Schimmel <idosch@nvidia.com> Cc: Ido Schimmel <idosch@idosch.org>, davem@davemloft.net, kuba@kernel.org, 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>, 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: Thu, 2 Jun 2022 13:30:06 +0300 [thread overview] Message-ID: <3d93d46d-c484-da0a-c12c-80e83eba31c9@blackwall.org> (raw) In-Reply-To: <86sfonjroi.fsf@gmail.com> On 02/06/2022 13:17, Hans Schultz wrote: > On tor, jun 02, 2022 at 12:33, Nikolay Aleksandrov <razor@blackwall.org> wrote: >> On 02/06/2022 12:17, Hans Schultz wrote: >>> On tis, maj 31, 2022 at 17:23, Ido Schimmel <idosch@nvidia.com> wrote: >>>> On Tue, May 31, 2022 at 11:34:21AM +0200, Hans Schultz wrote: > >>> Another issue is that >>> bridge fdb add MAC dev DEV master static >>> seems to add the entry with the SELF flag set, which I don't think is >>> what we would want it to do or? >> >> I don't see such thing (hacked iproute2 to print the flags before cmd): >> $ bridge fdb add 00:11:22:33:44:55 dev vnet110 master static >> flags 0x4 >> >> 0x4 = NTF_MASTER only >> > > I also get 0x4 from iproute2, but I still get SELF entries when I look > with: > bridge fdb show dev DEV > after the above add: $ bridge fdb show dev vnet110 | grep 00:11 00:11:22:33:44:55 master virbr0 static >>> Also the replace command is not really supported properly as it is. I >>> have made a fix for that which looks something like this: >>> >>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c >>> index 6cbb27e3b976..f43aa204f375 100644 >>> --- a/net/bridge/br_fdb.c >>> +++ b/net/bridge/br_fdb.c >>> @@ -917,6 +917,9 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source, >>> if (flags & NLM_F_EXCL) >>> return -EEXIST; >>> >>> + if (flags & NLM_F_REPLACE) >>> + modified = true; >>> + >>> if (READ_ONCE(fdb->dst) != source) { >>> WRITE_ONCE(fdb->dst, source); >>> modified = true; >>> >>> The argument for always sending notifications to the driver in the case >>> of replace is that a replace command will refresh the entries timeout if >>> the entry is the same. Any thoughts on this? >> >> I don't think so. It always updates its "used" timer, not its "updated" timer which is the one >> for expire. A replace that doesn't actually change anything on the entry shouldn't generate >> a notification. > > Okay, so then there is missing checks on flags as the issue arose from > replacing locked entries with dynamic entries. I will do another fix > based on flags as modified needs to be true for the driver to get notified.
WARNING: multiple messages have this Message-ID (diff)
From: Nikolay Aleksandrov <razor@blackwall.org> To: Hans Schultz <schultz.hans@gmail.com>, Ido Schimmel <idosch@nvidia.com> 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, bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Vivien Didelot <vivien.didelot@gmail.com>, Ido Schimmel <idosch@idosch.org>, Eric Dumazet <edumazet@google.com>, Paolo Abeni <pabeni@redhat.com>, linux-kselftest@vger.kernel.org, Roopa Prabhu <roopa@nvidia.com>, kuba@kernel.org, Vladimir Oltean <olteanv@gmail.com>, Shuah Khan <shuah@kernel.org>, davem@davemloft.net Subject: Re: [Bridge] [PATCH V3 net-next 1/4] net: bridge: add fdb flag to extent locked port feature Date: Thu, 2 Jun 2022 13:30:06 +0300 [thread overview] Message-ID: <3d93d46d-c484-da0a-c12c-80e83eba31c9@blackwall.org> (raw) In-Reply-To: <86sfonjroi.fsf@gmail.com> On 02/06/2022 13:17, Hans Schultz wrote: > On tor, jun 02, 2022 at 12:33, Nikolay Aleksandrov <razor@blackwall.org> wrote: >> On 02/06/2022 12:17, Hans Schultz wrote: >>> On tis, maj 31, 2022 at 17:23, Ido Schimmel <idosch@nvidia.com> wrote: >>>> On Tue, May 31, 2022 at 11:34:21AM +0200, Hans Schultz wrote: > >>> Another issue is that >>> bridge fdb add MAC dev DEV master static >>> seems to add the entry with the SELF flag set, which I don't think is >>> what we would want it to do or? >> >> I don't see such thing (hacked iproute2 to print the flags before cmd): >> $ bridge fdb add 00:11:22:33:44:55 dev vnet110 master static >> flags 0x4 >> >> 0x4 = NTF_MASTER only >> > > I also get 0x4 from iproute2, but I still get SELF entries when I look > with: > bridge fdb show dev DEV > after the above add: $ bridge fdb show dev vnet110 | grep 00:11 00:11:22:33:44:55 master virbr0 static >>> Also the replace command is not really supported properly as it is. I >>> have made a fix for that which looks something like this: >>> >>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c >>> index 6cbb27e3b976..f43aa204f375 100644 >>> --- a/net/bridge/br_fdb.c >>> +++ b/net/bridge/br_fdb.c >>> @@ -917,6 +917,9 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source, >>> if (flags & NLM_F_EXCL) >>> return -EEXIST; >>> >>> + if (flags & NLM_F_REPLACE) >>> + modified = true; >>> + >>> if (READ_ONCE(fdb->dst) != source) { >>> WRITE_ONCE(fdb->dst, source); >>> modified = true; >>> >>> The argument for always sending notifications to the driver in the case >>> of replace is that a replace command will refresh the entries timeout if >>> the entry is the same. Any thoughts on this? >> >> I don't think so. It always updates its "used" timer, not its "updated" timer which is the one >> for expire. A replace that doesn't actually change anything on the entry shouldn't generate >> a notification. > > Okay, so then there is missing checks on flags as the issue arose from > replacing locked entries with dynamic entries. I will do another fix > based on flags as modified needs to be true for the driver to get notified.
next prev parent reply other threads:[~2022-06-02 10:30 UTC|newest] Thread overview: 108+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-24 15:21 [PATCH V3 net-next 0/4] Extend locked port feature with FDB locked flag (MAC-Auth/MAB) Hans Schultz 2022-05-24 15:21 ` [Bridge] " Hans Schultz 2022-05-24 15:21 ` [PATCH V3 net-next 1/4] net: bridge: add fdb flag to extent locked port feature Hans Schultz 2022-05-24 15:21 ` [Bridge] " Hans Schultz 2022-05-24 15:39 ` Nikolay Aleksandrov 2022-05-24 15:39 ` [Bridge] " Nikolay Aleksandrov 2022-05-24 16:08 ` Hans Schultz 2022-05-24 16:08 ` [Bridge] " Hans Schultz 2022-05-24 16:21 ` Hans Schultz 2022-05-24 16:21 ` [Bridge] " Hans Schultz 2022-05-25 8:06 ` Nikolay Aleksandrov 2022-05-25 8:06 ` [Bridge] " Nikolay Aleksandrov 2022-05-25 8:34 ` Hans Schultz 2022-05-25 8:34 ` [Bridge] " Hans Schultz 2022-05-25 8:38 ` Nikolay Aleksandrov 2022-05-25 8:38 ` [Bridge] " Nikolay Aleksandrov 2022-05-25 9:11 ` Hans Schultz 2022-05-25 9:11 ` [Bridge] " Hans Schultz 2022-05-25 10:18 ` Nikolay Aleksandrov 2022-05-25 10:18 ` [Bridge] " Nikolay Aleksandrov 2022-07-06 18:13 ` Vladimir Oltean 2022-07-06 18:13 ` [Bridge] " Vladimir Oltean 2022-07-06 19:38 ` Nikolay Aleksandrov 2022-07-06 19:38 ` [Bridge] " Nikolay Aleksandrov 2022-07-06 20:21 ` Vladimir Oltean 2022-07-06 20:21 ` [Bridge] " Vladimir Oltean 2022-07-06 21:01 ` Nikolay Aleksandrov 2022-07-06 21:01 ` [Bridge] " Nikolay Aleksandrov 2022-07-07 14:08 ` Nikolay Aleksandrov 2022-07-07 14:08 ` [Bridge] " Nikolay Aleksandrov 2022-07-07 17:15 ` Vladimir Oltean 2022-07-07 17:15 ` [Bridge] " Vladimir Oltean 2022-07-07 17:26 ` Nikolay Aleksandrov 2022-07-07 17:26 ` [Bridge] " Nikolay Aleksandrov 2022-07-08 6:38 ` Hans S 2022-07-08 6:38 ` [Bridge] " Hans S 2022-05-26 14:13 ` Ido Schimmel 2022-05-26 14:13 ` [Bridge] " Ido Schimmel 2022-05-27 8:52 ` Hans Schultz 2022-05-27 8:52 ` [Bridge] " Hans Schultz 2022-05-27 9:58 ` Ido Schimmel 2022-05-27 9:58 ` [Bridge] " Ido Schimmel 2022-05-27 16:00 ` Hans Schultz 2022-05-27 16:00 ` [Bridge] " Hans Schultz 2022-05-31 9:34 ` Hans Schultz 2022-05-31 9:34 ` [Bridge] " Hans Schultz 2022-05-31 14:23 ` Ido Schimmel 2022-05-31 14:23 ` [Bridge] " Ido Schimmel 2022-05-31 15:49 ` Hans Schultz 2022-05-31 15:49 ` [Bridge] " Hans Schultz 2022-06-02 9:17 ` Hans Schultz 2022-06-02 9:17 ` [Bridge] " Hans Schultz 2022-06-02 9:33 ` Nikolay Aleksandrov 2022-06-02 9:33 ` [Bridge] " Nikolay Aleksandrov 2022-06-02 10:17 ` Hans Schultz 2022-06-02 10:17 ` [Bridge] " Hans Schultz 2022-06-02 10:30 ` Nikolay Aleksandrov [this message] 2022-06-02 10:30 ` Nikolay Aleksandrov 2022-06-02 10:39 ` Ido Schimmel 2022-06-02 10:39 ` [Bridge] " Ido Schimmel 2022-06-02 11:36 ` Hans Schultz 2022-06-02 11:36 ` [Bridge] " Hans Schultz 2022-06-02 11:55 ` Ido Schimmel 2022-06-02 11:55 ` [Bridge] " Ido Schimmel 2022-06-02 12:08 ` Hans Schultz 2022-06-02 12:08 ` [Bridge] " Hans Schultz 2022-06-02 12:18 ` Ido Schimmel 2022-06-02 12:18 ` [Bridge] " Ido Schimmel 2022-06-02 12:53 ` Hans S 2022-06-02 13:27 ` Hans S 2022-06-02 13:27 ` [Bridge] " Hans S 2022-05-24 15:21 ` [PATCH V3 net-next 2/4] net: switchdev: add support for offloading of fdb locked flag Hans Schultz 2022-05-24 15:21 ` [Bridge] " Hans Schultz 2022-06-27 16:06 ` Vladimir Oltean 2022-06-27 16:06 ` [Bridge] " Vladimir Oltean 2022-05-24 15:21 ` [PATCH V3 net-next 3/4] net: dsa: mv88e6xxx: mac-auth/MAB implementation Hans Schultz 2022-05-24 15:21 ` [Bridge] " Hans Schultz 2022-05-24 21:36 ` kernel test robot 2022-06-27 12:58 ` Hans S 2022-06-27 12:58 ` [Bridge] " Hans S 2022-06-27 18:05 ` Vladimir Oltean 2022-06-27 18:05 ` [Bridge] " Vladimir Oltean 2022-06-28 12:26 ` Hans S 2022-06-28 12:26 ` [Bridge] " Hans S 2022-07-05 15:05 ` Hans S 2022-07-05 15:05 ` [Bridge] " Hans S 2022-07-06 13:28 ` Vladimir Oltean 2022-07-06 13:28 ` [Bridge] " Vladimir Oltean 2022-07-06 13:48 ` Hans S 2022-07-06 13:48 ` [Bridge] " Hans S 2022-07-06 8:55 ` Vladimir Oltean 2022-07-06 8:55 ` [Bridge] " Vladimir Oltean 2022-07-06 10:12 ` Hans S 2022-07-06 10:12 ` [Bridge] " Hans S 2022-07-06 14:23 ` Hans S 2022-07-06 14:23 ` [Bridge] " Hans S 2022-07-06 14:33 ` Vladimir Oltean 2022-07-06 14:33 ` [Bridge] " Vladimir Oltean 2022-07-06 15:38 ` Hans S 2022-07-06 15:38 ` [Bridge] " Hans S 2022-07-07 6:54 ` Hans S 2022-07-07 6:54 ` [Bridge] " Hans S 2022-05-24 15:21 ` [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 ` [Bridge] " Hans Schultz 2022-05-26 14:27 ` Ido Schimmel 2022-05-26 14:27 ` [Bridge] " Ido Schimmel 2022-05-27 9:07 ` Hans Schultz 2022-05-27 9:07 ` [Bridge] " 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=3d93d46d-c484-da0a-c12c-80e83eba31c9@blackwall.org \ --to=razor@blackwall.org \ --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@idosch.org \ --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=roopa@nvidia.com \ --cc=schultz.hans@gmail.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: linkBe 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.