* [PATCH] cdc-ether: clean packet filter upon probe
@ 2014-07-28 8:56 Oliver Neukum
2014-07-29 19:22 ` David Miller
2014-08-14 11:11 ` Bjørn Mork
0 siblings, 2 replies; 11+ messages in thread
From: Oliver Neukum @ 2014-07-28 8:56 UTC (permalink / raw)
To: davem, netdev; +Cc: Oliver Neukum
There are devices that don't do reset all the way. So the packet filter should
be set to a sane initial value. Failure to do so leads to intermittent failures
of DHCP on some systems under some conditions.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
drivers/net/usb/cdc_ether.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 9ea4bfe..2a32d91 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -341,6 +341,22 @@ next_desc:
usb_driver_release_interface(driver, info->data);
return -ENODEV;
}
+
+ /* Some devices don't initialise properly. In particular
+ * the packet filter is not reset. There are devices that
+ * don't do reset all the way. So the packet filter should
+ * be set to a sane initial value.
+ */
+ usb_control_msg(dev->udev,
+ usb_sndctrlpipe(dev->udev, 0),
+ USB_CDC_SET_ETHERNET_PACKET_FILTER,
+ USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ USB_CDC_PACKET_TYPE_ALL_MULTICAST | USB_CDC_PACKET_TYPE_DIRECTED | USB_CDC_PACKET_TYPE_BROADCAST,
+ intf->cur_altsetting->desc.bInterfaceNumber,
+ NULL,
+ 0,
+ USB_CTRL_SET_TIMEOUT
+ );
return 0;
bad_desc:
--
1.8.4.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-07-28 8:56 [PATCH] cdc-ether: clean packet filter upon probe Oliver Neukum
@ 2014-07-29 19:22 ` David Miller
2014-08-14 11:11 ` Bjørn Mork
1 sibling, 0 replies; 11+ messages in thread
From: David Miller @ 2014-07-29 19:22 UTC (permalink / raw)
To: oneukum; +Cc: netdev
From: Oliver Neukum <oneukum@suse.de>
Date: Mon, 28 Jul 2014 10:56:36 +0200
> There are devices that don't do reset all the way. So the packet filter should
> be set to a sane initial value. Failure to do so leads to intermittent failures
> of DHCP on some systems under some conditions.
>
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
Applied.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-07-28 8:56 [PATCH] cdc-ether: clean packet filter upon probe Oliver Neukum
2014-07-29 19:22 ` David Miller
@ 2014-08-14 11:11 ` Bjørn Mork
2014-08-15 6:12 ` Oliver Neukum
1 sibling, 1 reply; 11+ messages in thread
From: Bjørn Mork @ 2014-08-14 11:11 UTC (permalink / raw)
To: Oliver Neukum; +Cc: davem, netdev
Oliver Neukum <oneukum@suse.de> writes:
> There are devices that don't do reset all the way. So the packet filter should
> be set to a sane initial value. Failure to do so leads to intermittent failures
> of DHCP on some systems under some conditions.
>
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
> drivers/net/usb/cdc_ether.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> index 9ea4bfe..2a32d91 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -341,6 +341,22 @@ next_desc:
> usb_driver_release_interface(driver, info->data);
> return -ENODEV;
> }
> +
> + /* Some devices don't initialise properly. In particular
> + * the packet filter is not reset. There are devices that
> + * don't do reset all the way. So the packet filter should
> + * be set to a sane initial value.
> + */
> + usb_control_msg(dev->udev,
> + usb_sndctrlpipe(dev->udev, 0),
> + USB_CDC_SET_ETHERNET_PACKET_FILTER,
> + USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> + USB_CDC_PACKET_TYPE_ALL_MULTICAST | USB_CDC_PACKET_TYPE_DIRECTED | USB_CDC_PACKET_TYPE_BROADCAST,
> + intf->cur_altsetting->desc.bInterfaceNumber,
> + NULL,
> + 0,
> + USB_CTRL_SET_TIMEOUT
> + );
> return 0;
>
> bad_desc:
Hmm, the usbnet_generic_cdc_bind() function is used by the vendor
specific zaurus and rndis_host drivers as well. Are we sure the
USB_CDC_SET_ETHERNET_PACKET_FILTER message is supported by all devices
supported by these drivers?
Bjørn
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-08-14 11:11 ` Bjørn Mork
@ 2014-08-15 6:12 ` Oliver Neukum
2014-08-15 7:13 ` Bjørn Mork
0 siblings, 1 reply; 11+ messages in thread
From: Oliver Neukum @ 2014-08-15 6:12 UTC (permalink / raw)
To: Bjørn Mork; +Cc: davem, netdev
On Thu, 2014-08-14 at 13:11 +0200, Bjørn Mork wrote:
> Oliver Neukum <oneukum@suse.de> writes:
>
> > There are devices that don't do reset all the way. So the packet filter should
> > be set to a sane initial value. Failure to do so leads to intermittent failures
> > of DHCP on some systems under some conditions.
> >
> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > ---
> > drivers/net/usb/cdc_ether.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> > index 9ea4bfe..2a32d91 100644
> > --- a/drivers/net/usb/cdc_ether.c
> > +++ b/drivers/net/usb/cdc_ether.c
> > @@ -341,6 +341,22 @@ next_desc:
> > usb_driver_release_interface(driver, info->data);
> > return -ENODEV;
> > }
> > +
> > + /* Some devices don't initialise properly. In particular
> > + * the packet filter is not reset. There are devices that
> > + * don't do reset all the way. So the packet filter should
> > + * be set to a sane initial value.
> > + */
> > + usb_control_msg(dev->udev,
> > + usb_sndctrlpipe(dev->udev, 0),
> > + USB_CDC_SET_ETHERNET_PACKET_FILTER,
> > + USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> > + USB_CDC_PACKET_TYPE_ALL_MULTICAST | USB_CDC_PACKET_TYPE_DIRECTED | USB_CDC_PACKET_TYPE_BROADCAST,
> > + intf->cur_altsetting->desc.bInterfaceNumber,
> > + NULL,
> > + 0,
> > + USB_CTRL_SET_TIMEOUT
> > + );
> > return 0;
> >
> > bad_desc:
>
> Hmm, the usbnet_generic_cdc_bind() function is used by the vendor
> specific zaurus and rndis_host drivers as well. Are we sure the
> USB_CDC_SET_ETHERNET_PACKET_FILTER message is supported by all devices
> supported by these drivers?
Support for this request is mandatory.
Regards
Oliver
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-08-15 6:12 ` Oliver Neukum
@ 2014-08-15 7:13 ` Bjørn Mork
2014-08-25 8:07 ` Oliver Neukum
0 siblings, 1 reply; 11+ messages in thread
From: Bjørn Mork @ 2014-08-15 7:13 UTC (permalink / raw)
To: Oliver Neukum; +Cc: davem, netdev
Oliver Neukum <oneukum@suse.de> writes:
> On Thu, 2014-08-14 at 13:11 +0200, Bjørn Mork wrote:
>> Oliver Neukum <oneukum@suse.de> writes:
>>
>> > There are devices that don't do reset all the way. So the packet filter should
>> > be set to a sane initial value. Failure to do so leads to intermittent failures
>> > of DHCP on some systems under some conditions.
>> >
>> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
>> > ---
>> > drivers/net/usb/cdc_ether.c | 16 ++++++++++++++++
>> > 1 file changed, 16 insertions(+)
>> >
>> > diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
>> > index 9ea4bfe..2a32d91 100644
>> > --- a/drivers/net/usb/cdc_ether.c
>> > +++ b/drivers/net/usb/cdc_ether.c
>> > @@ -341,6 +341,22 @@ next_desc:
>> > usb_driver_release_interface(driver, info->data);
>> > return -ENODEV;
>> > }
>> > +
>> > + /* Some devices don't initialise properly. In particular
>> > + * the packet filter is not reset. There are devices that
>> > + * don't do reset all the way. So the packet filter should
>> > + * be set to a sane initial value.
>> > + */
>> > + usb_control_msg(dev->udev,
>> > + usb_sndctrlpipe(dev->udev, 0),
>> > + USB_CDC_SET_ETHERNET_PACKET_FILTER,
>> > + USB_TYPE_CLASS | USB_RECIP_INTERFACE,
>> > + USB_CDC_PACKET_TYPE_ALL_MULTICAST | USB_CDC_PACKET_TYPE_DIRECTED | USB_CDC_PACKET_TYPE_BROADCAST,
>> > + intf->cur_altsetting->desc.bInterfaceNumber,
>> > + NULL,
>> > + 0,
>> > + USB_CTRL_SET_TIMEOUT
>> > + );
>> > return 0;
>> >
>> > bad_desc:
>>
>> Hmm, the usbnet_generic_cdc_bind() function is used by the vendor
>> specific zaurus and rndis_host drivers as well. Are we sure the
>> USB_CDC_SET_ETHERNET_PACKET_FILTER message is supported by all devices
>> supported by these drivers?
>
> Support for this request is mandatory.
Yes, there is no problem for standard conforming ECM devices. I fully
agree there.
But standard conformance is unfortunately not something we can expect.
My question was regarding the non-conforming devices, like the ones
supported by th zaurus and rndis_host drivers. These drivers are made
solely because the supported devices do *not* conform to the ECM spec.
If you know they support the request, then fine. But I do not think we
can assume that they do without testing it, despite what the ECM spec
says.
Bjørn
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-08-15 7:13 ` Bjørn Mork
@ 2014-08-25 8:07 ` Oliver Neukum
2014-08-25 8:39 ` Bjørn Mork
0 siblings, 1 reply; 11+ messages in thread
From: Oliver Neukum @ 2014-08-25 8:07 UTC (permalink / raw)
To: Bjørn Mork; +Cc: davem, netdev
On Fri, 2014-08-15 at 09:13 +0200, Bjørn Mork wrote:
> Oliver Neukum <oneukum@suse.de> writes:
>
> > On Thu, 2014-08-14 at 13:11 +0200, Bjørn Mork wrote:
> > Support for this request is mandatory.
>
> Yes, there is no problem for standard conforming ECM devices. I fully
> agree there.
>
> But standard conformance is unfortunately not something we can expect.
> My question was regarding the non-conforming devices, like the ones
> supported by th zaurus and rndis_host drivers. These drivers are made
You have a point. Yet another flag? Do you see an alternative?
Regards
Oliver
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-08-25 8:07 ` Oliver Neukum
@ 2014-08-25 8:39 ` Bjørn Mork
2014-08-25 12:13 ` Oliver Neukum
0 siblings, 1 reply; 11+ messages in thread
From: Bjørn Mork @ 2014-08-25 8:39 UTC (permalink / raw)
To: Oliver Neukum; +Cc: davem, netdev
Oliver Neukum <oneukum@suse.de> writes:
> On Fri, 2014-08-15 at 09:13 +0200, Bjørn Mork wrote:
>> Oliver Neukum <oneukum@suse.de> writes:
>>
>> > On Thu, 2014-08-14 at 13:11 +0200, Bjørn Mork wrote:
>
>> > Support for this request is mandatory.
>>
>> Yes, there is no problem for standard conforming ECM devices. I fully
>> agree there.
>>
>> But standard conformance is unfortunately not something we can expect.
>> My question was regarding the non-conforming devices, like the ones
>> supported by th zaurus and rndis_host drivers. These drivers are made
>
> You have a point. Yet another flag? Do you see an alternative?
Note that I don't know whether this is a real problem or not. I don't
believe I have any zaurus or rndis_host devices.
But how about just splitting the .bind functions for true ECM devices
and the others, letting the zaurus and rndis_host driver continue to use
usbnet_cdc_bind with no filter setting? Maybe define a new unexported
.bind in cdc_ether for this purpose, moving the ECM specific
initialization there?
Maybe something along this (instead of the change to
usbnet_generic_cdc_bind)?:
static int usbnet_cdc_ecm_bind(struct usbnet *dev, struct usb_interface *intf)
{
int rv = usbnet_cdc_bind(dev, intf);
if (rv < 0)
return rv;
/* Some devices don't initialise properly. In particular
* the packet filter is not reset. There are devices that
* don't do reset all the way. So the packet filter should
* be set to a sane initial value.
*/
rv = usb_control_msg(dev->udev,
usb_sndctrlpipe(dev->udev, 0),
USB_CDC_SET_ETHERNET_PACKET_FILTER,
USB_TYPE_CLASS | USB_RECIP_INTERFACE,
USB_CDC_PACKET_TYPE_ALL_MULTICAST | USB_CDC_PACKET_TYPE_DIRECTED | USB_CDC_PACKET_TYPE_BROADCAST,
intf->cur_altsetting->desc.bInterfaceNumber,
NULL,
0,
USB_CTRL_SET_TIMEOUT);
if (rv < 0)
return rv;
return 0;
}
..
static const struct driver_info cdc_info = {
.description = "CDC Ethernet Device",
.flags = FLAG_ETHER | FLAG_POINTTOPOINT,
.bind = usbnet_cdc_ecm_bind,
.unbind = usbnet_cdc_unbind,
.status = usbnet_cdc_status,
.manage_power = usbnet_manage_power,
};
Bjørn
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-08-25 8:39 ` Bjørn Mork
@ 2014-08-25 12:13 ` Oliver Neukum
0 siblings, 0 replies; 11+ messages in thread
From: Oliver Neukum @ 2014-08-25 12:13 UTC (permalink / raw)
To: Bjørn Mork; +Cc: davem, netdev
On Mon, 2014-08-25 at 10:39 +0200, Bjørn Mork wrote:
> Oliver Neukum <oneukum@suse.de> writes:
> > On Fri, 2014-08-15 at 09:13 +0200, Bjørn Mork wrote:
> >> Oliver Neukum <oneukum@suse.de> writes:
> >>
> >> > On Thu, 2014-08-14 at 13:11 +0200, Bjørn Mork wrote:
> >
> >> > Support for this request is mandatory.
> >>
> >> Yes, there is no problem for standard conforming ECM devices. I fully
> >> agree there.
> >>
> >> But standard conformance is unfortunately not something we can expect.
> >> My question was regarding the non-conforming devices, like the ones
> >> supported by th zaurus and rndis_host drivers. These drivers are made
> >
> > You have a point. Yet another flag? Do you see an alternative?
>
> Note that I don't know whether this is a real problem or not. I don't
> believe I have any zaurus or rndis_host devices.
>
> But how about just splitting the .bind functions for true ECM devices
> and the others, letting the zaurus and rndis_host driver continue to use
> usbnet_cdc_bind with no filter setting? Maybe define a new unexported
> .bind in cdc_ether for this purpose, moving the ECM specific
> initialization there?
>
> Maybe something along this (instead of the change to
> usbnet_generic_cdc_bind)?:
Yes, that looks superior.
Regards
Oliver
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-07-28 5:30 ` David Miller
@ 2014-07-28 9:22 ` Oliver Neukum
0 siblings, 0 replies; 11+ messages in thread
From: Oliver Neukum @ 2014-07-28 9:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Sun, 2014-07-27 at 22:30 -0700, David Miller wrote:
> From: Oliver Neukum <oneukum@suse.de>
> Date: Thu, 24 Jul 2014 13:56:29 +0200
>
> > + 1000 /* out of thin air */
>
> Just use USB_CTRL_SET_TIMEOUT if you can't come up with a specific
> reason to use another value.
>
> Thanks.
Certainly
Sorry
Oliver
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cdc-ether: clean packet filter upon probe
2014-07-24 11:56 Oliver Neukum
@ 2014-07-28 5:30 ` David Miller
2014-07-28 9:22 ` Oliver Neukum
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2014-07-28 5:30 UTC (permalink / raw)
To: oneukum; +Cc: netdev
From: Oliver Neukum <oneukum@suse.de>
Date: Thu, 24 Jul 2014 13:56:29 +0200
> + 1000 /* out of thin air */
Just use USB_CTRL_SET_TIMEOUT if you can't come up with a specific
reason to use another value.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] cdc-ether: clean packet filter upon probe
@ 2014-07-24 11:56 Oliver Neukum
2014-07-28 5:30 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Oliver Neukum @ 2014-07-24 11:56 UTC (permalink / raw)
To: davem, netdev; +Cc: Oliver Neukum
There are devices that don't do reset all the way. So the packet filter should
be set to a sane initial value. Failure to do so leads to intermittent failures
of DHCP on some systems under some conditions.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
drivers/net/usb/cdc_ether.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 9ea4bfe..ef8f8f8 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -341,6 +341,22 @@ next_desc:
usb_driver_release_interface(driver, info->data);
return -ENODEV;
}
+
+ /* Some devices don't initialise properly. In particular
+ * the packet filter is not reset. There are devices that
+ * don't do reset all the way. So the packet filter should
+ * be set to a sane initial value.
+ */
+ usb_control_msg(dev->udev,
+ usb_sndctrlpipe(dev->udev, 0),
+ USB_CDC_SET_ETHERNET_PACKET_FILTER,
+ USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ USB_CDC_PACKET_TYPE_ALL_MULTICAST | USB_CDC_PACKET_TYPE_DIRECTED | USB_CDC_PACKET_TYPE_BROADCAST,
+ intf->cur_altsetting->desc.bInterfaceNumber,
+ NULL,
+ 0,
+ 1000 /* out of thin air */
+ );
return 0;
bad_desc:
--
1.8.4.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-08-25 12:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-28 8:56 [PATCH] cdc-ether: clean packet filter upon probe Oliver Neukum
2014-07-29 19:22 ` David Miller
2014-08-14 11:11 ` Bjørn Mork
2014-08-15 6:12 ` Oliver Neukum
2014-08-15 7:13 ` Bjørn Mork
2014-08-25 8:07 ` Oliver Neukum
2014-08-25 8:39 ` Bjørn Mork
2014-08-25 12:13 ` Oliver Neukum
-- strict thread matches above, loose matches on Subject: below --
2014-07-24 11:56 Oliver Neukum
2014-07-28 5:30 ` David Miller
2014-07-28 9:22 ` Oliver Neukum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.