2006-01-12 16:29:54

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 00/13] USBATM: summary

Hi Greg, here are some fixes and improvements to the USB ATM
modem drivers, in thirteen patches:

01: trivial modifications (formatting, changes to variable names,
comments, log level changes, printk rate limiting).

02: have minidrivers tell the core about special requirements
using a flags field.

03: remove the unused .owner field in struct usbatm_driver.

04: use kzalloc rather than kmalloc + memset.

05: make xusbatm useable.

06: sternly tell open connections that the game is up when the
modem is disconnected.

07: return the correct error code when out of memory.

08: use dev_kfree_skb_any rather than dev_kfree_skb.

09: specify buffer sizes in bytes, rather than in ATM cells.

10: add mechanism for turning on isochronous urb support.

11: remove the assumption that incoming urbs contain
complete ATM cells.

12: bump some version numbers.

13: EILSEQ hack needed by the ueagle.

All the best,

Duncan.


2006-01-12 16:43:31

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 01/13] USBATM: trivial modifications

Formatting, changes to variable names, comments, log level changes,
printk rate limiting.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (138.00 B)
trivial (29.47 kB)
Download all attachments

2006-01-12 17:40:30

by Duncan Sands

[permalink] [raw]
Subject: Re: [PATCH 01/13] USBATM: trivial modifications

Sorry, it wasn't a -p1 patch (I should really automate this).

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (110.00 B)
trivial (29.67 kB)
Download all attachments

2006-01-12 17:48:47

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 02/13] USBATM: add flags field

Have minidrivers and the core signal special requirements
using a flags field in struct usbatm_data. For the moment
this is only used to replace the need_heavy_init bind
parameter, but there'll be new flags in later patches.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (274.00 B)
flags (5.74 kB)
Download all attachments

2006-01-12 18:30:17

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 00/13] USBATM: summary

On Thu, Jan 12, 2006 at 05:29:51PM +0100, Duncan Sands wrote:
> Hi Greg, here are some fixes and improvements to the USB ATM
> modem drivers, in thirteen patches:

I only got 2 of these, is my mail just being slow (which it does at odd
times), or did you stop sending them based on some problems on your end?

thanks,

greg k-h

2006-01-12 19:15:43

by Duncan Sands

[permalink] [raw]
Subject: Re: [PATCH 00/13] USBATM: summary

> I only got 2 of these, is my mail just being slow (which it does at odd
> times), or did you stop sending them based on some problems on your end?

I stopped sending them based on problems on my end. I'll send the rest
tomorrow.

Ciao,

Duncan.

2006-01-13 08:36:21

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 03/13] USBATM: remove .owner

Remove the unused .owner field in struct usbatm_driver.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (104.00 B)
03-owner (2.63 kB)
Download all attachments

2006-01-13 08:38:21

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 04/13] USBATM: kzalloc conversion

Convert kmalloc + memset to kzalloc.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (85.00 B)
04-kzalloc (2.51 kB)
Download all attachments

2006-01-13 08:48:36

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 05/13] USBATM: xusbatm rewrite

The xusbatm driver is for otherwise unsupported modems.
All it does is grab hold of a user-specified set of
interfaces - the generic usbatm core methods (hopefully)
do the rest. As Aurelio Arroyo discovered when he tried
to use xusbatm (big mistake!), the interface grabbing logic
was completely borked. Here is a rewrite that works.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (384.00 B)
05-xusbatm (6.49 kB)
Download all attachments

2006-01-13 09:05:16

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 06/13] USBATM: shutdown open connections when disconnected

This patch causes vcc_release_async to be applied to any open
vcc's when the modem is disconnected. This signals a socket
shutdown, letting the socket user know that the game is up.
I wrote this patch because of reports that pppd would keep
connections open forever when the modem is disconnected.
This patch does not fix that problem, but it's a step in the
right direction. It doesn't help because the pppoatm module
doesn't yet monitor state changes on the ATM socket, so simply
never realises that the ATM connection has gone down (meaning
it doesn't tell the ppp layer). But at least there is a socket
state change now. Unfortunately this patch may create problems
for those rare users like me who use routed IP or some other
non-ppp connection method that goes via the ATM ARP daemon: the
daemon is buggy, and with this patch will crash when the modem
is disconnected. Users with a buggy atmarpd can simply restart
it after disconnecting the modem.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (0.98 kB)
06-disconnect (4.24 kB)
Download all attachments

2006-01-13 09:13:19

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 08/13] USBATM: use dev_kfree_skb_any rather than dev_kfree_skb

In one spot (usbatm_cancel_send) we were calling dev_kfree_skb with irqs
disabled. This mistake is just too easy to make, so systematically use
dev_kfree_skb_any rather than dev_kfree_skb.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (238.00 B)
08-dev_kfree_skb_any (873.00 B)
Download all attachments

2006-01-13 09:52:38

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 09/13] USBATM: measure buffer size in bytes; force valid sizes

Change the module parameters rcv_buf_size and snd_buf_size to
specify buffer sizes in bytes rather than ATM cells. Since
there is some danger that users may not notice this change,
the parameters are renamed to rcv_buf_bytes etc. The transmit
buffer needs to be a multiple of the ATM cell size in length,
while the receive buffer should be a multiple of the endpoint
maxpacket size (this wasn't enforced before, which causes trouble
with isochronous transfers), so enforce these restrictions. Now
that the usbatm probe method inspects the endpoint maxpacket size,
minidriver bind routines need to set the correct alternate setting
for the interface in their bind routine. This is the reason for
the speedtch changes.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (769.00 B)
09-bytes (8.98 kB)
Download all attachments

2006-01-13 09:59:24

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 10/13] USBATM: allow isochronous transfer

While the usbatm core has had some support for using isoc urbs
for some time, there was no way for users to turn it on. While
use of isoc transfer should still be considered experimental, it
now works well enough to let users turn it on. Minidrivers signal
to the core that they want to use isoc transfer by setting the new
UDSL_USE_ISOC flag. The speedtch minidriver gets a new module
parameter enable_isoc (defaults to false), plus some logic that
checks for the existence of an isoc receive endpoint (not all
speedtouch modems have one).

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (592.00 B)
10-isoc (11.67 kB)
Download all attachments

2006-01-13 10:06:46

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 11/13] USBATM: handle urbs containing partial cells

The receive logic has always assumed that urbs contain an integral
number of ATM cells, which is a bit naughty, though it never caused
any problems with bulk transfers. Isochronous urbs spank us soundly
for this. Fixed thanks to this patch, mostly by Stanislaw Gruszka.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (320.00 B)
11-merge (11.87 kB)
Download all attachments

2006-01-13 10:12:59

by Duncan Sands

[permalink] [raw]
Subject: [PATCH 13/13] USBATM: -EILSEQ workaround

Don't throttle on -EILSEQ urb status if requested by a minidriver.
It seems the ueagle modems are buggy, giving -EILSEQ when they
have no data to send. The ueagle change will be sent separately
by the ueagle guys. Patch by Matthieu Castet.

Signed-off-by: Duncan Sands <[email protected]>


Attachments:
(No filename) (290.00 B)
13-EILSEQ (0.98 kB)
Download all attachments

2006-01-13 10:30:06

by Duncan Sands

[permalink] [raw]
Subject: Re: [linux-usb-devel] Re: [PATCH 00/13] USBATM: summary

> I only got 2 of these, is my mail just being slow (which it does at odd
> times), or did you stop sending them based on some problems on your end?

Hi Greg, I stopped because I noticed that part of patch 2 had slipped
into patch 3 (the bit that slipped was exactly the tweak I made to patch 2
to have it compile by itself... probably ended up in patch 3 due to a
forgotten "quilt refresh"), and I wanted to check all the other patches.
They are fine, so I have now sent them. However, patch 2 won't compile
unless you apply patch 3 as well. Since they are both trivial, I hope you
are ok with that, otherwise I can rediff them

All the best,

Duncan.

2006-08-18 19:44:33

by Giampaolo Tomassoni

[permalink] [raw]
Subject: R: [PATCH 06/13] USBATM: shutdown open connections when disconnected

> -----Messaggio originale-----
> Da: [email protected]
> [mailto:[email protected]]Per conto di Duncan Sands
> Inviato: venerd? 13 gennaio 2006 10.05
> A: Greg KH
> Cc: [email protected];
> [email protected]; [email protected];
> [email protected]
> Oggetto: [PATCH 06/13] USBATM: shutdown open connections when
> disconnected
>
>
> This patch causes vcc_release_async to be applied to any open
> vcc's when the modem is disconnected. This signals a socket
> shutdown, letting the socket user know that the game is up.
> I wrote this patch because of reports that pppd would keep
> connections open forever when the modem is disconnected.
> This patch does not fix that problem, but it's a step in the
> right direction. It doesn't help because the pppoatm module
> doesn't yet monitor state changes on the ATM socket, so simply
> never realises that the ATM connection has gone down (meaning
> it doesn't tell the ppp layer). But at least there is a socket
> state change now.

> Unfortunately this patch may create problems
> for those rare users like me who use routed IP or some other
> non-ppp connection method that goes via the ATM ARP daemon: the
> daemon is buggy, and with this patch will crash when the modem
> is disconnected. Users with a buggy atmarpd can simply restart
> it after disconnecting the modem.

The problem is not only atmarpd, but also atmarp: it fails when establishing a PVC on a 'disconnected' VC, ie: a VC on and ADSL modem which didn't yet reach showtime.

CLIP and other protocols-over-ATM are designed to be connectionless. Thereby, you setup a CLIP connection, probably at systems startup, and forget about it: if a carrier is available, cells are exchanged, otherwise both parties discard outgoing cells. The connectionless design is one of the pros of CLIP with respect to, in example, PPPoA. It is not a con.

With this patch, the CLIP protocol becomes susceptible to connection status: it fails during setup if the ADSL modem is not yet at showtime (and who knows how long it will take to get it) and may discard the VC when the carrier happens to get lost after CLIP is started.

To my knowledge, no other ATM interface driver under Linux does reject a vcc open or a send due to the state of the physical carrier. Also note that there is not a per-interface instance of atmarpd and friends: they are scheduled systemwide and they may work at once on more interfaces. Making them die causes troubles not only to the outstanding network activity of the disconnected ADSL, but also to the one on other ATM interfaces.

pppd is designed to work both with connectionless (a cross-wire cable or an ATM VC) and connection-oriented carriers (a voice or ISDN modem): enabling its lcp echo feature usually is enough in order to detect a lost carrier. Out-of-sync peers state should be also detected. If it doesn't, it's pppd failure.

In summary, I'm not shure that the new USBATM behaviour is compatible with the (at least de-facto) actual standard of the Linux ATM stack. I'm not even shure that's compatible with ATM blueprints.

I'm however pretty shure that, when there is a patch that may impair and/or be incompatible with current software, the Linux end-user had always been provided with an option to disable the new feature and continue working the way it was used to.

I need that option.

Regards,

giampaolo

>
> Signed-off-by: Duncan Sands <[email protected]>
>