| Message ID | 20210520140158.10132-16-m.chetan.kumar@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: iosm: PCIe Driver for Intel M.2 Modem | expand |
Hi Chetan, On Thu, 20 May 2021 at 16:09, M Chetan Kumar <m.chetan.kumar@intel.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@intel.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
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. Can you please share us more details on wwan_core changes(if any)/how we should use /sys/class/wwan0 for link creation ? > 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. Regards, Chetan
Hi Chetan, On Mon, 24 May 2021 at 12:36, Kumar, M Chetan <m.chetan.kumar@intel.com> 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
On Tue, 2021-05-25 at 10:24 +0200, Loic Poulain wrote: > > > > 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? Note that I kind of tried this in my version back when: https://lore.kernel.org/netdev/20200225105149.59963c95aa29.Id0e40565452d0d5bb9ce5cc00b8755ec96db8559@changeid/#Z30include:net:wwan.h See struct wwan_component_device_ops. I had a different *generic* netlink family rather than rtnetlink ops, but that's mostly an implementation detail. I tend to like genetlink better, but having rtnetlink makes it easier in iproute2 (though it has some generic netlink code too, of course.) Nobody really seemed all that interested back then. And frankly, I'm really annoyed that while all of this played out we let QMI enter the tree with their home-grown stuff (and dummy netdevs, FWIW), *then* said the IOSM driver has to go to rtnetlink ops like them, instead of what older drivers are doing, and *now* are shifting goalposts again towards something like the framework I started writing early on for the IOSM driver, while the QMI driver was happening, and nobody cared ... Yeah, life's not fair and all that, but it does kind of feel like arbitrary shifting of the goal posts, while QMI is already in tree. Of course it's not like we have a competition with them here, but having some help from there would've been nice. Oh well. Not that I disagree with any of this, it does technically make sense. However, I think at this point it'd be good to see some comments here from DaveM/Jakub that they're going to force Qualcomm to also go down this route, because they're now *heavily* invested in their own APIs, and inventing a framework just for the IOSM driver is fairly pointless. > 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...). Be careful with that, I tend to think we made some mistakes in this area, look at the recent locking things there. We used to do stuff from a netdev notifier, and that has caused all kinds of pain recently when I reworked the locking to not be so overly dependent on the RTNL all the time (which really has become the new BKL, at least for desktop work, sometimes I can't even type in the UI when the RTNL is blocked). So wwan_register_netdevice() is probably fine, doing netdev notifier I'd now recommend against. But in any case, that's just a side thread. johannes
Hi Johannes, On Tue, 25 May 2021 at 14:55, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Tue, 2021-05-25 at 10:24 +0200, Loic Poulain wrote: > > > > > > > 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? > > Note that I kind of tried this in my version back when: > > https://lore.kernel.org/netdev/20200225105149.59963c95aa29.Id0e40565452d0d5bb9ce5cc00b8755ec96db8559@changeid/#Z30include:net:wwan.h > > See struct wwan_component_device_ops. > > I had a different *generic* netlink family rather than rtnetlink ops, > but that's mostly an implementation detail. I tend to like genetlink > better, but having rtnetlink makes it easier in iproute2 (though it has > some generic netlink code too, of course.) > > Nobody really seemed all that interested back then. > > And frankly, I'm really annoyed that while all of this played out we let > QMI enter the tree with their home-grown stuff (and dummy netdevs, > FWIW), *then* said the IOSM driver has to go to rtnetlink ops like them, > instead of what older drivers are doing, and *now* are shifting > goalposts again towards something like the framework I started writing > early on for the IOSM driver, while the QMI driver was happening, and > nobody cared ... Yes, I guess it's all about timings... At least, I care now... I've recently worked on the mhi_net driver, which is basically the netdev driver for Qualcomm PCIe modems. MHI being similar to IOSM (exposing logical channels over PCI). Like QCOM USB variants, data can be transferred in QMAP format (I guess what you call QMI), via the `rmnet` link type (setup via iproute2). > > Yeah, life's not fair and all that, but it does kind of feel like > arbitrary shifting of the goal posts, while QMI is already in tree. Of > course it's not like we have a competition with them here, but having > some help from there would've been nice. Oh well. > > Not that I disagree with any of this, it does technically make sense. > > However, I think at this point it'd be good to see some comments here > from DaveM/Jakub that they're going to force Qualcomm to also go down > this route, because they're now *heavily* invested in their own APIs, > and inventing a framework just for the IOSM driver is fairly pointless. This a legitimate point, but it's not too late to do the 'right' thing, + It should not be too much change in the IOSM driver. Regarding Qualcomm, I think it should be possible to fit QCOM Modem drivers into that solution. It would consist of creating a simple wrapper in QMAP/rmnet so that the rmnet link can (also) be created from the kernel side (e.g. from mhi_net driver): wwan_new_link() => wwan->add_intf_cb() => mhi_net_add_intf() => rmnet_newlink() That way mhi_net driver would comply with the new hw agnostic wwan link API, without breaking backward compatibility if someone wants to explicitly create a rmnet link. Moreover, it could also be applicable to USB modems based on MBIM and their VLAN link types. That's my guess, but maybe I do not have the whole picture and miss something... Anyway, I'll not blame the IOSM driver for not doing this. But If you decide to go with the wwan link type, I'll do my best to adapt that to QCOM mhi_net as well. > > 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...). > > Be careful with that, I tend to think we made some mistakes in this > area, look at the recent locking things there. We used to do stuff from > a netdev notifier, and that has caused all kinds of pain recently when I > reworked the locking to not be so overly dependent on the RTNL all the > time (which really has become the new BKL, at least for desktop work, > sometimes I can't even type in the UI when the RTNL is blocked). So > wwan_register_netdevice() is probably fine, doing netdev notifier I'd > now recommend against. > But in any case, that's just a side thread. Sure, thanks for pointing this. Regards, Loic
Hi Loic, > Yes, I guess it's all about timings... At least, I care now... :) > I've recently worked on the mhi_net driver, which is basically the > netdev driver for Qualcomm PCIe modems. MHI being similar to IOSM > (exposing logical channels over PCI). Like QCOM USB variants, data can > be transferred in QMAP format (I guess what you call QMI), via the > `rmnet` link type (setup via iproute2). Right. (I know nothing about the formats, so if I said anything about 'QMI' just ignore and think 'qualcomm stuff') > > This a legitimate point, but it's not too late to do the 'right' > thing, + It should not be too much change in the IOSM driver. Agree. Though I looked at it now in the last couple of hours, and it's actually not easy to do. I came up with these patches for now: https://p.sipsolutions.net/d8d8897c3f43cb85.txt (on top of 5.13-rc3 + the patchset we're discussing here) The key problem is that rtnetlink ops are meant to be for a single device family, and don't really generalize well. For example: +static void wwan_rtnl_setup(struct net_device *dev) +{ + /* FIXME - how do we implement this? we dont have any data + * at this point ..., i.e. we can't look up the context yet? + * We'd need data[IFLA_WWAN_DEV_NAME], see wwan_rtnl_newlink(). + */ +} or +static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = { [...] + .priv_size = WWAN_MAX_NETDEV_PRIV, are both rather annoying. Making this more generic should of course be possible, but would require fairly large changes all over the kernel - passing the tb/data to all the handlers involved here, etc. That seems awkward? What do you think? The alternative I could see is doing what wifi did and create a completely new (generic) netlink family, but that's also awkward to some extend and requires writing more code to handle stuff that rtnetlink already does ... Please take a look. I suppose we could change rtnetlink to make it possible to have this behind it ... but that might even be tricky, because setup() is called in the context of alloc_netdev_mqs(), and that also has no private data to pass through. So would we have to extend rtnetlink ops with a "get_setup()" method that actually *returns* a pointer to the setup method, so that it can be per-user (such as IOSM)? Tricky stuff. > Regarding Qualcomm, I think it should be possible to fit QCOM Modem > drivers into that solution. It would consist of creating a simple > wrapper in QMAP/rmnet so that the rmnet link can (also) be created > from the kernel side (e.g. from mhi_net driver): > wwan_new_link() => wwan->add_intf_cb() => mhi_net_add_intf() => rmnet_newlink() > > That way mhi_net driver would comply with the new hw agnostic wwan link > API, without breaking backward compatibility if someone wants to > explicitly create a rmnet link. Moreover, it could also be applicable > to USB modems based on MBIM and their VLAN link types. Maybe. It'd just need some incentive, and there's none now that the Qualcomm stuff is already there :) johannes
Hi Johannes, On Thu, 27 May 2021 at 11:40, Johannes Berg <johannes@sipsolutions.net> wrote: > > Hi Loic, > > > Yes, I guess it's all about timings... At least, I care now... > > :) > > > I've recently worked on the mhi_net driver, which is basically the > > netdev driver for Qualcomm PCIe modems. MHI being similar to IOSM > > (exposing logical channels over PCI). Like QCOM USB variants, data can > > be transferred in QMAP format (I guess what you call QMI), via the > > `rmnet` link type (setup via iproute2). > > Right. > > (I know nothing about the formats, so if I said anything about 'QMI' > just ignore and think 'qualcomm stuff') > > > > > This a legitimate point, but it's not too late to do the 'right' > > thing, + It should not be too much change in the IOSM driver. > > Agree. Though I looked at it now in the last couple of hours, and it's > actually not easy to do. > > I came up with these patches for now: > https://p.sipsolutions.net/d8d8897c3f43cb85.txt > > (on top of 5.13-rc3 + the patchset we're discussing here) Great, that looks exactly what we need. > > The key problem is that rtnetlink ops are meant to be for a single > device family, and don't really generalize well. For example: > > +static void wwan_rtnl_setup(struct net_device *dev) > +{ > + /* FIXME - how do we implement this? we dont have any data > + * at this point ..., i.e. we can't look up the context yet? > + * We'd need data[IFLA_WWAN_DEV_NAME], see wwan_rtnl_newlink(). > + */ > +} Argh, yes I've overlooked that issue. But, do we even need to do something here? What if we do nothing here and call wwan->ops->setup() early in wwan_rtnl_newlink(). AFAIU, we don't really use data setup info until we actually register the netdev, except maybe for the netdev->txqueue_len. Though it's probably not a robust solution... > > or > > +static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = { > [...] > + .priv_size = WWAN_MAX_NETDEV_PRIV, > > are both rather annoying. > > Making this more generic should of course be possible, but would require > fairly large changes all over the kernel - passing the tb/data to all > the handlers involved here, etc. That seems awkward? Yes, or alternatively add an optional alloc_netdev() rtnl ops, e.g. in rtnl_create_link: - dev = alloc_netdev_mqs(ops->priv_size, ifname, name_assign_type, - ops->setup, num_tx_queues, num_rx_queues); + if (ops->alloc_netdev) { + dev = ops->alloc_netdev(ifname, name_assign_type, num_tx_queues, + num_rx_queues, tb); + } else { + dev = alloc_netdev_mqs(ops->priv_size, ifname, name_assign_type, + ops->setup, num_tx_queues, num_rx_queues); + } That would solve both the issues (setup, priv_size), without entire kernel refactoring. > > What do you think? > > The alternative I could see is doing what wifi did and create a > completely new (generic) netlink family, but that's also awkward to some > extend and requires writing more code to handle stuff that rtnetlink > already does ... That would work indeed, but I would prefer avoiding such 'complexity', mainly because link management is all we need. Indeed, except if we want to abstract and handle control protocols (MBIM, QMI, AT) in the kernel, we should not have to expose additional high-level operations as in nl80211/cfg80211. > > > Please take a look. I suppose we could change rtnetlink to make it > possible to have this behind it ... but that might even be tricky, > because setup() is called in the context of alloc_netdev_mqs(), and that > also has no private data to pass through. So would we have to extend > rtnetlink ops with a "get_setup()" method that actually *returns* a > pointer to the setup method, so that it can be per-user (such as IOSM)? > Tricky stuff. What do you think about this alloc_netdev() ops? we should be able to retrieve all we need from that. Regards, Loic
Hi, > Yes, or alternatively add an optional alloc_netdev() rtnl ops, e.g. in > rtnl_create_link: Yes, that works. It needs some more fiddling (we need 'data', not just 'tb') and it cannot be called 'alloc_netdev' (that's a macro!), but yeah, otherwise works. I'll just call it alloc(). I'll send a few updated patches in a few minutes. johannes
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 + +#define IOSM_IF_ID_PAYLOAD 2 + +static struct device_type wwan_type = { .name = "wwan" }; + +static const struct nla_policy ipc_wwan_policy[IFLA_IOSM_MAX + 1] = { + [IFLA_IOSM_IF_ID] = { .type = NLA_U16 }, +}; + +/** + * struct iosm_net_link - This structure includes information about interface + * dev. + * @if_id: Interface id for device. + * @ch_id: IPC channel number for which interface device is created. + * @netdev: Pointer to network interface device structure + * @ipc_wwan: Pointer to iosm_wwan struct + */ + +struct iosm_net_link { + int if_id; + int ch_id; + struct net_device *netdev; + struct iosm_wwan *ipc_wwan; +}; + +/** + * struct iosm_wwan - This structure contains information about WWAN root device + * and interface to the IPC layer. + * @netdev: Pointer to network interface device structure. + * @sub_netlist: List of netlink interfaces + * @ipc_imem: Pointer to imem data-struct + * @dev: Pointer device structure + * @if_mutex: Mutex used for add and remove interface id + * @is_registered: Registration status with netdev + */ +struct iosm_wwan { + struct net_device *netdev; + struct iosm_net_link __rcu *sub_netlist[MAX_IP_MUX_SESSION]; + struct iosm_imem *ipc_imem; + struct device *dev; + struct mutex if_mutex; /* Mutex used for add and remove interface id */ + u8 is_registered:1; +}; + +/* Bring-up the wwan net link */ +static int ipc_wwan_link_open(struct net_device *netdev) +{ + struct iosm_net_link *netlink = netdev_priv(netdev); + struct iosm_wwan *ipc_wwan = netlink->ipc_wwan; + int if_id = netlink->if_id; + int ret = -EINVAL; + + if (if_id < IP_MUX_SESSION_START || if_id > IP_MUX_SESSION_END) + return ret; + + mutex_lock(&ipc_wwan->if_mutex); + + /* get channel id */ + netlink->ch_id = + ipc_imem_sys_wwan_open(ipc_wwan->ipc_imem, if_id); + + if (netlink->ch_id < 0) { + dev_err(ipc_wwan->dev, + "cannot connect wwan0 & id %d to the IPC mem layer", + if_id); + ret = -ENODEV; + goto out; + } + + /* enable tx path, DL data may follow */ + netif_start_queue(netdev); + + dev_dbg(ipc_wwan->dev, "Channel id %d allocated to if_id %d", + netlink->ch_id, netlink->if_id); + + ret = 0; +out: + mutex_unlock(&ipc_wwan->if_mutex); + return ret; +} + +/* Bring-down the wwan net link */ +static int ipc_wwan_link_stop(struct net_device *netdev) +{ + struct iosm_net_link *netlink = netdev_priv(netdev); + + netif_stop_queue(netdev); + + mutex_lock(&netlink->ipc_wwan->if_mutex); + ipc_imem_sys_wwan_close(netlink->ipc_wwan->ipc_imem, netlink->if_id, + netlink->ch_id); + mutex_unlock(&netlink->ipc_wwan->if_mutex); + + return 0; +} + +/* Transmit a packet */ +static int ipc_wwan_link_transmit(struct sk_buff *skb, + struct net_device *netdev) +{ + struct iosm_net_link *netlink = netdev_priv(netdev); + struct iosm_wwan *ipc_wwan = netlink->ipc_wwan; + int if_id = netlink->if_id; + int ret = -EINVAL; + + /* Interface IDs from 1 to 8 are for IP data + * & from 257 to 261 are for non-IP data + */ + if (if_id < IP_MUX_SESSION_START || if_id > IP_MUX_SESSION_END) + goto exit; + + /* Send the SKB to device for transmission */ + ret = ipc_imem_sys_wwan_transmit(ipc_wwan->ipc_imem, + if_id, netlink->ch_id, skb); + + /* Return code of zero is success */ + if (ret == 0) { + ret = NETDEV_TX_OK; + } else if (ret == -EBUSY) { + ret = NETDEV_TX_BUSY; + dev_err(ipc_wwan->dev, "unable to push packets"); + } else { + goto exit; + } + + return ret; + +exit: + /* Log any skb drop */ + if (if_id) + dev_dbg(ipc_wwan->dev, "skb dropped. IF_ID: %d, ret: %d", if_id, + ret); + + dev_kfree_skb_any(skb); + return ret; +} + +/* Ops structure for wwan net link */ +static const struct net_device_ops ipc_inm_ops = { + .ndo_open = ipc_wwan_link_open, + .ndo_stop = ipc_wwan_link_stop, + .ndo_start_xmit = ipc_wwan_link_transmit, +}; + +/* Setup function for creating new net link */ +static void ipc_wwan_setup(struct net_device *iosm_dev) +{ + iosm_dev->header_ops = NULL; + iosm_dev->hard_header_len = 0; + iosm_dev->priv_flags |= IFF_NO_QUEUE; + + iosm_dev->type = ARPHRD_NONE; + iosm_dev->min_mtu = ETH_MIN_MTU; + iosm_dev->max_mtu = ETH_MAX_MTU; + + iosm_dev->flags = IFF_POINTOPOINT | IFF_NOARP; + + iosm_dev->netdev_ops = &ipc_inm_ops; +} + +static struct device_type inm_type = { .name = "iosmdev" }; + +/* Create new wwan net link */ +static int ipc_wwan_newlink(struct net *src_net, struct net_device *dev, + struct nlattr *tb[], struct nlattr *data[], + struct netlink_ext_ack *extack) +{ + struct iosm_net_link *netlink = netdev_priv(dev); + struct iosm_wwan *ipc_wwan; + struct net_device *netdev; + int err = -EINVAL; + int index; + + if (!data[IFLA_IOSM_IF_ID]) { + pr_err("Interface ID not specified"); + goto out; + } + + if (!tb[IFLA_LINK]) { + pr_err("Link not specified"); + goto out; + } + + netlink->netdev = dev; + + netdev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK])); + + netlink->ipc_wwan = netdev_priv(netdev); + + ipc_wwan = netlink->ipc_wwan; + + if (ipc_wwan->netdev != netdev) + goto out; + + netlink->if_id = nla_get_u16(data[IFLA_IOSM_IF_ID]); + index = netlink->if_id; + + /* Complete all memory stores before this point */ + smp_mb(); + if (index < IP_MUX_SESSION_START || index > IP_MUX_SESSION_END) + goto out; + + rcu_read_lock(); + + if (rcu_access_pointer(ipc_wwan->sub_netlist[index - 1])) { + pr_err("IOSM interface ID already in use"); + goto out_free_lock; + } + + SET_NETDEV_DEVTYPE(dev, &inm_type); + + eth_hw_addr_random(dev); + err = register_netdevice(dev); + if (err) { + dev_err(ipc_wwan->dev, "register netlink failed.\n"); + goto out_free_lock; + } + + err = netdev_upper_dev_link(ipc_wwan->netdev, dev, extack); + + if (err) { + dev_err(ipc_wwan->dev, "netdev linking with parent failed.\n"); + goto netlink_err; + } + + rcu_assign_pointer(ipc_wwan->sub_netlist[index - 1], netlink); + netif_device_attach(dev); + rcu_read_unlock(); + + return 0; + +netlink_err: + unregister_netdevice(dev); +out_free_lock: + rcu_read_unlock(); +out: + return err; +} + +/* Delete new wwan net link */ +static void ipc_wwan_dellink(struct net_device *dev, struct list_head *head) +{ + struct iosm_net_link *netlink = netdev_priv(dev); + u16 index = netlink->if_id; + + netdev_upper_dev_unlink(netlink->ipc_wwan->netdev, dev); + unregister_netdevice(dev); + + mutex_lock(&netlink->ipc_wwan->if_mutex); + rcu_assign_pointer(netlink->ipc_wwan->sub_netlist[index - 1], NULL); + mutex_unlock(&netlink->ipc_wwan->if_mutex); +} + +/* Get size for iosm net link payload*/ +static size_t ipc_wwan_get_size(const struct net_device *dev) +{ + return nla_total_size(IOSM_IF_ID_PAYLOAD); +} + +/* Validate the input parameters for wwan net link */ +static int ipc_wwan_validate(struct nlattr *tb[], struct nlattr *data[], + struct netlink_ext_ack *extack) +{ + u16 if_id; + + if (!data || !data[IFLA_IOSM_IF_ID]) { + NL_SET_ERR_MSG_MOD(extack, "IF ID not specified"); + return -EINVAL; + } + + if_id = nla_get_u16(data[IFLA_IOSM_IF_ID]); + + if (if_id < IP_MUX_SESSION_START || if_id > IP_MUX_SESSION_END) { + NL_SET_ERR_MSG_MOD(extack, "Invalid Interface"); + return -ERANGE; + } + + return 0; +} + +/* RT Net link ops structure for new wwan net link */ +struct rtnl_link_ops iosm_netlink __read_mostly = { + .kind = "iosm", + .maxtype = __IFLA_IOSM_MAX, + .priv_size = sizeof(struct iosm_net_link), + .setup = ipc_wwan_setup, + .validate = ipc_wwan_validate, + .newlink = ipc_wwan_newlink, + .dellink = ipc_wwan_dellink, + .get_size = ipc_wwan_get_size, + .policy = ipc_wwan_policy, +}; + +int ipc_wwan_receive(struct iosm_wwan *ipc_wwan, struct sk_buff *skb_arg, + bool dss, int if_id) +{ + struct sk_buff *skb = skb_arg; + struct net_device_stats stats; + int ret; + + if ((skb->data[0] & IOSM_IP_TYPE_MASK) == IOSM_IP_TYPE_IPV4) + skb->protocol = htons(ETH_P_IP); + else if ((skb->data[0] & IOSM_IP_TYPE_MASK) == + IOSM_IP_TYPE_IPV6) + skb->protocol = htons(ETH_P_IPV6); + + skb->pkt_type = PACKET_HOST; + + if (if_id < (IP_MUX_SESSION_START - 1) || + if_id > (IP_MUX_SESSION_END - 1)) { + dev_kfree_skb(skb); + return -EINVAL; + } + + rcu_read_lock(); + skb->dev = rcu_dereference(ipc_wwan->sub_netlist[if_id])->netdev; + stats = rcu_dereference(ipc_wwan->sub_netlist[if_id])->netdev->stats; + stats.rx_packets++; + stats.rx_bytes += skb->len; + + ret = netif_rx(skb); + rcu_read_unlock(); + + return ret; +} + +void ipc_wwan_tx_flowctrl(struct iosm_wwan *ipc_wwan, int if_id, bool on) +{ + struct net_device *netdev; + bool is_tx_blk; + + rcu_read_lock(); + netdev = rcu_dereference(ipc_wwan->sub_netlist[if_id])->netdev; + + is_tx_blk = netif_queue_stopped(netdev); + + if (on) + dev_dbg(ipc_wwan->dev, "session id[%d]: flowctrl enable", + if_id); + + if (on && !is_tx_blk) + netif_stop_queue(netdev); + else if (!on && is_tx_blk) + netif_wake_queue(netdev); + rcu_read_unlock(); +} + +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; + } + + ipc_wwan->is_registered = true; + + netif_device_attach(netdev); + + return ipc_wwan; + +reg_fail: + free_netdev(netdev); + return NULL; +} + +void ipc_wwan_deinit(struct iosm_wwan *ipc_wwan) +{ + struct iosm_net_link *netlink; + int i; + + if (ipc_wwan->is_registered) { + rcu_read_lock(); + for (i = IP_MUX_SESSION_START; i <= IP_MUX_SESSION_END; i++) { + if (rcu_access_pointer(ipc_wwan->sub_netlist[i - 1])) { + netlink = + rcu_dereference(ipc_wwan->sub_netlist[i - 1]); + rtnl_lock(); + netdev_upper_dev_unlink(ipc_wwan->netdev, + netlink->netdev); + unregister_netdevice(netlink->netdev); + rtnl_unlock(); + rcu_assign_pointer(ipc_wwan->sub_netlist[i - 1], + NULL); + } + } + rcu_read_unlock(); + + unregister_netdev(ipc_wwan->netdev); + free_netdev(ipc_wwan->netdev); + } +} diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.h b/drivers/net/wwan/iosm/iosm_ipc_wwan.h new file mode 100644 index 000000000000..4925f22dff0a --- /dev/null +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0-only + * + * Copyright (C) 2020-21 Intel Corporation. + */ + +#ifndef IOSM_IPC_WWAN_H +#define IOSM_IPC_WWAN_H + +/** + * ipc_wwan_init - Allocate, Init and register WWAN device + * @ipc_imem: Pointer to imem data-struct + * @dev: Pointer to device structure + * + * Returns: Pointer to instance on success else NULL + */ +struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev); + +/** + * ipc_wwan_deinit - Unregister and free WWAN device, clear pointer + * @ipc_wwan: Pointer to wwan instance data + */ +void ipc_wwan_deinit(struct iosm_wwan *ipc_wwan); + +/** + * ipc_wwan_receive - Receive a downlink packet from CP. + * @ipc_wwan: Pointer to wwan instance + * @skb_arg: Pointer to struct sk_buff + * @dss: Set to true if interafce id is from 257 to 261, + * else false + * @if_id: Interface ID + * + * Return: 0 on success and failure value on error + */ +int ipc_wwan_receive(struct iosm_wwan *ipc_wwan, struct sk_buff *skb_arg, + bool dss, int if_id); + +/** + * ipc_wwan_tx_flowctrl - Enable/Disable TX flow control + * @ipc_wwan: Pointer to wwan instance + * @id: Ipc mux channel session id + * @on: if true then flow ctrl would be enabled else disable + * + */ +void ipc_wwan_tx_flowctrl(struct iosm_wwan *ipc_wwan, int id, bool on); + +/** + * ipc_wwan_is_tx_stopped - Checks if Tx stopped for a Interface id. + * @ipc_wwan: Pointer to wwan instance + * @id: Ipc mux channel session id + * + * Return: true if stopped, false otherwise + */ +bool ipc_wwan_is_tx_stopped(struct iosm_wwan *ipc_wwan, int id); + +#endif
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@intel.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