netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] qmi_wwan: fix QMAP handling
@ 2019-06-12 17:01 Reinhard Speyerer
  2019-06-12 17:02 ` [PATCH 1/4] qmi_wwan: add support for QMAP padding in the RX path Reinhard Speyerer
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Reinhard Speyerer @ 2019-06-12 17:01 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Daniele Palmas, netdev, rspmn

This series addresses the following issues observed when using the
QMAP support of the qmi_wwan driver:

1. The QMAP code in the qmi_wwan driver is based on the CodeAurora
   GobiNet driver ([1], [2]) which does not process QMAP padding
   in the RX path correctly. This causes qmimux_rx_fixup() to pass
   incorrect data to the IP stack when padding is used.

2. qmimux devices currently lack proper network device usage statistics.

3. RCU stalls on device disconnect with QMAP activated like this

   # echo Y > /sys/class/net/wwan0/qmi/raw_ip 
   # echo 1 > /sys/class/net/wwan0/qmi/add_mux
   # echo 2 > /sys/class/net/wwan0/qmi/add_mux
   # echo 3 > /sys/class/net/wwan0/qmi/add_mux

   have been observed in certain setups:

   [ 2273.676593] option1 ttyUSB16: GSM modem (1-port) converter now disconnected from ttyUSB16
   [ 2273.676617] option 6-1.2:1.0: device disconnected
   [ 2273.676774] WARNING: CPU: 1 PID: 141 at kernel/rcu/tree_plugin.h:342 rcu_note_context_switch+0x2a/0x3d0
   [ 2273.676776] Modules linked in: option qmi_wwan cdc_mbim cdc_ncm qcserial cdc_wdm usb_wwan sierra sierra_net usbnet mii edd coretemp iptable_mangle ip6_tables iptable_filter ip_tables cdc_acm dm_mod dax iTCO_wdt evdev iTCO_vendor_support sg ftdi_sio usbserial e1000e ptp pps_core i2c_i801 ehci_pci button lpc_ich i2c_core mfd_core uhci_hcd ehci_hcd rtc_cmos usbcore usb_common sd_mod fan ata_piix thermal
   [ 2273.676817] CPU: 1 PID: 141 Comm: kworker/1:1 Not tainted 4.19.38-rsp-1 #1
   [ 2273.676819] Hardware name: Not Applicable   Not Applicable  /CX-GS/GM45-GL40             , BIOS V1.11      03/23/2011
   [ 2273.676828] Workqueue: usb_hub_wq hub_event [usbcore]
   [ 2273.676832] EIP: rcu_note_context_switch+0x2a/0x3d0
   [ 2273.676834] Code: 55 89 e5 57 56 89 c6 53 83 ec 14 89 45 f0 e8 5d ff ff ff 89 f0 64 8b 3d 24 a6 86 c0 84 c0 8b 87 04 02 00 00 75 7a 85 c0 7e 7a <0f> 0b 80 bf 08 02 00 00 00 0f 84 87 00 00 00 e8 b2 e2 ff ff bb dc
   [ 2273.676836] EAX: 00000001 EBX: f614bc00 ECX: 00000001 EDX: c0715b81
   [ 2273.676838] ESI: 00000000 EDI: f18beb40 EBP: f1a3dc20 ESP: f1a3dc00
   [ 2273.676840] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00010002
   [ 2273.676842] CR0: 80050033 CR2: b7e97230 CR3: 2f9c4000 CR4: 000406b0
   [ 2273.676843] Call Trace:
   [ 2273.676847]  ? preempt_count_add+0xa5/0xc0
   [ 2273.676852]  __schedule+0x4e/0x4f0
   [ 2273.676855]  ? __queue_work+0xf1/0x2a0
   [ 2273.676858]  ? _raw_spin_lock_irqsave+0x14/0x40
   [ 2273.676860]  ? preempt_count_add+0x52/0xc0
   [ 2273.676862]  schedule+0x33/0x80
   [ 2273.676865]  _synchronize_rcu_expedited+0x24e/0x280
   [ 2273.676867]  ? rcu_accelerate_cbs_unlocked+0x70/0x70
   [ 2273.676871]  ? wait_woken+0x70/0x70
   [ 2273.676873]  ? rcu_accelerate_cbs_unlocked+0x70/0x70
   [ 2273.676875]  ? _synchronize_rcu_expedited+0x280/0x280
   [ 2273.676877]  synchronize_rcu_expedited+0x22/0x30
   [ 2273.676881]  synchronize_net+0x25/0x30
   [ 2273.676885]  dev_deactivate_many+0x133/0x230
   [ 2273.676887]  ? preempt_count_add+0xa5/0xc0
   [ 2273.676890]  __dev_close_many+0x4d/0xc0
   [ 2273.676892]  ? skb_dequeue+0x40/0x50
   [ 2273.676895]  dev_close_many+0x5d/0xd0
   [ 2273.676898]  rollback_registered_many+0xbf/0x4c0
   [ 2273.676901]  ? raw_notifier_call_chain+0x1a/0x20
   [ 2273.676904]  ? call_netdevice_notifiers_info+0x23/0x60
   [ 2273.676906]  ? netdev_master_upper_dev_get+0xe/0x70
   [ 2273.676908]  rollback_registered+0x1f/0x30
   [ 2273.676911]  unregister_netdevice_queue+0x47/0xb0
   [ 2273.676915]  qmimux_unregister_device+0x1f/0x30 [qmi_wwan]
   [ 2273.676917]  qmi_wwan_disconnect+0x5d/0x90 [qmi_wwan]
   ...
   [ 2273.677001] ---[ end trace 0fcc5f88496b485a ]---
   [ 2294.679136] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
   [ 2294.679140] rcu: 	Tasks blocked on level-0 rcu_node (CPUs 0-1): P141
   [ 2294.679144] rcu: 	(detected by 0, t=21002 jiffies, g=265857, q=8446)
   [ 2294.679148] kworker/1:1     D    0   141      2 0x80000000


In addition the permitted QMAP mux_id value range is extended for
compatibility with ip(8) and the rmnet driver.

Reinhard

[1]: https://portland.source.codeaurora.org/patches/quic/gobi
[2]: https://portland.source.codeaurora.org/quic/qsdk/oss/lklm/gobinet/

Reinhard Speyerer (4):
  qmi_wwan: add support for QMAP padding in the RX path
  qmi_wwan: add network device usage statistics for qmimux devices
  qmi_wwan: avoid RCU stalls on device disconnect when in QMAP mode
  qmi_wwan: extend permitted QMAP mux_id value range

 Documentation/ABI/testing/sysfs-class-net-qmi |   4 +-
 drivers/net/usb/qmi_wwan.c                    | 103 ++++++++++++++++++++++----
 2 files changed, 91 insertions(+), 16 deletions(-)

-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] qmi_wwan: add support for QMAP padding in the RX path
  2019-06-12 17:01 [PATCH 0/4] qmi_wwan: fix QMAP handling Reinhard Speyerer
@ 2019-06-12 17:02 ` Reinhard Speyerer
  2019-06-12 17:02 ` [PATCH 2/4] qmi_wwan: add network device usage statistics for qmimux devices Reinhard Speyerer
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Reinhard Speyerer @ 2019-06-12 17:02 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Daniele Palmas, netdev, rspmn

The QMAP code in the qmi_wwan driver is based on the CodeAurora GobiNet
driver which does not process QMAP padding in the RX path correctly.
Add support for QMAP padding to qmimux_rx_fixup() according to the
description of the rmnet driver.

Fixes: c6adf77953bc ("net: usb: qmi_wwan: add qmap mux protocol support")
Cc: Daniele Palmas <dnlplm@gmail.com>
Signed-off-by: Reinhard Speyerer <rspmn@arcor.de>
---
 drivers/net/usb/qmi_wwan.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index d9a6699abe59..fd3d078a1923 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -153,7 +153,7 @@ static bool qmimux_has_slaves(struct usbnet *dev)
 
 static int qmimux_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
-	unsigned int len, offset = 0;
+	unsigned int len, offset = 0, pad_len, pkt_len;
 	struct qmimux_hdr *hdr;
 	struct net_device *net;
 	struct sk_buff *skbn;
@@ -171,10 +171,16 @@ static int qmimux_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 		if (hdr->pad & 0x80)
 			goto skip;
 
+		/* extract padding length and check for valid length info */
+		pad_len = hdr->pad & 0x3f;
+		if (len == 0 || pad_len >= len)
+			goto skip;
+		pkt_len = len - pad_len;
+
 		net = qmimux_find_dev(dev, hdr->mux_id);
 		if (!net)
 			goto skip;
-		skbn = netdev_alloc_skb(net, len);
+		skbn = netdev_alloc_skb(net, pkt_len);
 		if (!skbn)
 			return 0;
 		skbn->dev = net;
@@ -191,7 +197,7 @@ static int qmimux_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			goto skip;
 		}
 
-		skb_put_data(skbn, skb->data + offset + qmimux_hdr_sz, len);
+		skb_put_data(skbn, skb->data + offset + qmimux_hdr_sz, pkt_len);
 		if (netif_rx(skbn) != NET_RX_SUCCESS)
 			return 0;
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] qmi_wwan: add network device usage statistics for qmimux devices
  2019-06-12 17:01 [PATCH 0/4] qmi_wwan: fix QMAP handling Reinhard Speyerer
  2019-06-12 17:02 ` [PATCH 1/4] qmi_wwan: add support for QMAP padding in the RX path Reinhard Speyerer
@ 2019-06-12 17:02 ` Reinhard Speyerer
  2019-06-12 17:03 ` [PATCH 3/4] qmi_wwan: avoid RCU stalls on device disconnect when in QMAP mode Reinhard Speyerer
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Reinhard Speyerer @ 2019-06-12 17:02 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Daniele Palmas, netdev, rspmn

Add proper network device usage statistics for qmimux devices
instead of reporting all-zero values for them.

Fixes: c6adf77953bc ("net: usb: qmi_wwan: add qmap mux protocol support")
Cc: Daniele Palmas <dnlplm@gmail.com>
Signed-off-by: Reinhard Speyerer <rspmn@arcor.de>
---
 drivers/net/usb/qmi_wwan.c | 76 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index fd3d078a1923..b0a96459621f 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -22,6 +22,7 @@
 #include <linux/usb/cdc.h>
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc-wdm.h>
+#include <linux/u64_stats_sync.h>
 
 /* This driver supports wwan (3G/LTE/?) devices using a vendor
  * specific management protocol called Qualcomm MSM Interface (QMI) -
@@ -75,6 +76,7 @@ struct qmimux_hdr {
 struct qmimux_priv {
 	struct net_device *real_dev;
 	u8 mux_id;
+	struct pcpu_sw_netstats __percpu *stats64;
 };
 
 static int qmimux_open(struct net_device *dev)
@@ -101,19 +103,65 @@ static netdev_tx_t qmimux_start_xmit(struct sk_buff *skb, struct net_device *dev
 	struct qmimux_priv *priv = netdev_priv(dev);
 	unsigned int len = skb->len;
 	struct qmimux_hdr *hdr;
+	netdev_tx_t ret;
 
 	hdr = skb_push(skb, sizeof(struct qmimux_hdr));
 	hdr->pad = 0;
 	hdr->mux_id = priv->mux_id;
 	hdr->pkt_len = cpu_to_be16(len);
 	skb->dev = priv->real_dev;
-	return dev_queue_xmit(skb);
+	ret = dev_queue_xmit(skb);
+
+	if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
+		struct pcpu_sw_netstats *stats64 = this_cpu_ptr(priv->stats64);
+
+		u64_stats_update_begin(&stats64->syncp);
+		stats64->tx_packets++;
+		stats64->tx_bytes += len;
+		u64_stats_update_end(&stats64->syncp);
+	} else {
+		dev->stats.tx_dropped++;
+	}
+
+	return ret;
+}
+
+static void qmimux_get_stats64(struct net_device *net,
+			       struct rtnl_link_stats64 *stats)
+{
+	struct qmimux_priv *priv = netdev_priv(net);
+	unsigned int start;
+	int cpu;
+
+	netdev_stats_to_stats64(stats, &net->stats);
+
+	for_each_possible_cpu(cpu) {
+		struct pcpu_sw_netstats *stats64;
+		u64 rx_packets, rx_bytes;
+		u64 tx_packets, tx_bytes;
+
+		stats64 = per_cpu_ptr(priv->stats64, cpu);
+
+		do {
+			start = u64_stats_fetch_begin_irq(&stats64->syncp);
+			rx_packets = stats64->rx_packets;
+			rx_bytes = stats64->rx_bytes;
+			tx_packets = stats64->tx_packets;
+			tx_bytes = stats64->tx_bytes;
+		} while (u64_stats_fetch_retry_irq(&stats64->syncp, start));
+
+		stats->rx_packets += rx_packets;
+		stats->rx_bytes += rx_bytes;
+		stats->tx_packets += tx_packets;
+		stats->tx_bytes += tx_bytes;
+	}
 }
 
 static const struct net_device_ops qmimux_netdev_ops = {
-	.ndo_open       = qmimux_open,
-	.ndo_stop       = qmimux_stop,
-	.ndo_start_xmit = qmimux_start_xmit,
+	.ndo_open        = qmimux_open,
+	.ndo_stop        = qmimux_stop,
+	.ndo_start_xmit  = qmimux_start_xmit,
+	.ndo_get_stats64 = qmimux_get_stats64,
 };
 
 static void qmimux_setup(struct net_device *dev)
@@ -198,8 +246,19 @@ static int qmimux_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 		}
 
 		skb_put_data(skbn, skb->data + offset + qmimux_hdr_sz, pkt_len);
-		if (netif_rx(skbn) != NET_RX_SUCCESS)
+		if (netif_rx(skbn) != NET_RX_SUCCESS) {
+			net->stats.rx_errors++;
 			return 0;
+		} else {
+			struct pcpu_sw_netstats *stats64;
+			struct qmimux_priv *priv = netdev_priv(net);
+
+			stats64 = this_cpu_ptr(priv->stats64);
+			u64_stats_update_begin(&stats64->syncp);
+			stats64->rx_packets++;
+			stats64->rx_bytes += pkt_len;
+			u64_stats_update_end(&stats64->syncp);
+		}
 
 skip:
 		offset += len + qmimux_hdr_sz;
@@ -223,6 +282,12 @@ static int qmimux_register_device(struct net_device *real_dev, u8 mux_id)
 	priv->mux_id = mux_id;
 	priv->real_dev = real_dev;
 
+	priv->stats64 = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
+	if (!priv->stats64) {
+		err = -ENOBUFS;
+		goto out_free_newdev;
+	}
+
 	err = register_netdevice(new_dev);
 	if (err < 0)
 		goto out_free_newdev;
@@ -252,6 +317,7 @@ static void qmimux_unregister_device(struct net_device *dev)
 	struct qmimux_priv *priv = netdev_priv(dev);
 	struct net_device *real_dev = priv->real_dev;
 
+	free_percpu(priv->stats64);
 	netdev_upper_dev_unlink(real_dev, dev);
 	unregister_netdevice(dev);
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] qmi_wwan: avoid RCU stalls on device disconnect when in QMAP mode
  2019-06-12 17:01 [PATCH 0/4] qmi_wwan: fix QMAP handling Reinhard Speyerer
  2019-06-12 17:02 ` [PATCH 1/4] qmi_wwan: add support for QMAP padding in the RX path Reinhard Speyerer
  2019-06-12 17:02 ` [PATCH 2/4] qmi_wwan: add network device usage statistics for qmimux devices Reinhard Speyerer
@ 2019-06-12 17:03 ` Reinhard Speyerer
  2019-06-12 17:03 ` [PATCH 4/4] qmi_wwan: extend permitted QMAP mux_id value range Reinhard Speyerer
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Reinhard Speyerer @ 2019-06-12 17:03 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Daniele Palmas, netdev, rspmn

Switch qmimux_unregister_device() and qmi_wwan_disconnect() to
use unregister_netdevice_queue() and unregister_netdevice_many()
instead of unregister_netdevice(). This avoids RCU stalls which
have been observed on device disconnect in certain setups otherwise.

Fixes: c6adf77953bc ("net: usb: qmi_wwan: add qmap mux protocol support")
Cc: Daniele Palmas <dnlplm@gmail.com>
Signed-off-by: Reinhard Speyerer <rspmn@arcor.de>
---
 drivers/net/usb/qmi_wwan.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index b0a96459621f..c6fbc2a2a785 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -312,14 +312,15 @@ static int qmimux_register_device(struct net_device *real_dev, u8 mux_id)
 	return err;
 }
 
-static void qmimux_unregister_device(struct net_device *dev)
+static void qmimux_unregister_device(struct net_device *dev,
+				     struct list_head *head)
 {
 	struct qmimux_priv *priv = netdev_priv(dev);
 	struct net_device *real_dev = priv->real_dev;
 
 	free_percpu(priv->stats64);
 	netdev_upper_dev_unlink(real_dev, dev);
-	unregister_netdevice(dev);
+	unregister_netdevice_queue(dev, head);
 
 	/* Get rid of the reference to real_dev */
 	dev_put(real_dev);
@@ -490,7 +491,7 @@ static ssize_t del_mux_store(struct device *d,  struct device_attribute *attr, c
 		ret = -EINVAL;
 		goto err;
 	}
-	qmimux_unregister_device(del_dev);
+	qmimux_unregister_device(del_dev, NULL);
 
 	if (!qmimux_has_slaves(dev))
 		info->flags &= ~QMI_WWAN_FLAG_MUX;
@@ -1500,6 +1501,7 @@ static void qmi_wwan_disconnect(struct usb_interface *intf)
 	struct qmi_wwan_state *info;
 	struct list_head *iter;
 	struct net_device *ldev;
+	LIST_HEAD(list);
 
 	/* called twice if separate control and data intf */
 	if (!dev)
@@ -1512,8 +1514,9 @@ static void qmi_wwan_disconnect(struct usb_interface *intf)
 		}
 		rcu_read_lock();
 		netdev_for_each_upper_dev_rcu(dev->net, ldev, iter)
-			qmimux_unregister_device(ldev);
+			qmimux_unregister_device(ldev, &list);
 		rcu_read_unlock();
+		unregister_netdevice_many(&list);
 		rtnl_unlock();
 		info->flags &= ~QMI_WWAN_FLAG_MUX;
 	}
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] qmi_wwan: extend permitted QMAP mux_id value range
  2019-06-12 17:01 [PATCH 0/4] qmi_wwan: fix QMAP handling Reinhard Speyerer
                   ` (2 preceding siblings ...)
  2019-06-12 17:03 ` [PATCH 3/4] qmi_wwan: avoid RCU stalls on device disconnect when in QMAP mode Reinhard Speyerer
@ 2019-06-12 17:03 ` Reinhard Speyerer
  2019-06-13 15:14 ` [PATCH 0/4] qmi_wwan: fix QMAP handling Daniele Palmas
  2019-06-13 17:54 ` Bjørn Mork
  5 siblings, 0 replies; 8+ messages in thread
From: Reinhard Speyerer @ 2019-06-12 17:03 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Daniele Palmas, netdev, rspmn

Permit mux_id values up to 254 to be used in qmimux_register_device()
for compatibility with ip(8) and the rmnet driver.

Fixes: c6adf77953bc ("net: usb: qmi_wwan: add qmap mux protocol support")
Cc: Daniele Palmas <dnlplm@gmail.com>
Signed-off-by: Reinhard Speyerer <rspmn@arcor.de>
---
 Documentation/ABI/testing/sysfs-class-net-qmi | 4 ++--
 drivers/net/usb/qmi_wwan.c                    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-net-qmi b/Documentation/ABI/testing/sysfs-class-net-qmi
index 7122d6264c49..c310db4ccbc2 100644
--- a/Documentation/ABI/testing/sysfs-class-net-qmi
+++ b/Documentation/ABI/testing/sysfs-class-net-qmi
@@ -29,7 +29,7 @@ Contact:	Bjørn Mork <bjorn@mork.no>
 Description:
 		Unsigned integer.
 
-		Write a number ranging from 1 to 127 to add a qmap mux
+		Write a number ranging from 1 to 254 to add a qmap mux
 		based network device, supported by recent Qualcomm based
 		modems.
 
@@ -46,5 +46,5 @@ Contact:	Bjørn Mork <bjorn@mork.no>
 Description:
 		Unsigned integer.
 
-		Write a number ranging from 1 to 127 to delete a previously
+		Write a number ranging from 1 to 254 to delete a previously
 		created qmap mux based network device.
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index c6fbc2a2a785..780c10ee359b 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -429,8 +429,8 @@ static ssize_t add_mux_store(struct device *d,  struct device_attribute *attr, c
 	if (kstrtou8(buf, 0, &mux_id))
 		return -EINVAL;
 
-	/* mux_id [1 - 0x7f] range empirically found */
-	if (mux_id < 1 || mux_id > 0x7f)
+	/* mux_id [1 - 254] for compatibility with ip(8) and the rmnet driver */
+	if (mux_id < 1 || mux_id > 254)
 		return -EINVAL;
 
 	if (!rtnl_trylock())
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] qmi_wwan: fix QMAP handling
  2019-06-12 17:01 [PATCH 0/4] qmi_wwan: fix QMAP handling Reinhard Speyerer
                   ` (3 preceding siblings ...)
  2019-06-12 17:03 ` [PATCH 4/4] qmi_wwan: extend permitted QMAP mux_id value range Reinhard Speyerer
@ 2019-06-13 15:14 ` Daniele Palmas
  2019-06-13 17:54 ` Bjørn Mork
  5 siblings, 0 replies; 8+ messages in thread
From: Daniele Palmas @ 2019-06-13 15:14 UTC (permalink / raw)
  To: Reinhard Speyerer; +Cc: Bjørn Mork, netdev

Hi Reinhard,

Il giorno mer 12 giu 2019 alle ore 19:02 Reinhard Speyerer
<rspmn@arcor.de> ha scritto:
>
> This series addresses the following issues observed when using the
> QMAP support of the qmi_wwan driver:
>
> 1. The QMAP code in the qmi_wwan driver is based on the CodeAurora
>    GobiNet driver ([1], [2]) which does not process QMAP padding
>    in the RX path correctly. This causes qmimux_rx_fixup() to pass
>    incorrect data to the IP stack when padding is used.
>
> 2. qmimux devices currently lack proper network device usage statistics.
>
> 3. RCU stalls on device disconnect with QMAP activated like this
>
>    # echo Y > /sys/class/net/wwan0/qmi/raw_ip
>    # echo 1 > /sys/class/net/wwan0/qmi/add_mux
>    # echo 2 > /sys/class/net/wwan0/qmi/add_mux
>    # echo 3 > /sys/class/net/wwan0/qmi/add_mux
>
>    have been observed in certain setups:
>
>    [ 2273.676593] option1 ttyUSB16: GSM modem (1-port) converter now disconnected from ttyUSB16
>    [ 2273.676617] option 6-1.2:1.0: device disconnected
>    [ 2273.676774] WARNING: CPU: 1 PID: 141 at kernel/rcu/tree_plugin.h:342 rcu_note_context_switch+0x2a/0x3d0
>    [ 2273.676776] Modules linked in: option qmi_wwan cdc_mbim cdc_ncm qcserial cdc_wdm usb_wwan sierra sierra_net usbnet mii edd coretemp iptable_mangle ip6_tables iptable_filter ip_tables cdc_acm dm_mod dax iTCO_wdt evdev iTCO_vendor_support sg ftdi_sio usbserial e1000e ptp pps_core i2c_i801 ehci_pci button lpc_ich i2c_core mfd_core uhci_hcd ehci_hcd rtc_cmos usbcore usb_common sd_mod fan ata_piix thermal
>    [ 2273.676817] CPU: 1 PID: 141 Comm: kworker/1:1 Not tainted 4.19.38-rsp-1 #1
>    [ 2273.676819] Hardware name: Not Applicable   Not Applicable  /CX-GS/GM45-GL40             , BIOS V1.11      03/23/2011
>    [ 2273.676828] Workqueue: usb_hub_wq hub_event [usbcore]
>    [ 2273.676832] EIP: rcu_note_context_switch+0x2a/0x3d0
>    [ 2273.676834] Code: 55 89 e5 57 56 89 c6 53 83 ec 14 89 45 f0 e8 5d ff ff ff 89 f0 64 8b 3d 24 a6 86 c0 84 c0 8b 87 04 02 00 00 75 7a 85 c0 7e 7a <0f> 0b 80 bf 08 02 00 00 00 0f 84 87 00 00 00 e8 b2 e2 ff ff bb dc
>    [ 2273.676836] EAX: 00000001 EBX: f614bc00 ECX: 00000001 EDX: c0715b81
>    [ 2273.676838] ESI: 00000000 EDI: f18beb40 EBP: f1a3dc20 ESP: f1a3dc00
>    [ 2273.676840] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00010002
>    [ 2273.676842] CR0: 80050033 CR2: b7e97230 CR3: 2f9c4000 CR4: 000406b0
>    [ 2273.676843] Call Trace:
>    [ 2273.676847]  ? preempt_count_add+0xa5/0xc0
>    [ 2273.676852]  __schedule+0x4e/0x4f0
>    [ 2273.676855]  ? __queue_work+0xf1/0x2a0
>    [ 2273.676858]  ? _raw_spin_lock_irqsave+0x14/0x40
>    [ 2273.676860]  ? preempt_count_add+0x52/0xc0
>    [ 2273.676862]  schedule+0x33/0x80
>    [ 2273.676865]  _synchronize_rcu_expedited+0x24e/0x280
>    [ 2273.676867]  ? rcu_accelerate_cbs_unlocked+0x70/0x70
>    [ 2273.676871]  ? wait_woken+0x70/0x70
>    [ 2273.676873]  ? rcu_accelerate_cbs_unlocked+0x70/0x70
>    [ 2273.676875]  ? _synchronize_rcu_expedited+0x280/0x280
>    [ 2273.676877]  synchronize_rcu_expedited+0x22/0x30
>    [ 2273.676881]  synchronize_net+0x25/0x30
>    [ 2273.676885]  dev_deactivate_many+0x133/0x230
>    [ 2273.676887]  ? preempt_count_add+0xa5/0xc0
>    [ 2273.676890]  __dev_close_many+0x4d/0xc0
>    [ 2273.676892]  ? skb_dequeue+0x40/0x50
>    [ 2273.676895]  dev_close_many+0x5d/0xd0
>    [ 2273.676898]  rollback_registered_many+0xbf/0x4c0
>    [ 2273.676901]  ? raw_notifier_call_chain+0x1a/0x20
>    [ 2273.676904]  ? call_netdevice_notifiers_info+0x23/0x60
>    [ 2273.676906]  ? netdev_master_upper_dev_get+0xe/0x70
>    [ 2273.676908]  rollback_registered+0x1f/0x30
>    [ 2273.676911]  unregister_netdevice_queue+0x47/0xb0
>    [ 2273.676915]  qmimux_unregister_device+0x1f/0x30 [qmi_wwan]
>    [ 2273.676917]  qmi_wwan_disconnect+0x5d/0x90 [qmi_wwan]
>    ...
>    [ 2273.677001] ---[ end trace 0fcc5f88496b485a ]---
>    [ 2294.679136] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
>    [ 2294.679140] rcu:  Tasks blocked on level-0 rcu_node (CPUs 0-1): P141
>    [ 2294.679144] rcu:  (detected by 0, t=21002 jiffies, g=265857, q=8446)
>    [ 2294.679148] kworker/1:1     D    0   141      2 0x80000000
>
>
> In addition the permitted QMAP mux_id value range is extended for
> compatibility with ip(8) and the rmnet driver.
>
> Reinhard
>

I tested your patch-set with LM940 and it seems working properly.

If you think it is worth, the tag

Tested-by: Daniele Palmas <dnlplm@gmail.com>

can be added.

By the way, not related to your patches, but I've received reports of
throughput tests with LM960
(dl-max-datagrams=32,dl-datagram-max-size=16384, single PDN, UDP)
reaching around 1.15 Gbps in downlink, so the driver seems to be
working pretty well. This, however, requires usbnet rx_urb_size to be
changed (currently I'm just modifying the parent network adapter MTU
to achieve this).

Thanks,
Daniele

> [1]: https://portland.source.codeaurora.org/patches/quic/gobi
> [2]: https://portland.source.codeaurora.org/quic/qsdk/oss/lklm/gobinet/
>
> Reinhard Speyerer (4):
>   qmi_wwan: add support for QMAP padding in the RX path
>   qmi_wwan: add network device usage statistics for qmimux devices
>   qmi_wwan: avoid RCU stalls on device disconnect when in QMAP mode
>   qmi_wwan: extend permitted QMAP mux_id value range
>
>  Documentation/ABI/testing/sysfs-class-net-qmi |   4 +-
>  drivers/net/usb/qmi_wwan.c                    | 103 ++++++++++++++++++++++----
>  2 files changed, 91 insertions(+), 16 deletions(-)
>
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] qmi_wwan: fix QMAP handling
  2019-06-12 17:01 [PATCH 0/4] qmi_wwan: fix QMAP handling Reinhard Speyerer
                   ` (4 preceding siblings ...)
  2019-06-13 15:14 ` [PATCH 0/4] qmi_wwan: fix QMAP handling Daniele Palmas
@ 2019-06-13 17:54 ` Bjørn Mork
  2019-06-15  2:06   ` David Miller
  5 siblings, 1 reply; 8+ messages in thread
From: Bjørn Mork @ 2019-06-13 17:54 UTC (permalink / raw)
  To: Reinhard Speyerer; +Cc: Daniele Palmas, netdev

Reinhard Speyerer <rspmn@arcor.de> writes:

> This series addresses the following issues observed when using the
> QMAP support of the qmi_wwan driver:

Really nice work!  Thanks.

Acked-by: Bjørn Mork <bjorn@mork.no>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] qmi_wwan: fix QMAP handling
  2019-06-13 17:54 ` Bjørn Mork
@ 2019-06-15  2:06   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2019-06-15  2:06 UTC (permalink / raw)
  To: bjorn; +Cc: rspmn, dnlplm, netdev

From: Bjørn Mork <bjorn@mork.no>
Date: Thu, 13 Jun 2019 19:54:40 +0200

> Reinhard Speyerer <rspmn@arcor.de> writes:
> 
>> This series addresses the following issues observed when using the
>> QMAP support of the qmi_wwan driver:
> 
> Really nice work!  Thanks.
> 
> Acked-by: Bjørn Mork <bjorn@mork.no>

Series applied.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-06-15  2:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-12 17:01 [PATCH 0/4] qmi_wwan: fix QMAP handling Reinhard Speyerer
2019-06-12 17:02 ` [PATCH 1/4] qmi_wwan: add support for QMAP padding in the RX path Reinhard Speyerer
2019-06-12 17:02 ` [PATCH 2/4] qmi_wwan: add network device usage statistics for qmimux devices Reinhard Speyerer
2019-06-12 17:03 ` [PATCH 3/4] qmi_wwan: avoid RCU stalls on device disconnect when in QMAP mode Reinhard Speyerer
2019-06-12 17:03 ` [PATCH 4/4] qmi_wwan: extend permitted QMAP mux_id value range Reinhard Speyerer
2019-06-13 15:14 ` [PATCH 0/4] qmi_wwan: fix QMAP handling Daniele Palmas
2019-06-13 17:54 ` Bjørn Mork
2019-06-15  2:06   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).