Skip to content

L
linux
Commit 2309f517 authored by David S. Miller's avatar David S. Miller
Browse files
Options

Merge branch 'qmi_wwan-fix-QMAP-handling' 

Reinhard Speyerer says:

====================
qmi_wwan: fix QMAP handling

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/
====================
Tested-by: default avatarDaniele Palmas <dnlplm@gmail.com>
Acked-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 2a2af5e6 36815b41
Hide whitespace changes
Inline Side-by-side
Showing with 91 additions and 16 deletions
Documentation/ABI/testing/sysfs-class-net-qmi
View file @ 2309f517
......@@ -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.
drivers/net/usb/qmi_wwan.c
View file @ 2309f517
......@@ -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)
......@@ -153,7 +201,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 +219,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,9 +245,20 @@ 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);
if (netif_rx(skbn) != NET_RX_SUCCESS)
skb_put_data(skbn, skb->data + offset + qmimux_hdr_sz, pkt_len);
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;
......@@ -217,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;
......@@ -241,13 +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);
......@@ -356,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())
......@@ -418,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;
......@@ -1428,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)
......@@ -1440,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;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7