netdev - Re: [PATCH net-next v3 6/6] net: lan966x: Add switchdev 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]

Message-ID: <20211213134319.dp6b3or24pl3p4en@skbuf>
Date:   Mon, 13 Dec 2021 13:43:20 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>
Subject: Re: [PATCH net-next v3 6/6] net: lan966x: Add switchdev support

On Mon, Dec 13, 2021 at 11:25:29AM +0100, Horatiu Vultur wrote:
> The 12/09/2021 17:43, Horatiu Vultur wrote:
> > > > +int lan966x_register_notifier_blocks(struct lan966x *lan966x)
> > > > +{
> > > > +     int err;
> > > > +
> > > > +     lan966x->netdevice_nb.notifier_call = lan966x_netdevice_event;
> > > > +     err = register_netdevice_notifier(&lan966x->netdevice_nb);
> > > > +     if (err)
> > > > +             return err;
> > > > +
> > > > +     lan966x->switchdev_nb.notifier_call = lan966x_switchdev_event;
> > > > +     err = register_switchdev_notifier(&lan966x->switchdev_nb);
> > > > +     if (err)
> > > > +             goto err_switchdev_nb;
> > > > +
> > > > +     lan966x->switchdev_blocking_nb.notifier_call = lan966x_switchdev_blocking_event;
> > > > +     err = register_switchdev_blocking_notifier(&lan966x->switchdev_blocking_nb);
> > > > +     if (err)
> > > > +             goto err_switchdev_blocking_nb;
> > > > +
> > > > +     lan966x_owq = alloc_ordered_workqueue("lan966x_order", 0);
> > > > +     if (!lan966x_owq) {
> > > > +             err = -ENOMEM;
> > > > +             goto err_switchdev_blocking_nb;
> > > > +     }
> > > 
> > > These should be singleton objects, otherwise things get problematic if
> > > you have more than one switch device instantiated in the system.
> > 
> > Yes, I will update this.
> 
> Actually I think they need to be part of lan966x.
> Because we want each lan966x instance to be independent of each other.
> This is not seen in this version but is more clear in the next version
> (v4).

They are independent of each other. You deduce the interface on which
the notifier was emitted using switchdev_notifier_info_to_dev() and act
upon it, if lan966x_netdevice_check() is true. The notifier handling
code itself is stateless, all the state is per port / per switch.
If you register one notifier handler per switch, lan966x_netdevice_check()
would return true for each notifier handler instance, and you would
handle each event twice, would you not? This is why I'm saying that the
notifier handlers should be registered as singletons, like other drivers
do.