Comment 33 for bug 659143

Hi Seth,

Sorry for the lateish response. Great that you are backporting my patch.

Be aware that I don't have that much C/kernel/WLAN knowledge beyond this patch (my first).

The usb drivers use REGISTER_TIMEOUT32(entry->skb->len), evidently basing the timeout value on the length of the data written. Based on that it would now be REGISTER_TIMEOUT32(entry->skb->len + padding_len). However, that time seems to be rather long already, my beacon e.g. would have a timeout of 11500ms. padding_len would only add a maximum of 375ms. I tried to find some indication of how long that timeout really has to be. It feels like people are using various incarnations of "plenty".

It would have been possible to just backport the beacon padding for the two pci drivers, since only they use the changed register_multiwrite() from the second patch in 2.6.35, so only they need it as preparation for that patch. But the usb beacons should be padded anyway, its nice to have.

Regarding the missing return value check. Good question. No, not sure that it will always be successful. Normal beacons should always have enough tailroom so that skb_pad() does not even have to ask for more memory, but we are not sure that this will always be the case. If an extra long beacon should need to be padded in an out of memory situation skb_pad() might fail, not getting the memory. I tried but could not provoke that. Seems linux does like to give skb_pad() what it asks for even while oom-killing. If it does happen however skb_pad() would free the skb and the call to free it again with dev_kfree_skb_any() produces a hard panic that will leave the user without anything useful to report (unless he already lay in wait before the console with an HD video camera). That, plus seeing that this is checked everywhere else gives me second thoughts, so I am considering proposing a patch upstream (for linux-next). I'd appreciate your thoughts on this issue.

I successfully tried your test kernel with rt2800pci. It now loads the firmware (and also works after that in station mode). Testing against regressions introduced by the padding patch is trickier as it involves four different drivers and this code does only get used in ad hoc mode.

Thanks,
Wolfgang