Hi Chetan,

On Mon, 24 May 2021 at 12:36, Kumar, M Chetan [off-list ref] wrote:

Hi Loic,

quoted

quoted

+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,

quoted

+                             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?

That’s correct driver creates wwan0 (no-op netdev) to represent the
modem instance and is referenced for link creation over IOSM rtnetlinks.

quoted

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.

In order to use WWAN device instance any additional changes required inside
wwan_core ?  Or simply passing /sys/class/wwan0 device to ip link add is enough.

So basically the rtnetlink ops would be implemented and define in
wwan_core, as "wwan" link  type.
Allowing users to create a new WWAN link/context whatever the
underlying hardware is. We could therefore pass the WWAN device name
or index to e.g:
ip link add wwan0.1 type wwan hw wwan0 session 1

Can you please share us more details on wwan_core changes(if any)/how we should
use /sys/class/wwan0 for link creation ?

Well, move rtnetlink ops to wwan_core (or wwan_rtnetlink), and parse
netlink parameters into the wwan core. Add support for registering
`wwan_ops`, something like:
wwan_register_ops(wwan_ops *ops, struct device *wwan_root_device)

The ops could be basically:
struct wwan_ops {
    int (*add_intf)(struct device *wwan_root_device, const char *name,
struct wwan_intf_params *params);
    int (*del_intf) ...
}

Then you could implement your own ops in iosm, with ios_add_intf()
allocating and registering the netdev as you already do.
struct wwan_intf_params would contain parameters of the interface,
like the session_id (and possibly extended later with others, like
checksum offload, etc...).

What do you think?

quoted

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).

Common interface could benefit both wwan drivers and user side integration.
WWAN framework generalizes WWAN device control port, would it also consider
WWAN netdev part ? Is there a plan to support such implementation inside
wwan_core.

I also plan to submit a change to add a wwan_register_netdevice()
function (similarly to WiFi cfg80211_register_netdevice), that could
be used instead of register_netdevice(), basically factorizing wwan
netdev registration (add "wwan" dev_type, add sysfs link to the 'wwan'
device...).

Regards,
Loic