[PATCH net-next v2 00/10] net: WWAN link creation improvements

[PATCH net-next v2 00/10] net: WWAN link creation improvements

[Sergey Ryazanov]

This series is intended to make the WWAN network links management easier
for WWAN device drivers.

The series begins with adding support for network links creation to the
WWAN HW simulator to facilitate code testing. Then there are a couple of
changes that prepe the WWAN core code for further modifications. The
following patches (4-6) simplify driver unregistering procedures by
performing the created links cleanup in the WWAN core. 7th patch is to
avoid the odd hold of a driver module. Next patches (8th and 9th) make
it easier for drivers to create a network interface for a default data
channel. Finally, 10th patch adds support for reporting of data link
(aka channel aka context) id to make user aware which network
interface is bound to which WWAN device data channel.

All core changes have been tested with the HW simulator. The MHI and
IOSM drivers were only compile tested as I have no access to this
hardware. So the coresponding patches require ACK from the driver
authors.

Changelog:
  v1 -> v2:
    * rebased on top of latest net-next
    * patch that reworks the creation of mhi_net default netdev was
      dropped; as Loic explained, this network device has different
      purpose depending on a driver mode; Loic has a plan to rework the
      mhi_net driver, so we will defer the default netdev creation
      reworkings
    * add a new patch that creates a default network interface for IOSM
      modems
    * 7th, 8th, 10th patches have a minor updates (see the patches for
      details)

Sergey Ryazanov (10):
  wwan_hwsim: support network interface creation
  wwan: core: relocate ops registering code
  wwan: core: require WWAN netdev setup callback existence
  wwan: core: multiple netdevs deletion support
  wwan: core: remove all netdevs on ops unregistering
  net: iosm: drop custom netdev(s) removing
  wwan: core: no more hold netdev ops owning module
  wwan: core: support default netdev creation
  net: iosm: create default link via WWAN core
  wwan: core: add WWAN common private data for netdev

 drivers/net/mhi/net.c                     |  18 +-
 drivers/net/mhi/proto_mbim.c              |   5 +-
 drivers/net/wwan/iosm/iosm_ipc_imem_ops.h |   3 +
 drivers/net/wwan/iosm/iosm_ipc_wwan.c     |  31 +--
 drivers/net/wwan/wwan_core.c              | 258 ++++++++++++++++------
 drivers/net/wwan/wwan_hwsim.c             |  47 ++++
 include/linux/wwan.h                      |  28 ++-
 7 files changed, 282 insertions(+), 108 deletions(-)

-- 
2.26.3

[PATCH net-next v2 01/10] wwan_hwsim: support network interface creation

[Sergey Ryazanov]

Add support for networking interface creation via the WWAN core by
registering the WWAN netdev creation ops for each simulated WWAN device.
Implemented minimalistic netdev support where the xmit callback just
consumes all egress skbs.

This should help with WWAN network interfaces creation testing.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

v1 -> v2:
 * no changes

 drivers/net/wwan/wwan_hwsim.c | 48 +++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/net/wwan/wwan_hwsim.c b/drivers/net/wwan/wwan_hwsim.c
index 472cae544a2b..c1e850b9c087 100644
--- a/drivers/net/wwan/wwan_hwsim.c
+++ b/drivers/net/wwan/wwan_hwsim.c
@@ -14,10 +14,13 @@
 #include <linux/spinlock.h>
 #include <linux/list.h>
 #include <linux/skbuff.h>
+#include <linux/netdevice.h>
 #include <linux/wwan.h>
 #include <linux/debugfs.h>
 #include <linux/workqueue.h>
 
+#include <net/arp.h>
+
 static int wwan_hwsim_devsnum = 2;
 module_param_named(devices, wwan_hwsim_devsnum, int, 0444);
 MODULE_PARM_DESC(devices, "Number of simulated devices");
@@ -64,6 +67,38 @@ static const struct file_operations wwan_hwsim_debugfs_devdestroy_fops;
 static void wwan_hwsim_port_del_work(struct work_struct *work);
 static void wwan_hwsim_dev_del_work(struct work_struct *work);
 
+static netdev_tx_t wwan_hwsim_netdev_xmit(struct sk_buff *skb,
+					  struct net_device *ndev)
+{
+	ndev->stats.tx_packets++;
+	ndev->stats.tx_bytes += skb->len;
+	consume_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops wwan_hwsim_netdev_ops = {
+	.ndo_start_xmit = wwan_hwsim_netdev_xmit,
+};
+
+static void wwan_hwsim_netdev_setup(struct net_device *ndev)
+{
+	ndev->netdev_ops = &wwan_hwsim_netdev_ops;
+	ndev->needs_free_netdev = true;
+
+	ndev->mtu = ETH_DATA_LEN;
+	ndev->min_mtu = ETH_MIN_MTU;
+	ndev->max_mtu = ETH_MAX_MTU;
+
+	ndev->type = ARPHRD_NONE;
+	ndev->flags = IFF_POINTOPOINT | IFF_NOARP;
+}
+
+static const struct wwan_ops wwan_hwsim_wwan_rtnl_ops = {
+	.owner = THIS_MODULE,
+	.priv_size = 0,			/* No private data */
+	.setup = wwan_hwsim_netdev_setup,
+};
+
 static int wwan_hwsim_port_start(struct wwan_port *wport)
 {
 	struct wwan_hwsim_port *port = wwan_port_get_drvdata(wport);
@@ -254,6 +289,10 @@ static struct wwan_hwsim_dev *wwan_hwsim_dev_new(void)
 
 	INIT_WORK(&dev->del_work, wwan_hwsim_dev_del_work);
 
+	err = wwan_register_ops(&dev->dev, &wwan_hwsim_wwan_rtnl_ops, dev);
+	if (err)
+		goto err_unreg_dev;
+
 	dev->debugfs_topdir = debugfs_create_dir(dev_name(&dev->dev),
 						 wwan_hwsim_debugfs_topdir);
 	debugfs_create_file("destroy", 0200, dev->debugfs_topdir, dev,
@@ -265,6 +304,12 @@ static struct wwan_hwsim_dev *wwan_hwsim_dev_new(void)
 
 	return dev;
 
+err_unreg_dev:
+	device_unregister(&dev->dev);
+	/* Memory will be freed in the device release callback */
+
+	return ERR_PTR(err);
+
 err_free_dev:
 	kfree(dev);
 
@@ -290,6 +335,9 @@ static void wwan_hwsim_dev_del(struct wwan_hwsim_dev *dev)
 
 	debugfs_remove(dev->debugfs_topdir);
 
+	/* This will remove all child netdev(s) */
+	wwan_unregister_ops(&dev->dev);
+
 	/* Make sure that there is no pending deletion work */
 	if (current_work() != &dev->del_work)
 		cancel_work_sync(&dev->del_work);
-- 
2.26.3

[PATCH net-next v2 02/10] wwan: core: relocate ops registering code

[Sergey Ryazanov]

It is unlikely that RTNL callbacks will call WWAN ops (un-)register
functions, but it is highly likely that the ops (un-)register functions
will use RTNL link create/destroy handlers. So move the WWAN network
interface ops (un-)register functions below the RTNL callbacks to be
able to call them without forward declarations.

No functional changes, just code relocation.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

v1 -> v2:
 * no changes

 drivers/net/wwan/wwan_core.c | 142 +++++++++++++++++------------------
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 165afec1dbd1..688a7278a396 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -789,77 +789,6 @@ static const struct file_operations wwan_port_fops = {
 	.llseek = noop_llseek,
 };
 
-/**
- * wwan_register_ops - register WWAN device ops
- * @parent: Device to use as parent and shared by all WWAN ports and
- *	created netdevs
- * @ops: operations to register
- * @ctxt: context to pass to operations
- *
- * Returns: 0 on success, a negative error code on failure
- */
-int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
-		      void *ctxt)
-{
-	struct wwan_device *wwandev;
-
-	if (WARN_ON(!parent || !ops))
-		return -EINVAL;
-
-	wwandev = wwan_create_dev(parent);
-	if (!wwandev)
-		return -ENOMEM;
-
-	if (WARN_ON(wwandev->ops)) {
-		wwan_remove_dev(wwandev);
-		return -EBUSY;
-	}
-
-	if (!try_module_get(ops->owner)) {
-		wwan_remove_dev(wwandev);
-		return -ENODEV;
-	}
-
-	wwandev->ops = ops;
-	wwandev->ops_ctxt = ctxt;
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(wwan_register_ops);
-
-/**
- * wwan_unregister_ops - remove WWAN device ops
- * @parent: Device to use as parent and shared by all WWAN ports and
- *	created netdevs
- */
-void wwan_unregister_ops(struct device *parent)
-{
-	struct wwan_device *wwandev = wwan_dev_get_by_parent(parent);
-	bool has_ops;
-
-	if (WARN_ON(IS_ERR(wwandev)))
-		return;
-
-	has_ops = wwandev->ops;
-
-	/* put the reference obtained by wwan_dev_get_by_parent(),
-	 * we should still have one (that the owner is giving back
-	 * now) due to the ops being assigned, check that below
-	 * and return if not.
-	 */
-	put_device(&wwandev->dev);
-
-	if (WARN_ON(!has_ops))
-		return;
-
-	module_put(wwandev->ops->owner);
-
-	wwandev->ops = NULL;
-	wwandev->ops_ctxt = NULL;
-	wwan_remove_dev(wwandev);
-}
-EXPORT_SYMBOL_GPL(wwan_unregister_ops);
-
 static int wwan_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
 			      struct netlink_ext_ack *extack)
 {
@@ -974,6 +903,77 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
 	.policy = wwan_rtnl_policy,
 };
 
+/**
+ * wwan_register_ops - register WWAN device ops
+ * @parent: Device to use as parent and shared by all WWAN ports and
+ *	created netdevs
+ * @ops: operations to register
+ * @ctxt: context to pass to operations
+ *
+ * Returns: 0 on success, a negative error code on failure
+ */
+int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
+		      void *ctxt)
+{
+	struct wwan_device *wwandev;
+
+	if (WARN_ON(!parent || !ops))
+		return -EINVAL;
+
+	wwandev = wwan_create_dev(parent);
+	if (!wwandev)
+		return -ENOMEM;
+
+	if (WARN_ON(wwandev->ops)) {
+		wwan_remove_dev(wwandev);
+		return -EBUSY;
+	}
+
+	if (!try_module_get(ops->owner)) {
+		wwan_remove_dev(wwandev);
+		return -ENODEV;
+	}
+
+	wwandev->ops = ops;
+	wwandev->ops_ctxt = ctxt;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wwan_register_ops);
+
+/**
+ * wwan_unregister_ops - remove WWAN device ops
+ * @parent: Device to use as parent and shared by all WWAN ports and
+ *	created netdevs
+ */
+void wwan_unregister_ops(struct device *parent)
+{
+	struct wwan_device *wwandev = wwan_dev_get_by_parent(parent);
+	bool has_ops;
+
+	if (WARN_ON(IS_ERR(wwandev)))
+		return;
+
+	has_ops = wwandev->ops;
+
+	/* put the reference obtained by wwan_dev_get_by_parent(),
+	 * we should still have one (that the owner is giving back
+	 * now) due to the ops being assigned, check that below
+	 * and return if not.
+	 */
+	put_device(&wwandev->dev);
+
+	if (WARN_ON(!has_ops))
+		return;
+
+	module_put(wwandev->ops->owner);
+
+	wwandev->ops = NULL;
+	wwandev->ops_ctxt = NULL;
+	wwan_remove_dev(wwandev);
+}
+EXPORT_SYMBOL_GPL(wwan_unregister_ops);
+
 static int __init wwan_init(void)
 {
 	int err;
-- 
2.26.3

[PATCH net-next v2 03/10] wwan: core: require WWAN netdev setup callback existence

[Sergey Ryazanov]

The setup callback will be unconditionally passed to the
alloc_netdev_mqs(), where the NULL pointer dereference will cause the
kernel panic. So refuse to register WWAN netdev ops with warning
generation if the setup callback is not provided.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

v1 -> v2:
 * no changes

 drivers/net/wwan/wwan_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 688a7278a396..1bd472195813 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -917,7 +917,7 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
 {
 	struct wwan_device *wwandev;
 
-	if (WARN_ON(!parent || !ops))
+	if (WARN_ON(!parent || !ops || !ops->setup))
 		return -EINVAL;
 
 	wwandev = wwan_create_dev(parent);
-- 
2.26.3

[PATCH net-next v2 04/10] wwan: core: multiple netdevs deletion support

[Sergey Ryazanov]

Use unregister_netdevice_queue() instead of simple
unregister_netdevice() if the WWAN netdev ops does not provide a dellink
callback. This will help to accelerate deletion of multiple netdevs.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

v1 -> v2:
 * no changes

 drivers/net/wwan/wwan_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 1bd472195813..b6b9c52f617c 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -882,7 +882,7 @@ static void wwan_rtnl_dellink(struct net_device *dev, struct list_head *head)
 	if (wwandev->ops->dellink)
 		wwandev->ops->dellink(wwandev->ops_ctxt, dev, head);
 	else
-		unregister_netdevice(dev);
+		unregister_netdevice_queue(dev, head);
 
 out:
 	/* release the reference */
-- 
2.26.3

[PATCH net-next v2 05/10] wwan: core: remove all netdevs on ops unregistering

[Sergey Ryazanov]

We use the ops owner module hold to protect against ops memory
disappearing. But this approach does not protect us from a driver that
unregisters ops but forgets to remove netdev(s) that were created using
this ops. In such case, we are left with netdev(s), which can not be
removed since ops is gone. Moreover, batch netdevs removing on
deinitialization is a desireable option for WWAN drivers as it is a
quite common task.

Implement deletion of all created links on WWAN netdev ops unregistering
in the same way that RTNL removes all links on RTNL ops unregistering.
Simply remove all child netdevs of a device whose WWAN netdev ops is
unregistering. This way we protecting the kernel from buggy drivers and
make it easier to write a driver deinitialization code.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

v1 -> v2:
 * no changes

 drivers/net/wwan/wwan_core.c | 40 ++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index b6b9c52f617c..ec6a69b23dd1 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -941,6 +941,17 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
 }
 EXPORT_SYMBOL_GPL(wwan_register_ops);
 
+/* Enqueue child netdev deletion */
+static int wwan_child_dellink(struct device *dev, void *data)
+{
+	struct list_head *kill_list = data;
+
+	if (dev->type == &wwan_type)
+		wwan_rtnl_dellink(to_net_dev(dev), kill_list);
+
+	return 0;
+}
+
 /**
  * wwan_unregister_ops - remove WWAN device ops
  * @parent: Device to use as parent and shared by all WWAN ports and
@@ -949,26 +960,37 @@ EXPORT_SYMBOL_GPL(wwan_register_ops);
 void wwan_unregister_ops(struct device *parent)
 {
 	struct wwan_device *wwandev = wwan_dev_get_by_parent(parent);
-	bool has_ops;
+	struct module *owner;
+	LIST_HEAD(kill_list);
 
 	if (WARN_ON(IS_ERR(wwandev)))
 		return;
-
-	has_ops = wwandev->ops;
+	if (WARN_ON(!wwandev->ops)) {
+		put_device(&wwandev->dev);
+		return;
+	}
 
 	/* put the reference obtained by wwan_dev_get_by_parent(),
 	 * we should still have one (that the owner is giving back
-	 * now) due to the ops being assigned, check that below
-	 * and return if not.
+	 * now) due to the ops being assigned.
 	 */
 	put_device(&wwandev->dev);
 
-	if (WARN_ON(!has_ops))
-		return;
+	owner = wwandev->ops->owner;	/* Preserve ops owner */
+
+	rtnl_lock();	/* Prevent concurent netdev(s) creation/destroying */
+
+	/* Remove all child netdev(s), using batch removing */
+	device_for_each_child(&wwandev->dev, &kill_list,
+			      wwan_child_dellink);
+	unregister_netdevice_many(&kill_list);
+
+	wwandev->ops = NULL;	/* Finally remove ops */
+
+	rtnl_unlock();
 
-	module_put(wwandev->ops->owner);
+	module_put(owner);
 
-	wwandev->ops = NULL;
 	wwandev->ops_ctxt = NULL;
 	wwan_remove_dev(wwandev);
 }
-- 
2.26.3

[PATCH net-next v2 06/10] net: iosm: drop custom netdev(s) removing

[Sergey Ryazanov]

Since the last commit, the WWAN core will remove all our network
interfaces for us at the time of the WWAN netdev ops unregistering.
Therefore, we can safely drop the custom code that cleans the list of
created netdevs. Anyway it no longer removes any netdev, since all
netdevs were removed earlier in the wwan_unregister_ops() call.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Reviewed-by: M Chetan Kumar <m.chetan.kumar@intel.com>
CC: M Chetan Kumar <m.chetan.kumar@intel.com>
CC: Intel Corporation <linuxwwan@intel.com>
---

v1 -> v2:
 * no changes

 drivers/net/wwan/iosm/iosm_ipc_wwan.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
index 1711b79fc616..bee9b278223d 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
@@ -329,22 +329,9 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
 
 void ipc_wwan_deinit(struct iosm_wwan *ipc_wwan)
 {
-	int if_id;
-
+	/* This call will remove all child netdev(s) */
 	wwan_unregister_ops(ipc_wwan->dev);
 
-	for (if_id = 0; if_id < ARRAY_SIZE(ipc_wwan->sub_netlist); if_id++) {
-		struct iosm_netdev_priv *priv;
-
-		priv = rcu_access_pointer(ipc_wwan->sub_netlist[if_id]);
-		if (!priv)
-			continue;
-
-		rtnl_lock();
-		ipc_wwan_dellink(ipc_wwan, priv->netdev, NULL);
-		rtnl_unlock();
-	}
-
 	mutex_destroy(&ipc_wwan->if_mutex);
 
 	kfree(ipc_wwan);
-- 
2.26.3

[PATCH net-next v2 07/10] wwan: core: no more hold netdev ops owning module

[Sergey Ryazanov]

The WWAN netdev ops owner holding was used to protect from the
unexpected memory disappear. This approach causes a dependency cycle
(driver -> core -> driver) and effectively prevents a WWAN driver
unloading. E.g. WWAN hwsim could not be unloaded until all simulated
devices are removed:

~# modprobe wwan_hwsim devices=2
~# lsmod | grep wwan
wwan_hwsim             16384  2
wwan                   20480  1 wwan_hwsim
~# rmmod wwan_hwsim
rmmod: ERROR: Module wwan_hwsim is in use
~# echo > /sys/kernel/debug/wwan_hwsim/hwsim0/destroy
~# echo > /sys/kernel/debug/wwan_hwsim/hwsim1/destroy
~# lsmod | grep wwan
wwan_hwsim             16384  0
wwan                   20480  1 wwan_hwsim
~# rmmod wwan_hwsim

For a real device driver this will cause an inability to unload module
until a served device is physically detached.

Since the last commit we are removing all child netdev(s) when a driver
unregister the netdev ops. This allows us to permit the driver
unloading, since any sane driver will call ops unregistering on a device
deinitialization. So, remove the holding of an ops owner to make it
easier to unload a driver module. The owner field has also beed removed
from the ops structure as there are no more users of this field.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

v1 -> v2:
 * fix a comment before wwan_unregister_ops() in the mhi/net.c

 drivers/net/mhi/net.c         |  3 +--
 drivers/net/wwan/wwan_core.c  | 10 ----------
 drivers/net/wwan/wwan_hwsim.c |  1 -
 include/linux/wwan.h          |  2 --
 4 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
index 6aa753387372..ffd1c01b3f35 100644
--- a/drivers/net/mhi/net.c
+++ b/drivers/net/mhi/net.c
@@ -383,7 +383,6 @@ static void mhi_net_dellink(void *ctxt, struct net_device *ndev,
 }
 
 static const struct wwan_ops mhi_wwan_ops = {
-	.owner = THIS_MODULE,
 	.priv_size = sizeof(struct mhi_net_dev),
 	.setup = mhi_net_setup,
 	.newlink = mhi_net_newlink,
@@ -436,7 +435,7 @@ static void mhi_net_remove(struct mhi_device *mhi_dev)
 	struct mhi_net_dev *mhi_netdev = dev_get_drvdata(&mhi_dev->dev);
 	struct mhi_controller *cntrl = mhi_dev->mhi_cntrl;
 
-	/* rtnetlink takes care of removing remaining links */
+	/* WWAN core takes care of removing remaining links */
 	wwan_unregister_ops(&cntrl->mhi_dev->dev);
 
 	if (create_default_iface)
diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index ec6a69b23dd1..b634a0ba1196 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -929,11 +929,6 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
 		return -EBUSY;
 	}
 
-	if (!try_module_get(ops->owner)) {
-		wwan_remove_dev(wwandev);
-		return -ENODEV;
-	}
-
 	wwandev->ops = ops;
 	wwandev->ops_ctxt = ctxt;
 
@@ -960,7 +955,6 @@ static int wwan_child_dellink(struct device *dev, void *data)
 void wwan_unregister_ops(struct device *parent)
 {
 	struct wwan_device *wwandev = wwan_dev_get_by_parent(parent);
-	struct module *owner;
 	LIST_HEAD(kill_list);
 
 	if (WARN_ON(IS_ERR(wwandev)))
@@ -976,8 +970,6 @@ void wwan_unregister_ops(struct device *parent)
 	 */
 	put_device(&wwandev->dev);
 
-	owner = wwandev->ops->owner;	/* Preserve ops owner */
-
 	rtnl_lock();	/* Prevent concurent netdev(s) creation/destroying */
 
 	/* Remove all child netdev(s), using batch removing */
@@ -989,8 +981,6 @@ void wwan_unregister_ops(struct device *parent)
 
 	rtnl_unlock();
 
-	module_put(owner);
-
 	wwandev->ops_ctxt = NULL;
 	wwan_remove_dev(wwandev);
 }
diff --git a/drivers/net/wwan/wwan_hwsim.c b/drivers/net/wwan/wwan_hwsim.c
index c1e850b9c087..a8582a58a385 100644
--- a/drivers/net/wwan/wwan_hwsim.c
+++ b/drivers/net/wwan/wwan_hwsim.c
@@ -94,7 +94,6 @@ static void wwan_hwsim_netdev_setup(struct net_device *ndev)
 }
 
 static const struct wwan_ops wwan_hwsim_wwan_rtnl_ops = {
-	.owner = THIS_MODULE,
 	.priv_size = 0,			/* No private data */
 	.setup = wwan_hwsim_netdev_setup,
 };
diff --git a/include/linux/wwan.h b/include/linux/wwan.h
index 34222230360c..e1981ea3a2fd 100644
--- a/include/linux/wwan.h
+++ b/include/linux/wwan.h
@@ -128,14 +128,12 @@ void *wwan_port_get_drvdata(struct wwan_port *port);
 
 /**
  * struct wwan_ops - WWAN device ops
- * @owner: module owner of the WWAN ops
  * @priv_size: size of private netdev data area
  * @setup: set up a new netdev
  * @newlink: register the new netdev
  * @dellink: remove the given netdev
  */
 struct wwan_ops {
-	struct module *owner;
 	unsigned int priv_size;
 	void (*setup)(struct net_device *dev);
 	int (*newlink)(void *ctxt, struct net_device *dev,
-- 
2.26.3

[PATCH net-next v2 08/10] wwan: core: support default netdev creation

[Sergey Ryazanov]

Most, if not each WWAN device driver will create a netdev for the
default data channel. Therefore, add an option for the WWAN netdev ops
registration function to create a default netdev for the WWAN device.

A WWAN device driver should pass a default data channel link id to the
ops registering function to request the creation of a default netdev, or
a special value WWAN_NO_DEFAULT_LINK to inform the WWAN core that the
default netdev should not be created.

For now, only wwan_hwsim utilize the default link creation option. Other
drivers will be reworked next.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
CC: M Chetan Kumar <m.chetan.kumar@intel.com>
CC: Intel Corporation <linuxwwan@intel.com>
---

v1 -> v2:
 * fix a comment style '/**' -> '/*' to avoid confusion with kernel-doc

 drivers/net/mhi/net.c                 |  3 +-
 drivers/net/wwan/iosm/iosm_ipc_wwan.c |  3 +-
 drivers/net/wwan/wwan_core.c          | 75 ++++++++++++++++++++++++++-
 drivers/net/wwan/wwan_hwsim.c         |  2 +-
 include/linux/wwan.h                  |  8 ++-
 5 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
index ffd1c01b3f35..f36ca5c0dfe9 100644
--- a/drivers/net/mhi/net.c
+++ b/drivers/net/mhi/net.c
@@ -397,7 +397,8 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
 	struct net_device *ndev;
 	int err;
 
-	err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev);
+	err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev,
+				WWAN_NO_DEFAULT_LINK);
 	if (err)
 		return err;
 
diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
index bee9b278223d..adb2bd40a404 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
@@ -317,7 +317,8 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
 	ipc_wwan->dev = dev;
 	ipc_wwan->ipc_imem = ipc_imem;
 
-	if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan)) {
+	if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan,
+			      WWAN_NO_DEFAULT_LINK)) {
 		kfree(ipc_wwan);
 		return NULL;
 	}
diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index b634a0ba1196..ef6ec641d877 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -903,17 +903,81 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
 	.policy = wwan_rtnl_policy,
 };
 
+static void wwan_create_default_link(struct wwan_device *wwandev,
+				     u32 def_link_id)
+{
+	struct nlattr *tb[IFLA_MAX + 1], *linkinfo[IFLA_INFO_MAX + 1];
+	struct nlattr *data[IFLA_WWAN_MAX + 1];
+	struct net_device *dev;
+	struct nlmsghdr *nlh;
+	struct sk_buff *msg;
+
+	/* Forge attributes required to create a WWAN netdev. We first
+	 * build a netlink message and then parse it. This looks
+	 * odd, but such approach is less error prone.
+	 */
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (WARN_ON(!msg))
+		return;
+	nlh = nlmsg_put(msg, 0, 0, RTM_NEWLINK, 0, 0);
+	if (WARN_ON(!nlh))
+		goto free_attrs;
+
+	if (nla_put_string(msg, IFLA_PARENT_DEV_NAME, dev_name(&wwandev->dev)))
+		goto free_attrs;
+	tb[IFLA_LINKINFO] = nla_nest_start(msg, IFLA_LINKINFO);
+	if (!tb[IFLA_LINKINFO])
+		goto free_attrs;
+	linkinfo[IFLA_INFO_DATA] = nla_nest_start(msg, IFLA_INFO_DATA);
+	if (!linkinfo[IFLA_INFO_DATA])
+		goto free_attrs;
+	if (nla_put_u32(msg, IFLA_WWAN_LINK_ID, def_link_id))
+		goto free_attrs;
+	nla_nest_end(msg, linkinfo[IFLA_INFO_DATA]);
+	nla_nest_end(msg, tb[IFLA_LINKINFO]);
+
+	nlmsg_end(msg, nlh);
+
+	/* The next three parsing calls can not fail */
+	nlmsg_parse_deprecated(nlh, 0, tb, IFLA_MAX, NULL, NULL);
+	nla_parse_nested_deprecated(linkinfo, IFLA_INFO_MAX, tb[IFLA_LINKINFO],
+				    NULL, NULL);
+	nla_parse_nested_deprecated(data, IFLA_WWAN_MAX,
+				    linkinfo[IFLA_INFO_DATA], NULL, NULL);
+
+	rtnl_lock();
+
+	dev = rtnl_create_link(&init_net, "wwan%d", NET_NAME_ENUM,
+			       &wwan_rtnl_link_ops, tb, NULL);
+	if (WARN_ON(IS_ERR(dev)))
+		goto unlock;
+
+	if (WARN_ON(wwan_rtnl_newlink(&init_net, dev, tb, data, NULL))) {
+		free_netdev(dev);
+		goto unlock;
+	}
+
+unlock:
+	rtnl_unlock();
+
+free_attrs:
+	nlmsg_free(msg);
+}
+
 /**
  * wwan_register_ops - register WWAN device ops
  * @parent: Device to use as parent and shared by all WWAN ports and
  *	created netdevs
  * @ops: operations to register
  * @ctxt: context to pass to operations
+ * @def_link_id: id of the default link that will be automatically created by
+ *	the WWAN core for the WWAN device. The default link will not be created
+ *	if the passed value is WWAN_NO_DEFAULT_LINK.
  *
  * Returns: 0 on success, a negative error code on failure
  */
 int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
-		      void *ctxt)
+		      void *ctxt, u32 def_link_id)
 {
 	struct wwan_device *wwandev;
 
@@ -932,6 +996,15 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
 	wwandev->ops = ops;
 	wwandev->ops_ctxt = ctxt;
 
+	/* NB: we do not abort ops registration in case of default link
+	 * creation failure. Link ops is the management interface, while the
+	 * default link creation is a service option. And we should not prevent
+	 * a user from manually creating a link latter if service option failed
+	 * now.
+	 */
+	if (def_link_id != WWAN_NO_DEFAULT_LINK)
+		wwan_create_default_link(wwandev, def_link_id);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(wwan_register_ops);
diff --git a/drivers/net/wwan/wwan_hwsim.c b/drivers/net/wwan/wwan_hwsim.c
index a8582a58a385..5b62cf3b3c42 100644
--- a/drivers/net/wwan/wwan_hwsim.c
+++ b/drivers/net/wwan/wwan_hwsim.c
@@ -288,7 +288,7 @@ static struct wwan_hwsim_dev *wwan_hwsim_dev_new(void)
 
 	INIT_WORK(&dev->del_work, wwan_hwsim_dev_del_work);
 
-	err = wwan_register_ops(&dev->dev, &wwan_hwsim_wwan_rtnl_ops, dev);
+	err = wwan_register_ops(&dev->dev, &wwan_hwsim_wwan_rtnl_ops, dev, 1);
 	if (err)
 		goto err_unreg_dev;
 
diff --git a/include/linux/wwan.h b/include/linux/wwan.h
index e1981ea3a2fd..91590db70a12 100644
--- a/include/linux/wwan.h
+++ b/include/linux/wwan.h
@@ -126,6 +126,12 @@ void wwan_port_txon(struct wwan_port *port);
  */
 void *wwan_port_get_drvdata(struct wwan_port *port);
 
+/*
+ * Used to indicate that the WWAN core should not create a default network
+ * link.
+ */
+#define WWAN_NO_DEFAULT_LINK		U32_MAX
+
 /**
  * struct wwan_ops - WWAN device ops
  * @priv_size: size of private netdev data area
@@ -143,7 +149,7 @@ struct wwan_ops {
 };
 
 int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
-		      void *ctxt);
+		      void *ctxt, u32 def_link_id);
 
 void wwan_unregister_ops(struct device *parent);
 
-- 
2.26.3

[PATCH net-next v2 09/10] net: iosm: create default link via WWAN core

[Sergey Ryazanov]

Utilize the just introduced WWAN core feature to create a default netdev
for the default data (IP MUX) channel.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
CC: M Chetan Kumar <m.chetan.kumar@intel.com>
CC: Intel Corporation <linuxwwan@intel.com>
---

v1 -> v2:
 * new patch

 drivers/net/wwan/iosm/iosm_ipc_imem_ops.h | 3 +++
 drivers/net/wwan/iosm/iosm_ipc_wwan.c     | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h
index 84087cf33329..fd356dafbdd6 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.h
@@ -30,6 +30,9 @@
 #define IP_MUX_SESSION_START 1
 #define IP_MUX_SESSION_END 8
 
+/* Default IP MUX channel */
+#define IP_MUX_SESSION_DEFAULT	1
+
 /**
  * ipc_imem_sys_port_open - Open a port link to CP.
  * @ipc_imem:	Imem instance.
diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
index adb2bd40a404..d3cb28107836 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
@@ -317,8 +317,9 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
 	ipc_wwan->dev = dev;
 	ipc_wwan->ipc_imem = ipc_imem;
 
+	/* WWAN core will create a netdev for the default IP MUX channel */
 	if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan,
-			      WWAN_NO_DEFAULT_LINK)) {
+			      IP_MUX_SESSION_DEFAULT)) {
 		kfree(ipc_wwan);
 		return NULL;
 	}
-- 
2.26.3

[PATCH net-next v2 10/10] wwan: core: add WWAN common private data for netdev

[Sergey Ryazanov]

The WWAN core not only multiplex the netdev configuration data, but
process it too, and needs some space to store its private data
associated with the netdev. Add a structure to keep common WWAN core
data. The structure will be stored inside the netdev private data before
WWAN driver private data and have a field to make it easier to access
the driver data. Also add a helper function that simplifies drivers
access to their data.

At the moment we use the common WWAN private data to store the WWAN data
link (channel) id at the time the link is created, and report it back to
user using the .fill_info() RTNL callback. This should help the user to
be aware which network interface is bound to which WWAN device data
channel.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
CC: M Chetan Kumar <m.chetan.kumar@intel.com>
CC: Intel Corporation <linuxwwan@intel.com>
---

v1 -> v2:
 * fix grammar in description

 drivers/net/mhi/net.c                 | 12 +++++------
 drivers/net/mhi/proto_mbim.c          |  5 +++--
 drivers/net/wwan/iosm/iosm_ipc_wwan.c | 12 +++++------
 drivers/net/wwan/wwan_core.c          | 29 ++++++++++++++++++++++++++-
 include/linux/wwan.h                  | 18 +++++++++++++++++
 5 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
index f36ca5c0dfe9..e60e38c1f09d 100644
--- a/drivers/net/mhi/net.c
+++ b/drivers/net/mhi/net.c
@@ -32,7 +32,7 @@ struct mhi_device_info {
 
 static int mhi_ndo_open(struct net_device *ndev)
 {
-	struct mhi_net_dev *mhi_netdev = netdev_priv(ndev);
+	struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev);
 
 	/* Feed the rx buffer pool */
 	schedule_delayed_work(&mhi_netdev->rx_refill, 0);
@@ -47,7 +47,7 @@ static int mhi_ndo_open(struct net_device *ndev)
 
 static int mhi_ndo_stop(struct net_device *ndev)
 {
-	struct mhi_net_dev *mhi_netdev = netdev_priv(ndev);
+	struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev);
 
 	netif_stop_queue(ndev);
 	netif_carrier_off(ndev);
@@ -58,7 +58,7 @@ static int mhi_ndo_stop(struct net_device *ndev)
 
 static netdev_tx_t mhi_ndo_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
-	struct mhi_net_dev *mhi_netdev = netdev_priv(ndev);
+	struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev);
 	const struct mhi_net_proto *proto = mhi_netdev->proto;
 	struct mhi_device *mdev = mhi_netdev->mdev;
 	int err;
@@ -93,7 +93,7 @@ static netdev_tx_t mhi_ndo_xmit(struct sk_buff *skb, struct net_device *ndev)
 static void mhi_ndo_get_stats64(struct net_device *ndev,
 				struct rtnl_link_stats64 *stats)
 {
-	struct mhi_net_dev *mhi_netdev = netdev_priv(ndev);
+	struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev);
 	unsigned int start;
 
 	do {
@@ -322,7 +322,7 @@ static int mhi_net_newlink(void *ctxt, struct net_device *ndev, u32 if_id,
 	if (dev_get_drvdata(&mhi_dev->dev))
 		return -EBUSY;
 
-	mhi_netdev = netdev_priv(ndev);
+	mhi_netdev = wwan_netdev_drvpriv(ndev);
 
 	dev_set_drvdata(&mhi_dev->dev, mhi_netdev);
 	mhi_netdev->ndev = ndev;
@@ -367,7 +367,7 @@ static int mhi_net_newlink(void *ctxt, struct net_device *ndev, u32 if_id,
 static void mhi_net_dellink(void *ctxt, struct net_device *ndev,
 			    struct list_head *head)
 {
-	struct mhi_net_dev *mhi_netdev = netdev_priv(ndev);
+	struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev);
 	struct mhi_device *mhi_dev = ctxt;
 
 	if (head)
diff --git a/drivers/net/mhi/proto_mbim.c b/drivers/net/mhi/proto_mbim.c
index fc72b3f6ec9e..bf1ad863237d 100644
--- a/drivers/net/mhi/proto_mbim.c
+++ b/drivers/net/mhi/proto_mbim.c
@@ -16,6 +16,7 @@
 #include <linux/ip.h>
 #include <linux/mii.h>
 #include <linux/netdevice.h>
+#include <linux/wwan.h>
 #include <linux/skbuff.h>
 #include <linux/usb.h>
 #include <linux/usb/cdc.h>
@@ -56,7 +57,7 @@ static void __mbim_errors_inc(struct mhi_net_dev *dev)
 
 static int mbim_rx_verify_nth16(struct sk_buff *skb)
 {
-	struct mhi_net_dev *dev = netdev_priv(skb->dev);
+	struct mhi_net_dev *dev = wwan_netdev_drvpriv(skb->dev);
 	struct mbim_context *ctx = dev->proto_data;
 	struct usb_cdc_ncm_nth16 *nth16;
 	int len;
@@ -102,7 +103,7 @@ static int mbim_rx_verify_nth16(struct sk_buff *skb)
 
 static int mbim_rx_verify_ndp16(struct sk_buff *skb, struct usb_cdc_ncm_ndp16 *ndp16)
 {
-	struct mhi_net_dev *dev = netdev_priv(skb->dev);
+	struct mhi_net_dev *dev = wwan_netdev_drvpriv(skb->dev);
 	int ret;
 
 	if (le16_to_cpu(ndp16->wLength) < USB_CDC_NCM_NDP16_LENGTH_MIN) {
diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
index d3cb28107836..c999c64001f4 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
@@ -20,7 +20,7 @@
 #define IOSM_IF_ID_PAYLOAD 2
 
 /**
- * struct iosm_netdev_priv - netdev private data
+ * struct iosm_netdev_priv - netdev WWAN driver specific private data
  * @ipc_wwan:	Pointer to iosm_wwan struct
  * @netdev:	Pointer to network interface device structure
  * @if_id:	Interface id for device.
@@ -51,7 +51,7 @@ struct iosm_wwan {
 /* Bring-up the wwan net link */
 static int ipc_wwan_link_open(struct net_device *netdev)
 {
-	struct iosm_netdev_priv *priv = netdev_priv(netdev);
+	struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(netdev);
 	struct iosm_wwan *ipc_wwan = priv->ipc_wwan;
 	int if_id = priv->if_id;
 	int ret;
@@ -88,7 +88,7 @@ static int ipc_wwan_link_open(struct net_device *netdev)
 /* Bring-down the wwan net link */
 static int ipc_wwan_link_stop(struct net_device *netdev)
 {
-	struct iosm_netdev_priv *priv = netdev_priv(netdev);
+	struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(netdev);
 
 	netif_stop_queue(netdev);
 
@@ -105,7 +105,7 @@ static int ipc_wwan_link_stop(struct net_device *netdev)
 static int ipc_wwan_link_transmit(struct sk_buff *skb,
 				  struct net_device *netdev)
 {
-	struct iosm_netdev_priv *priv = netdev_priv(netdev);
+	struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(netdev);
 	struct iosm_wwan *ipc_wwan = priv->ipc_wwan;
 	int if_id = priv->if_id;
 	int ret;
@@ -178,7 +178,7 @@ static int ipc_wwan_newlink(void *ctxt, struct net_device *dev,
 	    if_id >= ARRAY_SIZE(ipc_wwan->sub_netlist))
 		return -EINVAL;
 
-	priv = netdev_priv(dev);
+	priv = wwan_netdev_drvpriv(dev);
 	priv->if_id = if_id;
 	priv->netdev = dev;
 	priv->ipc_wwan = ipc_wwan;
@@ -208,8 +208,8 @@ static int ipc_wwan_newlink(void *ctxt, struct net_device *dev,
 static void ipc_wwan_dellink(void *ctxt, struct net_device *dev,
 			     struct list_head *head)
 {
+	struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(dev);
 	struct iosm_wwan *ipc_wwan = ctxt;
-	struct iosm_netdev_priv *priv = netdev_priv(dev);
 	int if_id = priv->if_id;
 
 	if (WARN_ON(if_id < IP_MUX_SESSION_START ||
diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index ef6ec641d877..3e16c318e705 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -815,6 +815,7 @@ static struct net_device *wwan_rtnl_alloc(struct nlattr *tb[],
 	const char *devname = nla_data(tb[IFLA_PARENT_DEV_NAME]);
 	struct wwan_device *wwandev = wwan_dev_get_by_name(devname);
 	struct net_device *dev;
+	unsigned int priv_size;
 
 	if (IS_ERR(wwandev))
 		return ERR_CAST(wwandev);
@@ -825,7 +826,8 @@ static struct net_device *wwan_rtnl_alloc(struct nlattr *tb[],
 		goto out;
 	}
 
-	dev = alloc_netdev_mqs(wwandev->ops->priv_size, ifname, name_assign_type,
+	priv_size = sizeof(struct wwan_netdev_priv) + wwandev->ops->priv_size;
+	dev = alloc_netdev_mqs(priv_size, ifname, name_assign_type,
 			       wwandev->ops->setup, num_tx_queues, num_rx_queues);
 
 	if (dev) {
@@ -845,6 +847,7 @@ static int wwan_rtnl_newlink(struct net *src_net, struct net_device *dev,
 {
 	struct wwan_device *wwandev = wwan_dev_get_by_parent(dev->dev.parent);
 	u32 link_id = nla_get_u32(data[IFLA_WWAN_LINK_ID]);
+	struct wwan_netdev_priv *priv = netdev_priv(dev);
 	int ret;
 
 	if (IS_ERR(wwandev))
@@ -856,6 +859,7 @@ static int wwan_rtnl_newlink(struct net *src_net, struct net_device *dev,
 		goto out;
 	}
 
+	priv->link_id = link_id;
 	if (wwandev->ops->newlink)
 		ret = wwandev->ops->newlink(wwandev->ops_ctxt, dev,
 					    link_id, extack);
@@ -889,6 +893,27 @@ static void wwan_rtnl_dellink(struct net_device *dev, struct list_head *head)
 	put_device(&wwandev->dev);
 }
 
+static size_t wwan_rtnl_get_size(const struct net_device *dev)
+{
+	return
+		nla_total_size(4) +	/* IFLA_WWAN_LINK_ID */
+		0;
+}
+
+static int wwan_rtnl_fill_info(struct sk_buff *skb,
+			       const struct net_device *dev)
+{
+	struct wwan_netdev_priv *priv = netdev_priv(dev);
+
+	if (nla_put_u32(skb, IFLA_WWAN_LINK_ID, priv->link_id))
+		goto nla_put_failure;
+
+	return 0;
+
+nla_put_failure:
+	return -EMSGSIZE;
+}
+
 static const struct nla_policy wwan_rtnl_policy[IFLA_WWAN_MAX + 1] = {
 	[IFLA_WWAN_LINK_ID] = { .type = NLA_U32 },
 };
@@ -900,6 +925,8 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
 	.validate = wwan_rtnl_validate,
 	.newlink = wwan_rtnl_newlink,
 	.dellink = wwan_rtnl_dellink,
+	.get_size = wwan_rtnl_get_size,
+	.fill_info = wwan_rtnl_fill_info,
 	.policy = wwan_rtnl_policy,
 };
 
diff --git a/include/linux/wwan.h b/include/linux/wwan.h
index 91590db70a12..9fac819f92e3 100644
--- a/include/linux/wwan.h
+++ b/include/linux/wwan.h
@@ -9,6 +9,7 @@
 #include <linux/poll.h>
 #include <linux/skbuff.h>
 #include <linux/netlink.h>
+#include <linux/netdevice.h>
 
 /**
  * enum wwan_port_type - WWAN port types
@@ -126,6 +127,23 @@ void wwan_port_txon(struct wwan_port *port);
  */
 void *wwan_port_get_drvdata(struct wwan_port *port);
 
+/**
+ * struct wwan_netdev_priv - WWAN core network device private data
+ * @link_id: WWAN device data link id
+ * @drv_priv: driver private data area, size is determined in &wwan_ops
+ */
+struct wwan_netdev_priv {
+	u32 link_id;
+
+	/* must be last */
+	u8 drv_priv[] __aligned(sizeof(void *));
+};
+
+static inline void *wwan_netdev_drvpriv(struct net_device *dev)
+{
+	return ((struct wwan_netdev_priv *)netdev_priv(dev))->drv_priv;
+}
+
 /*
  * Used to indicate that the WWAN core should not create a default network
  * link.
-- 
2.26.3

Re: [PATCH net-next v2 03/10] wwan: core: require WWAN netdev setup callback existence

[Loic Poulain]

On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>
> The setup callback will be unconditionally passed to the
> alloc_netdev_mqs(), where the NULL pointer dereference will cause the
> kernel panic. So refuse to register WWAN netdev ops with warning
> generation if the setup callback is not provided.
>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Re: [PATCH net-next v2 04/10] wwan: core: multiple netdevs deletion support

[Loic Poulain]

On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>
> Use unregister_netdevice_queue() instead of simple
> unregister_netdevice() if the WWAN netdev ops does not provide a dellink
> callback. This will help to accelerate deletion of multiple netdevs.
>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Re: [PATCH net-next v2 05/10] wwan: core: remove all netdevs on ops unregistering

[Loic Poulain]

On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>
> We use the ops owner module hold to protect against ops memory
> disappearing. But this approach does not protect us from a driver that
> unregisters ops but forgets to remove netdev(s) that were created using
> this ops. In such case, we are left with netdev(s), which can not be
> removed since ops is gone. Moreover, batch netdevs removing on
> deinitialization is a desireable option for WWAN drivers as it is a
> quite common task.
>
> Implement deletion of all created links on WWAN netdev ops unregistering
> in the same way that RTNL removes all links on RTNL ops unregistering.
> Simply remove all child netdevs of a device whose WWAN netdev ops is
> unregistering. This way we protecting the kernel from buggy drivers and
> make it easier to write a driver deinitialization code.
>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Re: [PATCH net-next v2 07/10] wwan: core: no more hold netdev ops owning module

[Loic Poulain]

On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>
> The WWAN netdev ops owner holding was used to protect from the
> unexpected memory disappear. This approach causes a dependency cycle
> (driver -> core -> driver) and effectively prevents a WWAN driver
> unloading. E.g. WWAN hwsim could not be unloaded until all simulated
> devices are removed:
>
> ~# modprobe wwan_hwsim devices=2
> ~# lsmod | grep wwan
> wwan_hwsim             16384  2
> wwan                   20480  1 wwan_hwsim
> ~# rmmod wwan_hwsim
> rmmod: ERROR: Module wwan_hwsim is in use
> ~# echo > /sys/kernel/debug/wwan_hwsim/hwsim0/destroy
> ~# echo > /sys/kernel/debug/wwan_hwsim/hwsim1/destroy
> ~# lsmod | grep wwan
> wwan_hwsim             16384  0
> wwan                   20480  1 wwan_hwsim
> ~# rmmod wwan_hwsim
>
> For a real device driver this will cause an inability to unload module
> until a served device is physically detached.
>
> Since the last commit we are removing all child netdev(s) when a driver
> unregister the netdev ops. This allows us to permit the driver
> unloading, since any sane driver will call ops unregistering on a device
> deinitialization. So, remove the holding of an ops owner to make it
> easier to unload a driver module. The owner field has also beed removed
> from the ops structure as there are no more users of this field.
>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Re: [PATCH net-next v2 08/10] wwan: core: support default netdev creation

[Loic Poulain]

Hi Sergey,

On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>
> Most, if not each WWAN device driver will create a netdev for the
> default data channel. Therefore, add an option for the WWAN netdev ops
> registration function to create a default netdev for the WWAN device.
>
> A WWAN device driver should pass a default data channel link id to the
> ops registering function to request the creation of a default netdev, or
> a special value WWAN_NO_DEFAULT_LINK to inform the WWAN core that the
> default netdev should not be created.
>
> For now, only wwan_hwsim utilize the default link creation option. Other
> drivers will be reworked next.
>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
> CC: M Chetan Kumar <m.chetan.kumar@intel.com>
> CC: Intel Corporation <linuxwwan@intel.com>
> ---
>
> v1 -> v2:
>  * fix a comment style '/**' -> '/*' to avoid confusion with kernel-doc
>
>  drivers/net/mhi/net.c                 |  3 +-
>  drivers/net/wwan/iosm/iosm_ipc_wwan.c |  3 +-
>  drivers/net/wwan/wwan_core.c          | 75 ++++++++++++++++++++++++++-
>  drivers/net/wwan/wwan_hwsim.c         |  2 +-
>  include/linux/wwan.h                  |  8 ++-
>  5 files changed, 86 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
> index ffd1c01b3f35..f36ca5c0dfe9 100644
> --- a/drivers/net/mhi/net.c
> +++ b/drivers/net/mhi/net.c
> @@ -397,7 +397,8 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
>         struct net_device *ndev;
>         int err;
>
> -       err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev);
> +       err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev,
> +                               WWAN_NO_DEFAULT_LINK);
>         if (err)
>                 return err;
>
> diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> index bee9b278223d..adb2bd40a404 100644
> --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> @@ -317,7 +317,8 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
>         ipc_wwan->dev = dev;
>         ipc_wwan->ipc_imem = ipc_imem;
>
> -       if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan)) {
> +       if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan,
> +                             WWAN_NO_DEFAULT_LINK)) {
>                 kfree(ipc_wwan);
>                 return NULL;
>         }
> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> index b634a0ba1196..ef6ec641d877 100644
> --- a/drivers/net/wwan/wwan_core.c
> +++ b/drivers/net/wwan/wwan_core.c
> @@ -903,17 +903,81 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
>         .policy = wwan_rtnl_policy,
>  };
>
> +static void wwan_create_default_link(struct wwan_device *wwandev,
> +                                    u32 def_link_id)
> +{
> +       struct nlattr *tb[IFLA_MAX + 1], *linkinfo[IFLA_INFO_MAX + 1];
> +       struct nlattr *data[IFLA_WWAN_MAX + 1];
> +       struct net_device *dev;
> +       struct nlmsghdr *nlh;
> +       struct sk_buff *msg;
> +
> +       /* Forge attributes required to create a WWAN netdev. We first
> +        * build a netlink message and then parse it. This looks
> +        * odd, but such approach is less error prone.
> +        */
> +       msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +       if (WARN_ON(!msg))
> +               return;
> +       nlh = nlmsg_put(msg, 0, 0, RTM_NEWLINK, 0, 0);
> +       if (WARN_ON(!nlh))
> +               goto free_attrs;
> +
> +       if (nla_put_string(msg, IFLA_PARENT_DEV_NAME, dev_name(&wwandev->dev)))
> +               goto free_attrs;
> +       tb[IFLA_LINKINFO] = nla_nest_start(msg, IFLA_LINKINFO);
> +       if (!tb[IFLA_LINKINFO])
> +               goto free_attrs;
> +       linkinfo[IFLA_INFO_DATA] = nla_nest_start(msg, IFLA_INFO_DATA);
> +       if (!linkinfo[IFLA_INFO_DATA])
> +               goto free_attrs;
> +       if (nla_put_u32(msg, IFLA_WWAN_LINK_ID, def_link_id))
> +               goto free_attrs;
> +       nla_nest_end(msg, linkinfo[IFLA_INFO_DATA]);
> +       nla_nest_end(msg, tb[IFLA_LINKINFO]);
> +
> +       nlmsg_end(msg, nlh);
> +
> +       /* The next three parsing calls can not fail */
> +       nlmsg_parse_deprecated(nlh, 0, tb, IFLA_MAX, NULL, NULL);
> +       nla_parse_nested_deprecated(linkinfo, IFLA_INFO_MAX, tb[IFLA_LINKINFO],
> +                                   NULL, NULL);
> +       nla_parse_nested_deprecated(data, IFLA_WWAN_MAX,
> +                                   linkinfo[IFLA_INFO_DATA], NULL, NULL);
> +
> +       rtnl_lock();
> +
> +       dev = rtnl_create_link(&init_net, "wwan%d", NET_NAME_ENUM,
> +                              &wwan_rtnl_link_ops, tb, NULL);

That can be a bit confusing since the same naming convention as for
the WWAN devices is used, so you can end with something like a wwan0
netdev being a link of wwan1 dev. Maybe something like ('%s.%d',
dev_name(&wwandev->dev), link_id) to have e.g. wwan1.0 for link 0 of
the wwan1 device. Or alternatively, the specific wwan driver to
specify the default name to use.

Regards,
Loic

Re: [PATCH net-next v2 08/10] wwan: core: support default netdev creation

[Sergey Ryazanov]

Hi Loic,

On Tue, Jun 22, 2021 at 4:55 PM Loic Poulain <loic.poulain@linaro.org> wrote:
> On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>> Most, if not each WWAN device driver will create a netdev for the
>> default data channel. Therefore, add an option for the WWAN netdev ops
>> registration function to create a default netdev for the WWAN device.
>>
>> A WWAN device driver should pass a default data channel link id to the
>> ops registering function to request the creation of a default netdev, or
>> a special value WWAN_NO_DEFAULT_LINK to inform the WWAN core that the
>> default netdev should not be created.
>>
>> For now, only wwan_hwsim utilize the default link creation option. Other
>> drivers will be reworked next.
>>
>> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
>> CC: M Chetan Kumar <m.chetan.kumar@intel.com>
>> CC: Intel Corporation <linuxwwan@intel.com>
>> ---
>>
>> v1 -> v2:
>>  * fix a comment style '/**' -> '/*' to avoid confusion with kernel-doc
>>
>>  drivers/net/mhi/net.c                 |  3 +-
>>  drivers/net/wwan/iosm/iosm_ipc_wwan.c |  3 +-
>>  drivers/net/wwan/wwan_core.c          | 75 ++++++++++++++++++++++++++-
>>  drivers/net/wwan/wwan_hwsim.c         |  2 +-
>>  include/linux/wwan.h                  |  8 ++-
>>  5 files changed, 86 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
>> index ffd1c01b3f35..f36ca5c0dfe9 100644
>> --- a/drivers/net/mhi/net.c
>> +++ b/drivers/net/mhi/net.c
>> @@ -397,7 +397,8 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
>>         struct net_device *ndev;
>>         int err;
>>
>> -       err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev);
>> +       err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev,
>> +                               WWAN_NO_DEFAULT_LINK);
>>         if (err)
>>                 return err;
>>
>> diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
>> index bee9b278223d..adb2bd40a404 100644
>> --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
>> +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
>> @@ -317,7 +317,8 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
>>         ipc_wwan->dev = dev;
>>         ipc_wwan->ipc_imem = ipc_imem;
>>
>> -       if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan)) {
>> +       if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan,
>> +                             WWAN_NO_DEFAULT_LINK)) {
>>                 kfree(ipc_wwan);
>>                 return NULL;
>>         }
>> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
>> index b634a0ba1196..ef6ec641d877 100644
>> --- a/drivers/net/wwan/wwan_core.c
>> +++ b/drivers/net/wwan/wwan_core.c
>> @@ -903,17 +903,81 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
>>         .policy = wwan_rtnl_policy,
>>  };
>>
>> +static void wwan_create_default_link(struct wwan_device *wwandev,
>> +                                    u32 def_link_id)
>> +{
>> +       struct nlattr *tb[IFLA_MAX + 1], *linkinfo[IFLA_INFO_MAX + 1];
>> +       struct nlattr *data[IFLA_WWAN_MAX + 1];
>> +       struct net_device *dev;
>> +       struct nlmsghdr *nlh;
>> +       struct sk_buff *msg;
>> +
>> +       /* Forge attributes required to create a WWAN netdev. We first
>> +        * build a netlink message and then parse it. This looks
>> +        * odd, but such approach is less error prone.
>> +        */
>> +       msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>> +       if (WARN_ON(!msg))
>> +               return;
>> +       nlh = nlmsg_put(msg, 0, 0, RTM_NEWLINK, 0, 0);
>> +       if (WARN_ON(!nlh))
>> +               goto free_attrs;
>> +
>> +       if (nla_put_string(msg, IFLA_PARENT_DEV_NAME, dev_name(&wwandev->dev)))
>> +               goto free_attrs;
>> +       tb[IFLA_LINKINFO] = nla_nest_start(msg, IFLA_LINKINFO);
>> +       if (!tb[IFLA_LINKINFO])
>> +               goto free_attrs;
>> +       linkinfo[IFLA_INFO_DATA] = nla_nest_start(msg, IFLA_INFO_DATA);
>> +       if (!linkinfo[IFLA_INFO_DATA])
>> +               goto free_attrs;
>> +       if (nla_put_u32(msg, IFLA_WWAN_LINK_ID, def_link_id))
>> +               goto free_attrs;
>> +       nla_nest_end(msg, linkinfo[IFLA_INFO_DATA]);
>> +       nla_nest_end(msg, tb[IFLA_LINKINFO]);
>> +
>> +       nlmsg_end(msg, nlh);
>> +
>> +       /* The next three parsing calls can not fail */
>> +       nlmsg_parse_deprecated(nlh, 0, tb, IFLA_MAX, NULL, NULL);
>> +       nla_parse_nested_deprecated(linkinfo, IFLA_INFO_MAX, tb[IFLA_LINKINFO],
>> +                                   NULL, NULL);
>> +       nla_parse_nested_deprecated(data, IFLA_WWAN_MAX,
>> +                                   linkinfo[IFLA_INFO_DATA], NULL, NULL);
>> +
>> +       rtnl_lock();
>> +
>> +       dev = rtnl_create_link(&init_net, "wwan%d", NET_NAME_ENUM,
>> +                              &wwan_rtnl_link_ops, tb, NULL);
>
> That can be a bit confusing since the same naming convention as for
> the WWAN devices is used, so you can end with something like a wwan0
> netdev being a link of wwan1 dev. Maybe something like ('%s.%d',
> dev_name(&wwandev->dev), link_id) to have e.g. wwan1.0 for link 0 of
> the wwan1 device. Or alternatively, the specific wwan driver to
> specify the default name to use.

Yeah. Having wwan1 netdev for wwan0 device with control port
/dev/wwan0mbim0 could be йuite confusing for a _user_. While the
ModemManager daemon will keep track of parent<->child relations
without considering device names.

It is a permanent pain to manually find the right modem control port.
Even if you only have one modem, but have a UART-USB converter
attached before the modem, then /dev/ttyUSB0, which previously
belonged to a first modem AT-port, can become the UART-USB device, and
the first modem AT-port can become /dev/ttyUSB1. We never have
guarantees for consistent naming, and user space software should take
care of the matching and predictable names :(

wwanX.Y names look attractive. But how many users will actually use
multiple data channels and create wwanX.2, wwanX.3, etc. netdevs?
Moreover, some cellular modems do not support multiple data channels
and will never do so.

I assume that a typical usage scenario for an average user is a single
modem connected to a laptop with a single data channel for Internet
access. In this case, the user will not have a chance to see the wwan1
netdev. And no confusion is possible in practice. Even more, seeing a
wwan0.1 netdev when you have a single modem with a single data channel
will be a more confusing case. Those who will use a more complex setup
should be ready for some complexity. In fact, for a more complex setup
(e.g. if you have a pair of modems) you will need some sort of
management software (or udev scripts) since you can never be sure that
a USB modem connected to a first USB port, will always be wwan0.

Even if you have a couple of modems of different models (one works
as-is and the other uses the WWAN core), it will be a big surprise for
the user to find that the wwan0.1 netdev is not a second connection
via the first modem, but it is a main connection of the second modem
:)

And finally, I assume that in the mid-term, we will switch all WWAN
device drivers to the WWAN core usage, and the WWAN core will be the
only producer of wwanX devices. As well as all Wi-Fi device drivers
have been migrated to cfg80211/mac80211 usage. In that context,
introducing the wwanX.Y naming scheme now and changing it back to the
wwanX later will be some kind of API breakage.

With all this in mind, I chose the wwanX as the naming template for
the default data channel netdev.

-- 
Sergey

Re: [PATCH net-next v2 08/10] wwan: core: support default netdev creation

[Loic Poulain]

On Tue, 22 Jun 2021 at 17:19, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>
> Hi Loic,
>
> On Tue, Jun 22, 2021 at 4:55 PM Loic Poulain <loic.poulain@linaro.org> wrote:
> > On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
> >> Most, if not each WWAN device driver will create a netdev for the
> >> default data channel. Therefore, add an option for the WWAN netdev ops
> >> registration function to create a default netdev for the WWAN device.
> >>
> >> A WWAN device driver should pass a default data channel link id to the
> >> ops registering function to request the creation of a default netdev, or
> >> a special value WWAN_NO_DEFAULT_LINK to inform the WWAN core that the
> >> default netdev should not be created.
> >>
> >> For now, only wwan_hwsim utilize the default link creation option. Other
> >> drivers will be reworked next.
> >>
> >> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
> >> CC: M Chetan Kumar <m.chetan.kumar@intel.com>
> >> CC: Intel Corporation <linuxwwan@intel.com>
> >> ---
> >>
> >> v1 -> v2:
> >>  * fix a comment style '/**' -> '/*' to avoid confusion with kernel-doc
> >>
> >>  drivers/net/mhi/net.c                 |  3 +-
> >>  drivers/net/wwan/iosm/iosm_ipc_wwan.c |  3 +-
> >>  drivers/net/wwan/wwan_core.c          | 75 ++++++++++++++++++++++++++-
> >>  drivers/net/wwan/wwan_hwsim.c         |  2 +-
> >>  include/linux/wwan.h                  |  8 ++-
> >>  5 files changed, 86 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
> >> index ffd1c01b3f35..f36ca5c0dfe9 100644
> >> --- a/drivers/net/mhi/net.c
> >> +++ b/drivers/net/mhi/net.c
> >> @@ -397,7 +397,8 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
> >>         struct net_device *ndev;
> >>         int err;
> >>
> >> -       err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev);
> >> +       err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev,
> >> +                               WWAN_NO_DEFAULT_LINK);
> >>         if (err)
> >>                 return err;
> >>
> >> diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> >> index bee9b278223d..adb2bd40a404 100644
> >> --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> >> +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> >> @@ -317,7 +317,8 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
> >>         ipc_wwan->dev = dev;
> >>         ipc_wwan->ipc_imem = ipc_imem;
> >>
> >> -       if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan)) {
> >> +       if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan,
> >> +                             WWAN_NO_DEFAULT_LINK)) {
> >>                 kfree(ipc_wwan);
> >>                 return NULL;
> >>         }
> >> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> >> index b634a0ba1196..ef6ec641d877 100644
> >> --- a/drivers/net/wwan/wwan_core.c
> >> +++ b/drivers/net/wwan/wwan_core.c
> >> @@ -903,17 +903,81 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
> >>         .policy = wwan_rtnl_policy,
> >>  };
> >>
> >> +static void wwan_create_default_link(struct wwan_device *wwandev,
> >> +                                    u32 def_link_id)
> >> +{
> >> +       struct nlattr *tb[IFLA_MAX + 1], *linkinfo[IFLA_INFO_MAX + 1];
> >> +       struct nlattr *data[IFLA_WWAN_MAX + 1];
> >> +       struct net_device *dev;
> >> +       struct nlmsghdr *nlh;
> >> +       struct sk_buff *msg;
> >> +
> >> +       /* Forge attributes required to create a WWAN netdev. We first
> >> +        * build a netlink message and then parse it. This looks
> >> +        * odd, but such approach is less error prone.
> >> +        */
> >> +       msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> >> +       if (WARN_ON(!msg))
> >> +               return;
> >> +       nlh = nlmsg_put(msg, 0, 0, RTM_NEWLINK, 0, 0);
> >> +       if (WARN_ON(!nlh))
> >> +               goto free_attrs;
> >> +
> >> +       if (nla_put_string(msg, IFLA_PARENT_DEV_NAME, dev_name(&wwandev->dev)))
> >> +               goto free_attrs;
> >> +       tb[IFLA_LINKINFO] = nla_nest_start(msg, IFLA_LINKINFO);
> >> +       if (!tb[IFLA_LINKINFO])
> >> +               goto free_attrs;
> >> +       linkinfo[IFLA_INFO_DATA] = nla_nest_start(msg, IFLA_INFO_DATA);
> >> +       if (!linkinfo[IFLA_INFO_DATA])
> >> +               goto free_attrs;
> >> +       if (nla_put_u32(msg, IFLA_WWAN_LINK_ID, def_link_id))
> >> +               goto free_attrs;
> >> +       nla_nest_end(msg, linkinfo[IFLA_INFO_DATA]);
> >> +       nla_nest_end(msg, tb[IFLA_LINKINFO]);
> >> +
> >> +       nlmsg_end(msg, nlh);
> >> +
> >> +       /* The next three parsing calls can not fail */
> >> +       nlmsg_parse_deprecated(nlh, 0, tb, IFLA_MAX, NULL, NULL);
> >> +       nla_parse_nested_deprecated(linkinfo, IFLA_INFO_MAX, tb[IFLA_LINKINFO],
> >> +                                   NULL, NULL);
> >> +       nla_parse_nested_deprecated(data, IFLA_WWAN_MAX,
> >> +                                   linkinfo[IFLA_INFO_DATA], NULL, NULL);
> >> +
> >> +       rtnl_lock();
> >> +
> >> +       dev = rtnl_create_link(&init_net, "wwan%d", NET_NAME_ENUM,
> >> +                              &wwan_rtnl_link_ops, tb, NULL);
> >
> > That can be a bit confusing since the same naming convention as for
> > the WWAN devices is used, so you can end with something like a wwan0
> > netdev being a link of wwan1 dev. Maybe something like ('%s.%d',
> > dev_name(&wwandev->dev), link_id) to have e.g. wwan1.0 for link 0 of
> > the wwan1 device. Or alternatively, the specific wwan driver to
> > specify the default name to use.
>
> Yeah. Having wwan1 netdev for wwan0 device with control port
> /dev/wwan0mbim0 could be йuite confusing for a _user_. While the
> ModemManager daemon will keep track of parent<->child relations
> without considering device names.
>
> It is a permanent pain to manually find the right modem control port.
> Even if you only have one modem, but have a UART-USB converter
> attached before the modem, then /dev/ttyUSB0, which previously
> belonged to a first modem AT-port, can become the UART-USB device, and
> the first modem AT-port can become /dev/ttyUSB1. We never have
> guarantees for consistent naming, and user space software should take
> care of the matching and predictable names :(
>
> wwanX.Y names look attractive. But how many users will actually use
> multiple data channels and create wwanX.2, wwanX.3, etc. netdevs?
> Moreover, some cellular modems do not support multiple data channels
> and will never do so.
>
> I assume that a typical usage scenario for an average user is a single
> modem connected to a laptop with a single data channel for Internet
> access. In this case, the user will not have a chance to see the wwan1
> netdev. And no confusion is possible in practice. Even more, seeing a
> wwan0.1 netdev when you have a single modem with a single data channel
> will be a more confusing case. Those who will use a more complex setup
> should be ready for some complexity. In fact, for a more complex setup
> (e.g. if you have a pair of modems) you will need some sort of
> management software (or udev scripts) since you can never be sure that
> a USB modem connected to a first USB port, will always be wwan0.
>
> Even if you have a couple of modems of different models (one works
> as-is and the other uses the WWAN core), it will be a big surprise for
> the user to find that the wwan0.1 netdev is not a second connection
> via the first modem, but it is a main connection of the second modem
> :)
>
> And finally, I assume that in the mid-term, we will switch all WWAN
> device drivers to the WWAN core usage, and the WWAN core will be the
> only producer of wwanX devices. As well as all Wi-Fi device drivers
> have been migrated to cfg80211/mac80211 usage. In that context,
> introducing the wwanX.Y naming scheme now and changing it back to the
> wwanX later will be some kind of API breakage.
>
> With all this in mind, I chose the wwanX as the naming template for
> the default data channel netdev.

Ok, you've convinced me... USB WWAN netdev drivers also use this
naming (wwan%d), so if we switch them to WWAN framework at some point,
it will not disturb users (though netdev name should not be considered
as something stable).

Regards,
Loic

Re: [PATCH net-next v2 00/10] net: WWAN link creation improvements

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Tue, 22 Jun 2021 01:50:50 +0300 you wrote:
> This series is intended to make the WWAN network links management easier
> for WWAN device drivers.
> 
> The series begins with adding support for network links creation to the
> WWAN HW simulator to facilitate code testing. Then there are a couple of
> changes that prepe the WWAN core code for further modifications. The
> following patches (4-6) simplify driver unregistering procedures by
> performing the created links cleanup in the WWAN core. 7th patch is to
> avoid the odd hold of a driver module. Next patches (8th and 9th) make
> it easier for drivers to create a network interface for a default data
> channel. Finally, 10th patch adds support for reporting of data link
> (aka channel aka context) id to make user aware which network
> interface is bound to which WWAN device data channel.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,01/10] wwan_hwsim: support network interface creation
    https://git.kernel.org/netdev/net-next/c/f842f48891ad
  - [net-next,v2,02/10] wwan: core: relocate ops registering code
    https://git.kernel.org/netdev/net-next/c/355a4e7e0a23
  - [net-next,v2,03/10] wwan: core: require WWAN netdev setup callback existence
    https://git.kernel.org/netdev/net-next/c/58c3b421c62e
  - [net-next,v2,04/10] wwan: core: multiple netdevs deletion support
    https://git.kernel.org/netdev/net-next/c/f492fccf3d62
  - [net-next,v2,05/10] wwan: core: remove all netdevs on ops unregistering
    https://git.kernel.org/netdev/net-next/c/2f75238014f0
  - [net-next,v2,06/10] net: iosm: drop custom netdev(s) removing
    https://git.kernel.org/netdev/net-next/c/322a0ba99c50
  - [net-next,v2,07/10] wwan: core: no more hold netdev ops owning module
    https://git.kernel.org/netdev/net-next/c/9f0248ea476e
  - [net-next,v2,08/10] wwan: core: support default netdev creation
    https://git.kernel.org/netdev/net-next/c/ca374290aaad
  - [net-next,v2,09/10] net: iosm: create default link via WWAN core
    https://git.kernel.org/netdev/net-next/c/83068395bbfc
  - [net-next,v2,10/10] wwan: core: add WWAN common private data for netdev
    https://git.kernel.org/netdev/net-next/c/699409240389

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html