netdev - Re: [RFC PATCH 3/6] net: wwan: core: split port unregister and stop

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: <CAFEp6-0s21Xo7bGW9OYwwBGcKE7-W8hv8BUbh_CQScPfbrWniw@mail.gmail.com>
Date: Wed, 16 Apr 2025 22:21:53 +0200
From: Loic Poulain <loic.poulain@....qualcomm.com>
To: Sergey Ryazanov <ryazanov.s.a@...il.com>
Cc: Johannes Berg <johannes@...solutions.net>,
        Andrew Lunn <andrew+netdev@...n.ch>,
        Eric Dumazet <edumazet@...gle.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
        netdev@...r.kernel.org
Subject: Re: [RFC PATCH 3/6] net: wwan: core: split port unregister and stop

On Mon, Apr 14, 2025 at 11:33 PM Sergey Ryazanov <ryazanov.s.a@...il.com> wrote:
>
> On 14.04.2025 21:54, Loic Poulain wrote:
> > On Wed, Apr 9, 2025 at 1:31 AM Sergey Ryazanov <ryazanov.s.a@...il.com> wrote:
> >>
> >> Upcoming GNSS (NMEA) port type support requires exporting it via the
> >> GNSS subsystem. On another hand, we still need to do basic WWAN core
> >> work: call the port stop operation, purge queues, release the parent
> >> WWAN device, etc. To reuse as much code as possible, split the port
> >> unregistering function into the deregistration of a regular WWAN port
> >> device, and the common port tearing down code.
> >>
> >> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@...il.com>
> >> ---
> >>   drivers/net/wwan/wwan_core.c | 21 ++++++++++++++++-----
> >>   1 file changed, 16 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> >> index 045246d7cd50..439a57bc2b9c 100644
> >> --- a/drivers/net/wwan/wwan_core.c
> >> +++ b/drivers/net/wwan/wwan_core.c
> >> @@ -486,6 +486,18 @@ static int wwan_port_register_wwan(struct wwan_port *port)
> >>          return 0;
> >>   }
> >>
> >> +/* Unregister regular WWAN port (e.g. AT, MBIM, etc) */
> >> +static void wwan_port_unregister_wwan(struct wwan_port *port)
> >
> > Wouldn't it be simpler to name it  `wwan_port_unregister` ?
>
> I came with this complex name for a symm>
> >> +{
> >> +       struct wwan_device *wwandev = to_wwan_dev(port->dev.parent);
> >> +
> >> +       dev_set_drvdata(&port->dev, NULL);
> >> +
> >> +       dev_info(&wwandev->dev, "port %s disconnected\n", dev_name(&port->dev));
> >> +
> >> +       device_unregister(&port->dev);
> >> +}
> >> +
> >>   struct wwan_port *wwan_create_port(struct device *parent,
> >>                                     enum wwan_port_type type,
> >>                                     const struct wwan_port_ops *ops,
> >> @@ -542,18 +554,17 @@ void wwan_remove_port(struct wwan_port *port)
> >>          struct wwan_device *wwandev = to_wwan_dev(port->dev.parent);
> >>
> >>          mutex_lock(&port->ops_lock);
> >> -       if (port->start_count)
> >> +       if (port->start_count) {
> >>                  port->ops->stop(port);
> >> +               port->start_count = 0;
> >> +       }
> >>          port->ops = NULL; /* Prevent any new port operations (e.g. from fops) */
> >>          mutex_unlock(&port->ops_lock);
> >>
> >>          wake_up_interruptible(&port->waitqueue);
> >> -
> >>          skb_queue_purge(&port->rxq);
> >> -       dev_set_drvdata(&port->dev, NULL);
> >>
> >> -       dev_info(&wwandev->dev, "port %s disconnected\n", dev_name(&port->dev));
> >> -       device_unregister(&port->dev);
> >> +       wwan_port_unregister_wwan(port);
> >>
> >>          /* Release related wwan device */
> >>          wwan_remove_dev(wwandev);
> >> --
> >> 2.45.3
> >>
>etry purpose. The next patch
> going to introduce wwan_port_unregister_gnss() handler.
>
> The prefix indicates the module and the suffix indicates the type of the
> unregistering port.

Ok, fair enough.