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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help