linux-kernel - RE: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching

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:
 <TY4PR01MB142826BE20FB23122B72A96348248A@TY4PR01MB14282.jpnprd01.prod.outlook.com>
Date: Thu, 10 Jul 2025 12:36:10 +0000
From: Michael Dege <michael.dege@...esas.com>
To: Simon Horman <horms@...nel.org>
CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	Niklas Söderlund <niklas.soderlund@...natech.se>, Paul
 Barker <paul@...rker.dev>, 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>,
	"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>, Nikita
 Yushchenko <nikita.yoush@...entembedded.com>
Subject: RE: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2
 switching

Hello Simon,

> -----Original Message-----
> From: Simon Horman <horms@...nel.org>
> Sent: Tuesday, July 8, 2025 12:48 PM
> To: Michael Dege <michael.dege@...esas.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>; Niklas Söderlund
> <niklas.soderlund@...natech.se>; Paul Barker <paul@...rker.dev>; 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>; netdev@...r.kernel.org; linux-renesas-
> soc@...r.kernel.org; linux-kernel@...r.kernel.org; Nikita Yushchenko <nikita.yoush@...entembedded.com>
> Subject: Re: [PATCH v2 3/4] net: renesas: rswitch: add offloading for L2 switching
>
> On Tue, Jul 08, 2025 at 11:27:39AM +0200, Michael Dege wrote:
> > This commit adds hardware offloading for L2 switching on R-Car S4.
> >
> > On S4 brdev is limited to one per-device (not per port). Reasoning is
> > that hw L2 forwarding support lacks any sort of source port based
> > filtering, which makes it unusable to offload more than one bridge
> > device. Either you allow hardware to forward destination MAC to a
> > port, or you have to send it to CPU. You can't make it forward only if
> > src and dst ports are in the same brdev.
> >
> > Signed-off-by: Michael Dege <michael.dege@...esas.com>
> > Signed-off-by: Nikita Yushchenko <nikita.yoush@...entembedded.com>
>
> ...
>
> > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > b/drivers/net/ethernet/renesas/rswitch_l2.c
>
> ...
>
> > +static void rswitch_update_offload_brdev(struct rswitch_private *priv,
> > +                                    bool force_update_l2_offload)
> > +{
> > +   struct net_device *offload_brdev = NULL;
> > +   struct rswitch_device *rdev, *rdev2;
> > +
> > +   rswitch_for_all_ports(priv, rdev) {
> > +           if (!rdev->brdev)
> > +                   continue;
> > +           rswitch_for_all_ports(priv, rdev2) {
> > +                   if (rdev2 == rdev)
> > +                           break;
> > +                   if (rdev2->brdev == rdev->brdev) {
> > +                           offload_brdev = rdev->brdev;
> > +                           break;
> > +                   }
> > +           }
> > +           if (offload_brdev)
> > +                   break;
> > +   }
> > +
> > +   if (offload_brdev == priv->offload_brdev) {
> > +           if (offload_brdev && force_update_l2_offload)
> > +                   rswitch_update_l2_offload(priv);
> > +           return;
> > +   }
> > +
> > +   if (offload_brdev && !priv->offload_brdev)
> > +           dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
> > +                   netdev_name(offload_brdev));
> > +   else if (!offload_brdev && priv->offload_brdev)
> > +           dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
> > +                   netdev_name(priv->offload_brdev));
> > +   else
> > +           dev_dbg(&priv->pdev->dev,
> > +                   "changing l2 offload from %s to %s\n",
> > +                   netdev_name(priv->offload_brdev),
> > +                   netdev_name(offload_brdev));
>
> Smatch flags a false-positive about possible NULL references by the
> netdev_name() calls on the line above.
>
> Due to the previous if statement it seems to me that cannot occur.
> But it did take me a few moments to convince myself of that.
>
> So while I don't think we should write our code to static analysis tooling.
> I did play around a bit to see if I could come up with something that is both easier on the eyes and
> keeps Smatch happy.
>
> Perhaps it isn't easier on the eyes, but rather I'm just more familiar with the code now. But in any
> case, I'm sharing what I came up with in case it is useful. (Compile tested only!).
>
>
>         if (!offload_brdev && !priv->offload_brdev)
>                 return;
>
>         if (!priv->offload_brdev)
>                 dev_dbg(&priv->pdev->dev, "starting l2 offload for %s\n",
>                         netdev_name(offload_brdev));
>         else if (!offload_brdev)
>                 dev_dbg(&priv->pdev->dev, "stopping l2 offload for %s\n",
>                         netdev_name(priv->offload_brdev));
>         else if (offload_brdev != priv->offload_brdev)
>                 dev_dbg(&priv->pdev->dev,
>                         "changing l2 offload from %s to %s\n",
>                         netdev_name(priv->offload_brdev),
>                         netdev_name(offload_brdev));
>         else if (!force_update_l2_offload)
>                 return;
>

I updated the code, I hope it is OK, because I had to do it differently from your suggestion, because
not all cases worked as expected.

The reworked code is tested.

Best regards,

Michael

> > +
> > +   priv->offload_brdev = offload_brdev;
> > +
> > +   rswitch_update_l2_offload(priv);
> > +}
>
> ...
________________________________

Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793

Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.