netdev - Re: [net-next: PATCH 10/12] net: dsa: add ACPI support

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]

Date:   Tue, 21 Jun 2022 01:31:17 +0200
From:   Marcin Wojtas <mw@...ihalf.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Len Brown <lenb@...nel.org>, vivien.didelot@...il.com,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>, pabeni@...hat.com,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Grzegorz Bernacki <gjb@...ihalf.com>,
        Grzegorz Jaszczyk <jaz@...ihalf.com>,
        Tomasz Nowicki <tn@...ihalf.com>,
        Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@....com>,
        upstream@...ihalf.com
Subject: Re: [net-next: PATCH 10/12] net: dsa: add ACPI support

pon., 20 cze 2022 o 20:32 Andrew Lunn <andrew@...n.ch> napisał(a):
>
> >  static int dsa_port_parse_dsa(struct dsa_port *dp)
> >  {
> > +     /* Cascade switch connection is not supported in ACPI world. */
> > +     if (is_acpi_node(dp->fwnode)) {
> > +             dev_warn(dp->ds->dev,
> > +                      "DSA type is not supported with ACPI, disable port #%d\n",
> > +                      dp->index);
> > +             dp->type = DSA_PORT_TYPE_UNUSED;
> > +             return 0;
> > +     }
> > +
>
> Did you try this? I'm not sure it will work correctly. When a switch
> registers with the DSA core, the core will poke around in DT and fill
> in various bits of information, including the DSA links. Once that has
> completed, the core will look at all the switches registered so far
> and try to determine if it has a complete set, i.e, it has both ends
> of all DSA links. If it does have a complete set, it then calls the
> setup methods on each switch, and gets them configured. If it finds it
> does not have a complete setup, it does nothing, waiting for the next
> switch to register.
>
> So if somebody passed an ACPI description with multiple switches, it
> is likely to call the setup methods as soon as the first switch is
> registered. And it might call those same setup methods a second time,
> when the second switch registers, on both switches. And when the third
> switch registers, it will probably call the setup methods yet again on
> all the switches....
>
> You will have a much safer system if you return -EINVAL if you find a
> DSA link in ACPI. That should abort the switch probe.
>

I only set a single port to "dsa" label to check if this condition is
entered. I see 2 devices in the arm64 tree (fsl-lx2160a-bluebox3.dts
and armada-3720-turris-mox.dts) that support cascade switches via
"link" property. I don't have access to real life setup (and those
seem to not support ACPI anyway...).

In case this temporarily would remain as unsupported feature, I agree
-EINVAL is a safer solution.

Thanks,
Marcin