netdev - Re: [PATCH V3 15/16] net: iosm: net driver

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: <CAMZdPi-Xs00vMq-im_wHnNE5XkhXU1-mOgrNbGnExPbHYAL-rw@mail.gmail.com>
Date:   Fri, 21 May 2021 21:55:55 +0200
From:   Loic Poulain <loic.poulain@...aro.org>
To:     M Chetan Kumar <m.chetan.kumar@...el.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        linux-wireless@...r.kernel.org, johannes@...solutions.net,
        krishna.c.sudi@...el.com, linuxwwan@...el.com,
        Dan Williams <dcbw@...hat.com>,
        Bjørn Mork <bjorn@...k.no>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH V3 15/16] net: iosm: net driver

Hi Chetan,

On Thu, 20 May 2021 at 16:09, M Chetan Kumar <m.chetan.kumar@...el.com> wrote:
>
> 1) Create net device & implement net operations for data/IP communication.
> 2) Bind IP Link to mux IP session for simultaneous IP traffic.
>
> Signed-off-by: M Chetan Kumar <m.chetan.kumar@...el.com>
> ---
> v3:
> * Clean-up DSS channel implementation.
> * Aligned ipc_ prefix for function name to be consistent across file.
> v2:
> * Removed Ethernet header & VLAN tag handling from wwan net driver.
> * Implement rtnet_link interface for IP traffic handling.
> ---
>  drivers/net/wwan/iosm/iosm_ipc_wwan.c | 439 ++++++++++++++++++++++++++
>  drivers/net/wwan/iosm/iosm_ipc_wwan.h |  55 ++++
>  2 files changed, 494 insertions(+)
>  create mode 100644 drivers/net/wwan/iosm/iosm_ipc_wwan.c
>  create mode 100644 drivers/net/wwan/iosm/iosm_ipc_wwan.h
>
> diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> new file mode 100644
> index 000000000000..02c35bc86674
> --- /dev/null
> +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> @@ -0,0 +1,439 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020-21 Intel Corporation.
> + */
> +
> +#include <linux/etherdevice.h>
> +#include <linux/if_arp.h>
> +#include <linux/if_link.h>
> +#include <net/rtnetlink.h>
> +
> +#include "iosm_ipc_chnl_cfg.h"
> +#include "iosm_ipc_imem_ops.h"
> +#include "iosm_ipc_wwan.h"
> +
> +#define IOSM_IP_TYPE_MASK 0xF0
> +#define IOSM_IP_TYPE_IPV4 0x40
> +#define IOSM_IP_TYPE_IPV6 0x60

[...]

> +
> +static void ipc_netdev_setup(struct net_device *dev) {}
> +
> +struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
> +{
> +       static const struct net_device_ops iosm_wwandev_ops = {};
> +       struct iosm_wwan *ipc_wwan;
> +       struct net_device *netdev;
> +
> +       netdev = alloc_netdev(sizeof(*ipc_wwan), "wwan%d", NET_NAME_ENUM,
> +                             ipc_netdev_setup);
> +
> +       if (!netdev)
> +               return NULL;
> +
> +       ipc_wwan = netdev_priv(netdev);
> +
> +       ipc_wwan->dev = dev;
> +       ipc_wwan->netdev = netdev;
> +       ipc_wwan->is_registered = false;
> +
> +       ipc_wwan->ipc_imem = ipc_imem;
> +
> +       mutex_init(&ipc_wwan->if_mutex);
> +
> +       /* allocate random ethernet address */
> +       eth_random_addr(netdev->dev_addr);
> +       netdev->addr_assign_type = NET_ADDR_RANDOM;
> +
> +       netdev->netdev_ops = &iosm_wwandev_ops;
> +       netdev->flags |= IFF_NOARP;
> +
> +       SET_NETDEV_DEVTYPE(netdev, &wwan_type);
> +
> +       if (register_netdev(netdev)) {
> +               dev_err(ipc_wwan->dev, "register_netdev failed");
> +               goto reg_fail;
> +       }

So you register a no-op netdev which is only used to represent the
modem instance, and to be referenced for link creation over IOSM
rtnetlinks?
The new WWAN framework creates a logical WWAN device instance (e.g;
/sys/class/wwan0), I think it would make sense to use its index as
parameter when creating the new links, instead of relying on this
dummy netdev. Note that for now the wwan_device is private to
wwan_core and created implicitly on the WWAN control port
registration.
Moreover I wonder if it could also be possible to create a generic
WWAN link type instead of creating yet-another hw specific one, that
could benefit future WWAN drivers, and simplify user side integration,
with a common interface to create links and multiplex PDN (a bit like
wlan vif).

Regards,
Loic