netdev - Re: [RFC net-next] net: dsa: rtl8366rb: support bridge offloading

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: <CACRpkdZykWgxM7Ge40gpMBaVUoa7WqJrOugrvSpm2Lc52hHC8w@mail.gmail.com>
Date:   Mon, 1 Mar 2021 14:48:20 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     DENG Qingfang <dqfext@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        netdev <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next] net: dsa: rtl8366rb: support bridge offloading

On Wed, Feb 24, 2021 at 7:12 AM DENG Qingfang <dqfext@...il.com> wrote:

> Use port isolation registers to configure bridge offloading.
> Remove the VLAN init, as we have proper CPU tag and bridge offloading
> support now.
>
> Signed-off-by: DENG Qingfang <dqfext@...il.com>
> ---
> This is not tested, as I don't have a RTL8366RB board. And I think there
> is potential race condition in port_bridge_{join,leave}.

Compilation failed for me like this:

../drivers/net/dsa/rtl8366rb.c:1573:23: error: initialization of 'void
(*)(struct dsa_switch *, int,  struct net_device *)' from incompatible
pointer type 'int (*)(struct dsa_switch *, int,  struct net_device *)'
[-Werror=incompatible-pointer-types]
 1573 |  .port_bridge_leave = rtl8366rb_port_bridge_leave,
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/dsa/rtl8366rb.c:1573:23: note: (near initialization for
'rtl8366rb_switch_ops.port_bridge_leave')

I fixed it like this:

diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index e7abf846350d..0719fadadc3d 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1161,7 +1161,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch
*ds, int port,
                                  0, port_bitmap << 1);
 }

-static int
+static void
 rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
                            struct net_device *bridge)
 {
@@ -1176,14 +1176,17 @@ rtl8366rb_port_bridge_leave(struct dsa_switch
*ds, int port,
                        continue;
                ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
                                         BIT(port + 1), 0);
-               if (ret)
-                       return ret;
+               if (ret) {
+                       dev_err(smi->dev, "failed to leave port %d
from bridge\n",
+                               port);
+                       return;
+               }

                port_bitmap |= BIT(i);
        }

-       return regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(port),
-                                 port_bitmap << 1, 0);
+       regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(port),
+                          port_bitmap << 1, 0);
 }

After this it works like a charm.

> -       ret = rtl8366_init_vlan(smi);
> -       if (ret)
> -               return ret;

I suppose we can delete that confused VLAN set-up after this.

> -       ds->configure_vlan_while_not_filtering = false;

This is default true not IIUC so we should be good!

With my minor changes:
Tested-by: Linus Walleij <linus.walleij@...aro.org>

Yours,
Linus Walleij