[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ffe6f6cc-7157-48ad-9cde-dc38d8427849@blackwall.org>
Date: Fri, 28 Mar 2025 00:52:57 +0200
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Joseph Huang <joseph.huang.2024@...il.com>,
Joseph Huang <Joseph.Huang@...min.com>, netdev@...r.kernel.org
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Roopa Prabhu <roopa@...dia.com>, Simon Horman <horms@...nel.org>,
linux-kernel@...r.kernel.org, bridge@...ts.linux.dev
Subject: Re: [Patch net-next 1/3] net: bridge: mcast: Add offload failed mdb
flag
On 3/27/25 00:38, Joseph Huang wrote:
> On 3/21/2025 4:19 AM, Nikolay Aleksandrov wrote:
>>> @@ -516,11 +513,14 @@ static void br_switchdev_mdb_complete(struct
>>> net_device *dev, int err, void *pri
>>> pp = &p->next) {
>>> if (p->key.port != port)
>>> continue;
>>> - p->flags |= MDB_PG_FLAGS_OFFLOAD;
>>> +
>>> + if (err)
>>> + p->flags |= MDB_PG_FLAGS_OFFLOAD_FAILED;
>>> + else
>>> + p->flags |= MDB_PG_FLAGS_OFFLOAD;
>>
>> These two should be mutually exclusive, either it's offloaded or it
>> failed an offload,
>> shouldn't be possible to have both set. I'd recommend adding some
>> helper that takes
>> care of that.
>
> It is true that these two are mutually exclusive, but strictly speaking
> there are four types of entries:
>
> 1. Entries which are not offload-able (i.e., the ports are not backed by
> switchdev)
> 2. Entries which are being offloaded, but results yet unknown
> 3. Entries which are successfully offloaded, and
> 4. Entries which failed to be offloaded
>
> Even if we ignore the ones which are being offloaded (type 2 is
> transient), we still need two flags, otherwise we won't be able to tell
> type 1 from type 4 entries.
>
> If we need two flags anyway, having separate flags for type 3 and type 4
> simplifies the logic.
>
> Or did I misunderstood your comments?
>
> Thanks,
> Joseph
I think you misunderstood me, I don't mind having the two flags. :)
My point is that they must be managed correctly and shouldn't be allowed
to be set simultaneously.
Cheers,
Nik
Powered by blists - more mailing lists