From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=k1P3Y5KcTgUHU2mC7q6Eh1Wa9ToCw2l2iR/tA2vzR4I=; b=Ck6x8U6cRd++Vs08614bd5FirYAglGi7jdxollKezH8QXJK7JK1O3fRZ5xieiLgCy+ zjkyhCLZH+gKaBVtHX5RxG8hZvxZUQLTvfKbBxLlB0+aZt91I5mlMZFqciAlC9FZWJlW T4qG/mhSJ0C1HSgpqHuetEx1IdzV21DaumvZL2fnJIABxWjOJpTprN1TCJH97sUgvt1+ y4lyd4b6l5tkntLKOSVdZNzVtwGy7v5EqzHQ+DPx+4k/u2GGyTNHRqQiNEACaX/oByZP i5n8PxKc6lrahgbpLli6O5GaUbxRnXCTaS6m2kTQo5Yupkjq3vP9lfpNrn+40uduSU9F UvDA== MIME-Version: 1.0 References: <86sfov2w8k.fsf@gmail.com> <86sfoqgi5e.fsf@gmail.com> <86y1yfzap3.fsf@gmail.com> <86sfonjroi.fsf@gmail.com> <3d93d46d-c484-da0a-c12c-80e83eba31c9@blackwall.org> <86mtevjmie.fsf@gmail.com> In-Reply-To: From: Hans S Date: Thu, 2 Jun 2022 14:53:00 +0200 Message-ID: Content-Type: multipart/alternative; boundary="00000000000009789005e0767df3" Subject: Re: [Bridge] [PATCH V3 net-next 1/4] net: bridge: add fdb flag to extent locked port feature List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ido Schimmel Cc: Ivan Vecera , Andrew Lunn , Florian Fainelli , Jiri Pirko , Daniel Borkmann , netdev@vger.kernel.org, Nikolay Aleksandrov , bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Vivien Didelot , Ido Schimmel , Eric Dumazet , Paolo Abeni , linux-kselftest@vger.kernel.org, Roopa Prabhu , Jakub Kicinski , Vladimir Oltean , Shuah Khan , "David S. Miller" --00000000000009789005e0767df3 Content-Type: text/plain; charset="UTF-8" On tor, jun 02, 2022 at 15:18, Ido Schimmel wrote: > On Thu, Jun 02, 2022 at 02:08:41PM +0200, Hans Schultz wrote: > > br_fdb_find_rcu() does a lookup in the bridge driver FDB, but per the > output above, the entry isn't there for some reason. It's only in HW. > > Can it be that you driver is deleting these entries from the bridge > driver FDB via SWITCHDEV_FDB_DEL_TO_BRIDGE for some reason? > Yes, that sounds much like the case. So the replace of course just modifies the SW fdb entry, and then it obviously removes the HW entry, which triggers SWITCHDEV_FDB_DEL_TO_BRIDGE and then adds the replaced, but because it comes from the SW bridge it doesn't need to do more. But as there are only ops for port_fdb_add() and port_fdb_del(), there is no way to know how to handle a replace. So it ends up just deleting the SW fdb entry after all... It is difficult for me to see how this scenario will be able to work differently, or? On Thu, Jun 2, 2022 at 2:18 PM Ido Schimmel wrote: > On Thu, Jun 02, 2022 at 02:08:41PM +0200, Hans Schultz wrote: > > > > > > I think Hans is testing with mv88e6xxx which dumps entries directly > from > > > HW via ndo_fdb_dump(). See dsa_slave_port_fdb_do_dump() which sets > > > NTF_SELF. > > > > > > Hans, are you seeing the entry twice? Once with 'master' and once with > > > 'self'? > > > > > > > When replacing a locked entry it looks like this: > > > > # bridge fdb show dev eth6 | grep 4c > > 00:4c:4c:4c:4c:4c vlan 1 master br0 extern_learn offload locked > > > > # bridge fdb replace 00:4c:4c:4c:4c:4c dev eth6 vlan 1 master static ; > bridge fdb show dev eth6 | grep 4c > > 00:4c:4c:4c:4c:4c vlan 1 self static > > This output means that the FDB entry was deleted from the bridge driver > FDB. > > > > > The problem is then that the function > > br_fdb_find_rcu(br,eth_hdr(skb)->h_source, vid); > > , where the h_source and vid is the entry above, does not find the entry. > > My hypothesis was then that this is because of the 'self' flag that I > > see. > > br_fdb_find_rcu() does a lookup in the bridge driver FDB, but per the > output above, the entry isn't there for some reason. It's only in HW. > > Can it be that you driver is deleting these entries from the bridge > driver FDB via SWITCHDEV_FDB_DEL_TO_BRIDGE for some reason? > > > > > I am thinking that the function dsa_slave_port_fdb_do_dump() is only for > > debug, and thus does not really set any flags in the bridge modules FDB, > > but then I don't understand why the above find function does not find > > the entry? > --00000000000009789005e0767df3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On tor, jun 02, 2022 at 15:18, Ido Schimmel <idosch@nvidia.com> wrote:
> On Thu,= Jun 02, 2022 at 02:08:41PM +0200, Hans Schultz wrote:
>
> br_f= db_find_rcu() does a lookup in the bridge driver FDB, but per the
> o= utput above, the entry isn't there for some reason. It's only in HW= .
>
> Can it be that you driver is deleting these entries from = the bridge
> driver FDB via SWITCHDEV_FDB_DEL_TO_BRIDGE for some reas= on?
>

Yes, that sounds much like the case. So the replace of c= ourse just
modifies the SW fdb entry, and then it obviously removes the = HW entry,
which triggers SWITCHDEV_FDB_DEL_TO_BRIDGE and then adds the r= eplaced,
but because it comes from the SW bridge it doesn't need to = do more.
But as there are only ops for port_fdb_add() and port_fdb_del()= , there
is no way to know how to handle a replace. So it ends up just de= leting
the SW fdb entry after all...

It is difficult for me to se= e how this scenario will be able to work
differently, or?

<= div class=3D"gmail_quote">
On Thu, Jun= 2, 2022 at 2:18 PM Ido Schimmel <i= dosch@nvidia.com> wrote:
On Thu, Jun 02, 2022 at 02:08:41PM +0200, Hans Schultz wrot= e:
> >
> > I think Hans is testing with mv88e6xxx which dumps entries direct= ly from
> > HW via ndo_fdb_dump(). See dsa_slave_port_fdb_do_dump() which set= s
> > NTF_SELF.
> >
> > Hans, are you seeing the entry twice? Once with 'master' = and once with
> > 'self'?
> >
>
> When replacing a locked entry it looks like this:
>
> # bridge fdb show dev eth6 | grep 4c
> 00:4c:4c:4c:4c:4c vlan 1 master br0 extern_learn offload locked
>
> # bridge fdb replace 00:4c:4c:4c:4c:4c dev eth6 vlan 1 master static ;= bridge fdb show dev eth6 | grep 4c
> 00:4c:4c:4c:4c:4c vlan 1 self static

This output means that the FDB entry was deleted from the bridge driver
FDB.

>
> The problem is then that the function
> br_fdb_find_rcu(br,eth_hdr(skb)->h_source, vid);
> , where the h_source and vid is the entry above, does not find the ent= ry.
> My hypothesis was then that this is because of the 'self' flag= that I
> see.

br_fdb_find_rcu() does a lookup in the bridge driver FDB, but per the
output above, the entry isn't there for some reason. It's only in H= W.

Can it be that you driver is deleting these entries from the bridge
driver FDB via SWITCHDEV_FDB_DEL_TO_BRIDGE for some reason?

>
> I am thinking that the function dsa_slave_port_fdb_do_dump() is only f= or
> debug, and thus does not really set any flags in the bridge modules FD= B,
> but then I don't understand why the above find function does not f= ind
> the entry?
--00000000000009789005e0767df3--