linux-kernel - RE: [PATCH net] net: renesas: rswitch: fix forwarding offload statemachine

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <TY4PR01MB142829D9748A483ECAF19FD3D8299A@TY4PR01MB14282.jpnprd01.prod.outlook.com>
Date: Thu, 5 Feb 2026 12:49:07 +0000
From: Michael Dege <michael.dege@...esas.com>
To: Nikita Yushchenko <nikita.yoush@...entembedded.com>, Yoshihiro Shimoda
	<yoshihiro.shimoda.uh@...esas.com>, 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>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net] net: renesas: rswitch: fix forwarding offload
 statemachine

Hello Nikita,

Thank you once more for your comments.

> -----Original Message-----
> From: Nikita Yushchenko <nikita.yoush@...entembedded.com>
> Sent: Thursday, February 5, 2026 8:59 AM
> To: Michael Dege <michael.dege@...esas.com>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>;
> 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>
> Cc: netdev@...r.kernel.org; linux-renesas-soc@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH net] net: renesas: rswitch: fix forwarding offload statemachine
> 
> 
> 
> WBR,
> Nikita Yushchenko,
> System Software Engineer @ Cogent Embedded
> 
> 05.02.2026 08:47, Nikita Yushchenko wrote:
> > Hello Michael
> >
> >> -        } else if (rdev->forwarding_offloaded) {
> >> +        } else if (rdev->forwarding_offloaded &&
> >> +               !rdev->forwarding_requested) {
> >>               rswitch_change_l2_hw_offloading(rdev, false, false);
> >>           }
> >
> > Although indeed the condition in the current code is not correct, I'm not sure comfortable with this
> fix.
> >
> > Full condition for a port to be a valid candidate for hardware
> > forwarding is
> >
> >    rdev_for_l2_offload() && rdev->forwarding_requested
> >
> > It is not obvious if at this point rdev_for_l2_offload() could get
> > changed from the last call to rswitch_change_l2_hw_offloading(), so
> > using only the partial condition at this point does not look good for me.
> >
> > I'd suggest to either change to something like
> >
> > if (rdev_for_l2_offload() && rdev->forwarding_requested &&
> > !rdev->forwarding_offloaded)
> >      rswitch_change_l2_hw_offloading(rdev, true, false); if
> > (!(rdev_for_l2_offload() && rdev->forwarding_requested) &&
> > rdev->forwarding_offloaded)
> >      rswitch_change_l2_hw_offloading(rdev, false, false);

This works as expected.

> >
> > Or maybe just
> >
> > if (rdev_for_l2_offload() && rdev->forwarding_requested)
> >      rswitch_change_l2_hw_offloading(rdev, true, false); else
> >      rswitch_change_l2_hw_offloading(rdev, false, false);
> >
> > since rswitch_change_l2_hw_offloading() has internal check for the
> > current state and returns early if the requested change is already applied.

Unfortunately, this has a side effect, e.g., if you pull the cable on tsn0 and the link 
goes down, you will see that the offloading is disabled on all ports connected to the 
bridge and not just on tsn0.

> 
> May be even better to add
> 
>    bool new_forwarding_offloaded = rdev_for_l2_offload(rdev) && rdev->forwarding_requested;
> 
> at the beginning of the loop body, and use this flag over the loop - it will make the code shorter and
> cleaner.

Yes, this does make the code cleaner. 

I will send around the updated patch shortly.

Best regards,

Michael