Hey,
I've encountered that same problem a few days ago, found this thread
and these (unmerged) patches, "ported" them to a more current version
of the kernel (wasn't much work, let's be honest), and I was wondering
if it would be possible to get them merged, because this is still a
problem with cdc_ncm.
Here's the three "up to date" patches (based on 5.8-rc5), they work
insofar as I'm running with them, the bug isn't there anymore (I get
ethernet multicast packets correctly), and there's no obvious bug. I'm
not a dev though, so I have no idea if these are formatted correctly,
if the patch separation is correct, or anything like that, I just think
it would be great if this could be merged into mainline!
On Sun, 2018-07-01 at 11:05 +0200, Miguel Rodríguez Pérez wrote:
> Remove some unneded varibles to make the code easier to read
> and, replace the generic usb_control_msg function for the
> more specific usbnet_write_cmd.
>
> Signed-off-by: Miguel Rodríguez Pérez <miguel@xxxxxxxxxxxxx>
> NACKED-BY: Oliver Neukum <oneukum@xxxxxxxx>
> ---
> drivers/net/usb/cdc_ether.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ether.c
> b/drivers/net/usb/cdc_ether.c
> index 178b956501a7..815ed0dc18fe 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -77,9 +77,7 @@ static const u8 mbm_guid[16] = {
>
> static void usbnet_cdc_update_filter(struct usbnet *dev)
> {
> - struct cdc_state *info = (void *) &dev->data;
> - struct usb_interface *intf = info->control;
> - struct net_device *net = dev->net;
> + struct net_device *net = dev->net;
>
> u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED
> | USB_CDC_PACKET_TYPE_BROADCAST;
> @@ -93,16 +91,13 @@ static void usbnet_cdc_update_filter(struct
> usbnet *dev)
> if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI))
> cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST;
>
> - usb_control_msg(dev->udev,
> - usb_sndctrlpipe(dev->udev, 0),
> + usbnet_write_cmd(dev,
> USB_CDC_SET_ETHERNET_PACKET_FILTER,
> - USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> + USB_TYPE_CLASS | USB_DIR_OUT |
> USB_RECIP_INTERFACE,
> cdc_filter,
> - intf->cur_altsetting->desc.bInterfaceNumber,
> + dev->intf->cur_altsetting-
> >desc.bInterfaceNumber,
> NULL,
> - 0,
> - USB_CTRL_SET_TIMEOUT
> - );
> + 0);
> }
>
> /* probes control interface, claims data interface, collects the
> bulk
--
Wxcafé <wxcafe@xxxxxxxxxx>
From fb4c24637de82a81374622d3f2f96452e0fb07d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wxcaf=C3=A9?= <wxcafe@xxxxxxxxxx>
Date: Mon, 13 Jul 2020 16:16:14 -0400
Subject: [PATCH 1/3] net: cdc_ether: export generic usbnet_cdc_update_filter
This makes the function avaiable to other drivers, like cdn_ncm.
Other drivers will use different dev->data types, so the exported
function must not use it; instead the exported function takes an
additional pointer to the control interface.
Within cdc_ether the control interface is still taken from the control
field from struct cdc_state stored in dev->data.
---
drivers/net/usb/cdc_ether.c | 24 ++++++++++++++++--------
include/linux/usb/usbnet.h | 2 ++
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index a657943c9f01..b201adc885f6 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -63,10 +63,8 @@ static const u8 mbm_guid[16] = {
0xa6, 0x07, 0xc0, 0xff, 0xcb, 0x7e, 0x39, 0x2a,
};
-static void usbnet_cdc_update_filter(struct usbnet *dev)
+void usbnet_cdc_update_filter(struct usbnet *dev, struct usb_interface *control)
{
- struct cdc_state *info = (void *) &dev->data;
- struct usb_interface *intf = info->control;
struct net_device *net = dev->net;
u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED
@@ -86,12 +84,22 @@ static void usbnet_cdc_update_filter(struct usbnet *dev)
USB_CDC_SET_ETHERNET_PACKET_FILTER,
USB_TYPE_CLASS | USB_RECIP_INTERFACE,
cdc_filter,
- intf->cur_altsetting->desc.bInterfaceNumber,
+ control->cur_altsetting->desc.bInterfaceNumber,
NULL,
0,
USB_CTRL_SET_TIMEOUT
);
}
+EXPORT_SYMBOL_GPL(usbnet_cdc_update_filter);
+
+/* the control interface is not always necessarily the probed interface
+ * dev->intf, see rndis handling in usbnet_generic_cdc_bind.
+ */
+static void usbnet_cdc_ether_update_filter(struct usbnet *dev) {
+ struct cdc_state *info = (void *)&dev->data;
+
+ usbnet_cdc_update_filter(dev, info->control);
+}
/* probes control interface, claims data interface, collects the bulk
* endpoints, activates data interface (if needed), maybe sets MTU.
@@ -336,7 +344,7 @@ int usbnet_ether_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
* don't do reset all the way. So the packet filter should
* be set to a sane initial value.
*/
- usbnet_cdc_update_filter(dev);
+ usbnet_cdc_ether_update_filter(dev);
bail_out:
return rv;
@@ -514,7 +522,7 @@ static const struct driver_info cdc_info = {
.bind = usbnet_cdc_bind,
.unbind = usbnet_cdc_unbind,
.status = usbnet_cdc_status,
- .set_rx_mode = usbnet_cdc_update_filter,
+ .set_rx_mode = usbnet_cdc_ether_update_filter,
.manage_power = usbnet_manage_power,
};
@@ -524,7 +532,7 @@ static const struct driver_info zte_cdc_info = {
.bind = usbnet_cdc_zte_bind,
.unbind = usbnet_cdc_unbind,
.status = usbnet_cdc_zte_status,
- .set_rx_mode = usbnet_cdc_update_filter,
+ .set_rx_mode = usbnet_cdc_ether_update_filter,
.manage_power = usbnet_manage_power,
.rx_fixup = usbnet_cdc_zte_rx_fixup,
};
@@ -535,7 +543,7 @@ static const struct driver_info wwan_info = {
.bind = usbnet_cdc_bind,
.unbind = usbnet_cdc_unbind,
.status = usbnet_cdc_status,
- .set_rx_mode = usbnet_cdc_update_filter,
+ .set_rx_mode = usbnet_cdc_ether_update_filter,
.manage_power = usbnet_manage_power,
};
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index b0bff3083278..387f3da06e9d 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -286,4 +286,6 @@ extern void usbnet_update_max_qlen(struct usbnet *dev);
extern void usbnet_get_stats64(struct net_device *dev,
struct rtnl_link_stats64 *stats);
+extern void usbnet_cdc_update_filter(struct usbnet *, struct usb_interface *);
+
#endif /* __LINUX_USB_USBNET_H */
--
2.27.0
From 315f7e56a4a49e9cbafbd9a88444f8b213524d4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wxcaf=C3=A9?= <wxcafe@xxxxxxxxxx>
Date: Mon, 13 Jul 2020 16:17:21 -0400
Subject: [PATCH 2/3] net: usbnet: export usbnet_set_rx_mode
---
drivers/net/usb/usbnet.c | 3 ++-
include/linux/usb/usbnet.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 5ec97def3513..e45935a5856a 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1108,12 +1108,13 @@ static void __handle_link_change(struct usbnet *dev)
clear_bit(EVENT_LINK_CHANGE, &dev->flags);
}
-static void usbnet_set_rx_mode(struct net_device *net)
+void usbnet_set_rx_mode(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
usbnet_defer_kevent(dev, EVENT_SET_RX_MODE);
}
+EXPORT_SYMBOL_GPL(usbnet_set_rx_mode);
static void __handle_set_rx_mode(struct usbnet *dev)
{
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 387f3da06e9d..455b568ff4dd 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -254,6 +254,7 @@ extern int usbnet_stop(struct net_device *net);
extern netdev_tx_t usbnet_start_xmit(struct sk_buff *skb,
struct net_device *net);
extern void usbnet_tx_timeout(struct net_device *net, unsigned int txqueue);
+extern void usbnet_set_rx_mode(struct net_device *net);
extern int usbnet_change_mtu(struct net_device *net, int new_mtu);
extern int usbnet_get_endpoints(struct usbnet *, struct usb_interface *);
--
2.27.0
From b716a0fa777a9d7dee854b768efc3fff9074a2b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wxcaf=C3=A9?= <wxcafe@xxxxxxxxxx>
Date: Mon, 13 Jul 2020 16:20:41 -0400
Subject: [PATCH 3/3] net: cdc_ncm: hook into set_rx_mode to admit multicast
Use usbnet_cdc_update_filter from cdc_ether to admit all multicast
traffic if there is more than one multicast filter configured.
---
drivers/net/usb/cdc_ncm.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 8929669b5e6d..688d7e9df41e 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -792,6 +792,7 @@ static const struct net_device_ops cdc_ncm_netdev_ops = {
.ndo_stop = usbnet_stop,
.ndo_start_xmit = usbnet_start_xmit,
.ndo_tx_timeout = usbnet_tx_timeout,
+ .ndo_set_rx_mode = usbnet_set_rx_mode,
.ndo_get_stats64 = usbnet_get_stats64,
.ndo_change_mtu = cdc_ncm_change_mtu,
.ndo_set_mac_address = eth_mac_addr,
@@ -1885,6 +1886,13 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
}
}
+/* the control interface is always the probed one */
+static void usbnet_cdc_ncm_update_filter(struct usbnet *dev)
+{
+ usbnet_cdc_update_filter(dev, dev->intf);
+}
+
+
static const struct driver_info cdc_ncm_info = {
.description = "CDC NCM",
.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
@@ -1895,6 +1903,7 @@ static const struct driver_info cdc_ncm_info = {
.status = cdc_ncm_status,
.rx_fixup = cdc_ncm_rx_fixup,
.tx_fixup = cdc_ncm_tx_fixup,
+ .set_rx_mode = usbnet_cdc_ncm_update_filter,
};
/* Same as cdc_ncm_info, but with FLAG_WWAN */
@@ -1908,6 +1917,7 @@ static const struct driver_info wwan_info = {
.status = cdc_ncm_status,
.rx_fixup = cdc_ncm_rx_fixup,
.tx_fixup = cdc_ncm_tx_fixup,
+ .set_rx_mode = usbnet_cdc_ncm_update_filter,
};
/* Same as wwan_info, but with FLAG_NOARP */
@@ -1921,6 +1931,7 @@ static const struct driver_info wwan_noarp_info = {
.status = cdc_ncm_status,
.rx_fixup = cdc_ncm_rx_fixup,
.tx_fixup = cdc_ncm_tx_fixup,
+ .set_rx_mode = usbnet_cdc_ncm_update_filter,
};
static const struct usb_device_id cdc_devs[] = {
--
2.27.0
