Hi Chetan,
On Mon, 24 May 2021 at 12:36, Kumar, M Chetan <m.chetan.kumar@xxxxxxxxx> wrote:
>
> Hi Loic,
>
> > > +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?
>
> That’s correct driver creates wwan0 (no-op netdev) to represent the
> modem instance and is 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.
>
> 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?
>
> > 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
