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@cogentembedded.com>
Sent: Thursday, February 5, 2026 8:59 AM
To: Michael Dege <redacted>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
Andrew Lunn [off-list ref]; David S. Miller [off-list ref]; Eric Dumazet
[off-list ref]; Jakub Kicinski [off-list ref]; Paolo Abeni [off-list ref]
Cc: netdev@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-kernel@vger.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:

quoted

Hello Michael

quoted

-        } 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.

quoted

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.

quoted

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