Re: [PATCH 2/2] cdc_ncm: Admit multicast traffic
From: Bjørn Mork <bjorn@mork.no>
Date: 2018-06-30 12:17:31
Also in:
linux-usb
Miguel Rodríguez Pérez [off-list ref] writes:
+static void cdc_ncm_update_filter(struct usbnet *dev)
+{
+ struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
+ struct net_device *net = dev->net;
Why not change usbnet_cdc_update_filter to get the interface number from
dev->intf instead? Then you can export it and reuse it here instead of
creating a copy. And it will also work for any other usbnet minidriver.
I.e. change this:
static void usbnet_cdc_update_filter(struct usbnet *dev)
{
struct cdc_state *info = (void *) &dev->data;
struct usb_interface *intf = info->control;
into:
static void usbnet_cdc_update_filter(struct usbnet *dev)
{
struct usb_interface *intf = dev->intf;
or simply use dev->intf to deref directly into an interface number, like
you do in your version. The local 'intf' variable doesn't do much
good here.
+ + u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED + | USB_CDC_PACKET_TYPE_BROADCAST; + + /* filtering on the device is an optional feature and not worth + * the hassle so we just roughly care about snooping and if any + * multicast is requested, we take every multicast + */ + if (net->flags & IFF_PROMISC) + cdc_filter |= USB_CDC_PACKET_TYPE_PROMISCUOUS; + if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI)) + cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST; + + usbnet_write_cmd(dev,
This is a nice change. It should be probably done in usbnet_cdc_update_filter in any case. Unless there is some reason it doesn't alreay use usbnet_write_cmd?
quoted hunk
+ USB_CDC_SET_ETHERNET_PACKET_FILTER, + USB_TYPE_CLASS | USB_DIR_OUT | USB_RECIP_INTERFACE, + cdc_filter, + iface_no, + NULL, + 0); +} + static const struct ethtool_ops cdc_ncm_ethtool_ops = { .get_link = usbnet_get_link, .nway_reset = usbnet_nway_reset,@@ -1652,6 +1679,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 = cdc_ncm_update_filter, }; /* Same as cdc_ncm_info, but with FLAG_WWAN */
As the comment indicates: There are more 'struct driver_info' variants here. Please add .set_rx_mode to all of them, unless you have a reason not to? Bjørn