From 146a08d2d646983ebd2e510405e7554364675e39 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Sun, 11 May 2014 10:47:12 +0200 Subject: net: cdc_mbim: optionally use VLAN ID 4094 for IP session 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cdc_mbim driver maps 802.1q VLANs to MBIM IP and DSS sessions. MBIM IP session 0 is handled as an exception and is mapped to untagged frames. This patch adds optional support for remapping MBIM IP session 0 to 802.1q VLAN ID 4094 instead. The default behaviour is not changed. The new behaviour is triggered by adding a link for this previously unsupported VLAN. The untagged mapping was chosen initially to support the assumed most common use case: Most current MBIM devices only support a single IP session (i.e. session 0 only), and using untagged frames lets the users completely ignore the additonal complexity of the multiplexing layer. But when the multiplexing features of MBIM are used, then this netdev gets a double meaning: It becomes the master interface for all the VLAN subdevs the additional sessions are mapped to, while still serving as the untagged IP interface for session 0. This can be problematic, especially when using Device Service Streams (DSS), as have become apparent recently with the availability of devices with real DSS support. Some use cases need to e.g set a MTU which is higher than allowed for IP Session 0. The dual role also leads to the situation where the IP Session 0 interface cannot be taken down without breaking unrelated IP or DSS sessions - a devastating side effect which applications managing a simple IP session cannot be expected to be aware of. A typical DHCP client will assume that it should bring the interface down after releasing the IP lease. These problems can be avoided by tagging IP session 0 packets too, making this session similar to all other multiplexed sessions. This redefines the main netdev as an upper master interface only. Cc: Greg Suarez Signed-off-by: Bjørn Mork Signed-off-by: David S. Miller --- drivers/net/usb/cdc_mbim.c | 74 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 68 insertions(+), 6 deletions(-) (limited to 'drivers/net/usb/cdc_mbim.c') diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c index 0ab79fca822c..694a8790f695 100644 --- a/drivers/net/usb/cdc_mbim.c +++ b/drivers/net/usb/cdc_mbim.c @@ -24,13 +24,21 @@ #include #include +/* alternative VLAN for IP session 0 if not untagged */ +#define MBIM_IPS0_VID 4094 + /* driver specific data - must match cdc_ncm usage */ struct cdc_mbim_state { struct cdc_ncm_ctx *ctx; atomic_t pmcount; struct usb_driver *subdriver; - struct usb_interface *control; - struct usb_interface *data; + unsigned long _unused; + unsigned long flags; +}; + +/* flags for the cdc_mbim_state.flags field */ +enum cdc_mbim_flags { + FLAG_IPS0_VLAN = 1 << 0, /* IP session 0 is tagged */ }; /* using a counter to merge subdriver requests with our own into a combined state */ @@ -62,6 +70,42 @@ static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status) return cdc_mbim_manage_power(dev, status); } +static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid) +{ + struct usbnet *dev = netdev_priv(netdev); + struct cdc_mbim_state *info = (void *)&dev->data; + + /* creation of this VLAN is a request to tag IP session 0 */ + if (vid == MBIM_IPS0_VID) + info->flags |= FLAG_IPS0_VLAN; + else + if (vid >= 512) /* we don't map these to MBIM session */ + return -EINVAL; + return 0; +} + +static int cdc_mbim_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 vid) +{ + struct usbnet *dev = netdev_priv(netdev); + struct cdc_mbim_state *info = (void *)&dev->data; + + /* this is a request for an untagged IP session 0 */ + if (vid == MBIM_IPS0_VID) + info->flags &= ~FLAG_IPS0_VLAN; + return 0; +} + +static const struct net_device_ops cdc_mbim_netdev_ops = { + .ndo_open = usbnet_open, + .ndo_stop = usbnet_stop, + .ndo_start_xmit = usbnet_start_xmit, + .ndo_tx_timeout = usbnet_tx_timeout, + .ndo_change_mtu = usbnet_change_mtu, + .ndo_set_mac_address = eth_mac_addr, + .ndo_validate_addr = eth_validate_addr, + .ndo_vlan_rx_add_vid = cdc_mbim_rx_add_vid, + .ndo_vlan_rx_kill_vid = cdc_mbim_rx_kill_vid, +}; static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf) { @@ -101,7 +145,10 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf) dev->net->flags |= IFF_NOARP; /* no need to put the VLAN tci in the packet headers */ - dev->net->features |= NETIF_F_HW_VLAN_CTAG_TX; + dev->net->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_FILTER; + + /* monitor VLAN additions and removals */ + dev->net->netdev_ops = &cdc_mbim_netdev_ops; err: return ret; } @@ -164,12 +211,24 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb skb_pull(skb, ETH_HLEN); } + /* Is IP session <0> tagged too? */ + if (info->flags & FLAG_IPS0_VLAN) { + /* drop all untagged packets */ + if (!tci) + goto error; + /* map MBIM_IPS0_VID to IPS<0> */ + if (tci == MBIM_IPS0_VID) + tci = 0; + } + /* mapping VLANs to MBIM sessions: - * no tag => IPS session <0> + * no tag => IPS session <0> if !FLAG_IPS0_VLAN * 1 - 255 => IPS session * 256 - 511 => DSS session - * 512 - 4095 => unsupported, drop + * 512 - 4093 => unsupported, drop + * 4094 => IPS session <0> if FLAG_IPS0_VLAN */ + switch (tci & 0x0f00) { case 0x0000: /* VLAN ID 0 - 255 */ if (!is_ip) @@ -268,7 +327,7 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_ __be16 proto = htons(ETH_P_802_3); struct sk_buff *skb = NULL; - if (tci < 256) { /* IPS session? */ + if (tci < 256 || tci == MBIM_IPS0_VID) { /* IPS session? */ if (len < sizeof(struct iphdr)) goto err; @@ -338,6 +397,9 @@ next_ndp: case cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN): c = (u8 *)&ndp16->dwSignature; tci = c[3]; + /* tag IPS<0> packets too if MBIM_IPS0_VID exists */ + if (!tci && info->flags & FLAG_IPS0_VLAN) + tci = MBIM_IPS0_VID; break; case cpu_to_le32(USB_CDC_MBIM_NDP16_DSS_SIGN): c = (u8 *)&ndp16->dwSignature; -- cgit v1.2.3 From 6e1b3095ddd2aa5286c8219ee19ab77c0398b43f Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Sun, 11 May 2014 10:47:13 +0200 Subject: net: cdc_mbim: reject IP packets on DSS VLANs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DSS VLANs are pseudo network interfaces representing arbitrary data streams, and specifically not IP. Preventing spurious IP packets can sometimes be a hassle. The kernel will for example send an IPv6 Router Solicit when the interface is brought up unless the user has been careful enough to disable IPv6 first. Such packets forwared to a MBIM DSS session will look like spurious noise to the device, and can cause it to log an error or even malfunction. Drop all IP packets on the designated DSS VLANs to prevent such unwanted spurious transmissions. Cc: Greg Suarez Reported-by: Arnaud Desmier Signed-off-by: Bjørn Mork Signed-off-by: David S. Miller --- drivers/net/usb/cdc_mbim.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/net/usb/cdc_mbim.c') diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c index 694a8790f695..80d27719ba38 100644 --- a/drivers/net/usb/cdc_mbim.c +++ b/drivers/net/usb/cdc_mbim.c @@ -237,6 +237,8 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb c[3] = tci; break; case 0x0100: /* VLAN ID 256 - 511 */ + if (is_ip) + goto error; sign = cpu_to_le32(USB_CDC_MBIM_NDP16_DSS_SIGN); c = (u8 *)&sign; c[3] = tci; -- cgit v1.2.3 From 50a0ffaf75e9d2d97200b523f2c600f40e9756b1 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Sun, 11 May 2014 10:47:15 +0200 Subject: net: cdc_ncm/cdc_mbim: rework probing of NCM/MBIM functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NCM class match in the cdc_mbim driver is confusing and cause unexpected behaviour. The USB core guarantees that a USB interface is in altsetting 0 when probing starts. This means that devices implementing a NCM 1.0 backwards compatible MBIM function (a "NCM/MBIM function") always hit the NCM entry in the cdc_mbim driver match table. Such functions will never match any of the MBIM entries. This causes unexpeced behaviour for cases where the NCM and MBIM entries are differet, which is currently the case for all except Ericsson devices. Improve the probing of NCM/MBIM functions by looking up the device again in the cdc_mbim match table after switching to the MBIM identity. The shared altsetting selection is updated to better accommodate the new probing logic, returning the preferred altsetting for the control interface instead of the data interface. The control interface altsetting update is moved to the cdc_mbim driver. It is never necessary to change the control interface altsetting for NCM. Cc: Greg Suarez Reported by: Yu-an Shih Signed-off-by: Bjørn Mork Signed-off-by: David S. Miller --- drivers/net/usb/cdc_mbim.c | 43 +++++++++++++++++++++++++++++++++++++++++-- drivers/net/usb/cdc_ncm.c | 27 +++++++++++++-------------- include/linux/usb/cdc_ncm.h | 2 +- 3 files changed, 55 insertions(+), 17 deletions(-) (limited to 'drivers/net/usb/cdc_mbim.c') diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c index 80d27719ba38..bc23273d0455 100644 --- a/drivers/net/usb/cdc_mbim.c +++ b/drivers/net/usb/cdc_mbim.c @@ -107,15 +107,54 @@ static const struct net_device_ops cdc_mbim_netdev_ops = { .ndo_vlan_rx_kill_vid = cdc_mbim_rx_kill_vid, }; +/* Change the control interface altsetting and update the .driver_info + * pointer if the matching entry after changing class codes points to + * a different struct + */ +static int cdc_mbim_set_ctrlalt(struct usbnet *dev, struct usb_interface *intf, u8 alt) +{ + struct usb_driver *driver = to_usb_driver(intf->dev.driver); + const struct usb_device_id *id; + struct driver_info *info; + int ret; + + ret = usb_set_interface(dev->udev, + intf->cur_altsetting->desc.bInterfaceNumber, + alt); + if (ret) + return ret; + + id = usb_match_id(intf, driver->id_table); + if (!id) + return -ENODEV; + + info = (struct driver_info *)id->driver_info; + if (info != dev->driver_info) { + dev_dbg(&intf->dev, "driver_info updated to '%s'\n", + info->description); + dev->driver_info = info; + } + return 0; +} + static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf) { struct cdc_ncm_ctx *ctx; struct usb_driver *subdriver = ERR_PTR(-ENODEV); int ret = -ENODEV; - u8 data_altsetting = cdc_ncm_select_altsetting(dev, intf); + u8 data_altsetting = 1; struct cdc_mbim_state *info = (void *)&dev->data; - /* Probably NCM, defer for cdc_ncm_bind */ + /* should we change control altsetting on a NCM/MBIM function? */ + if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) { + data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM; + ret = cdc_mbim_set_ctrlalt(dev, intf, CDC_NCM_COMM_ALTSETTING_MBIM); + if (ret) + goto err; + ret = -ENODEV; + } + + /* we will hit this for NCM/MBIM functions if prefer_mbim is false */ if (!cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting)) goto err; diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 9a2bd11943eb..d23bca57a23f 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -541,10 +541,10 @@ void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf) } EXPORT_SYMBOL_GPL(cdc_ncm_unbind); -/* Select the MBIM altsetting iff it is preferred and available, - * returning the number of the corresponding data interface altsetting +/* Return the number of the MBIM control interface altsetting iff it + * is preferred and available, */ -u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf) +u8 cdc_ncm_select_altsetting(struct usb_interface *intf) { struct usb_host_interface *alt; @@ -563,15 +563,15 @@ u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf) * the rules given in section 6 (USB Device Model) of this * specification." */ - if (prefer_mbim && intf->num_altsetting == 2) { + if (intf->num_altsetting < 2) + return intf->cur_altsetting->desc.bAlternateSetting; + + if (prefer_mbim) { alt = usb_altnum_to_altsetting(intf, CDC_NCM_COMM_ALTSETTING_MBIM); - if (alt && cdc_ncm_comm_intf_is_mbim(alt) && - !usb_set_interface(dev->udev, - intf->cur_altsetting->desc.bInterfaceNumber, - CDC_NCM_COMM_ALTSETTING_MBIM)) - return CDC_NCM_DATA_ALTSETTING_MBIM; + if (alt && cdc_ncm_comm_intf_is_mbim(alt)) + return CDC_NCM_COMM_ALTSETTING_MBIM; } - return CDC_NCM_DATA_ALTSETTING_NCM; + return CDC_NCM_COMM_ALTSETTING_NCM; } EXPORT_SYMBOL_GPL(cdc_ncm_select_altsetting); @@ -580,12 +580,11 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf) int ret; /* MBIM backwards compatible function? */ - cdc_ncm_select_altsetting(dev, intf); - if (cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting)) + if (cdc_ncm_select_altsetting(intf) != CDC_NCM_COMM_ALTSETTING_NCM) return -ENODEV; - /* NCM data altsetting is always 1 */ - ret = cdc_ncm_bind_common(dev, intf, 1); + /* The NCM data altsetting is fixed */ + ret = cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM); /* * We should get an event when network connection is "connected" or diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h index 44b38b92236a..55b6feead93b 100644 --- a/include/linux/usb/cdc_ncm.h +++ b/include/linux/usb/cdc_ncm.h @@ -121,7 +121,7 @@ struct cdc_ncm_ctx { u16 connected; }; -u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf); +u8 cdc_ncm_select_altsetting(struct usb_interface *intf); int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting); void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf); struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign); -- cgit v1.2.3