From: Rafal Redzimski <[email protected]>
Current implementation updates the mtu size and notify cdc_ncm
device using USB_CDC_SET_MAX_DATAGRAM_SIZE request about datagram
size change instead of changing rx_urb_size.
Whenever mtu is being changed, datagram size should also be
updated.
Cc: <[email protected]>
Signed-off-by: Rafal Redzimski <[email protected]>
Signed-off-by: Robert Dobrowolski <[email protected]>
---
drivers/net/usb/cdc_ncm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 2fb31ed..4a47656 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -746,6 +746,8 @@ int cdc_ncm_change_mtu(struct net_device *net, int new_mtu)
if (new_mtu <= 0 || new_mtu > maxmtu)
return -EINVAL;
net->mtu = new_mtu;
+ cdc_ncm_set_dgram_size(dev, new_mtu);
+
return 0;
}
EXPORT_SYMBOL_GPL(cdc_ncm_change_mtu);
--
1.9.1
Robert Dobrowolski <[email protected]> writes:
> From: Rafal Redzimski <[email protected]>
>
> Current implementation updates the mtu size and notify cdc_ncm
> device using USB_CDC_SET_MAX_DATAGRAM_SIZE request about datagram
> size change instead of changing rx_urb_size.
>
> Whenever mtu is being changed, datagram size should also be
> updated.
Definitely! Thanks for this. But looking at the code I believe you
need to fix the calculation of maxmtu too. It is currently:
int maxmtu = ctx->max_datagram_size - cdc_ncm_eth_hlen(dev);
And cdc_ncm_set_dgram_size() updates ctx->max_datagram_size with the new
mtu, meaning that you can only reduce the mtu. We should probably use
cdc_ncm_max_dgram_size() instead here.
And cdc_ncm_set_dgram_size() takes the datagram size with header as
input (ref the above maxmtu calucalution), so it probably needs to
called as
cdc_ncm_set_dgram_size(dev, new_mtu + cdc_ncm_eth_hlen(dev));
to get it right. I think. None of this is tested on an actual device
yet... Care to test if I'm right, and do a v2 if necessry?
> Cc: <[email protected]>
This should be dropped for net. Ask David to queue it for stable
instead. I usually do that by using a subject prefix like
[PATCH net,stable v1] ...
Bjørn
> Robert Dobrowolski <[email protected]> writes:
>
>> From: Rafal Redzimski <[email protected]>
>>
>> Current implementation updates the mtu size and notify cdc_ncm
>> device using USB_CDC_SET_MAX_DATAGRAM_SIZE request about datagram
>> size change instead of changing rx_urb_size.
>>
>> Whenever mtu is being changed, datagram size should also be
>> updated.
>
> Definitely! Thanks for this. But looking at the code I believe you
> need to fix the calculation of maxmtu too. It is currently:
>
> int maxmtu = ctx->max_datagram_size - cdc_ncm_eth_hlen(dev);
>
> And cdc_ncm_set_dgram_size() updates ctx->max_datagram_size with the new
> mtu, meaning that you can only reduce the mtu. We should probably use
> cdc_ncm_max_dgram_size() instead here.
>
> And cdc_ncm_set_dgram_size() takes the datagram size with header as
> input (ref the above maxmtu calucalution), so it probably needs to
> called as
>
> cdc_ncm_set_dgram_size(dev, new_mtu + cdc_ncm_eth_hlen(dev));
>
> to get it right. I think. None of this is tested on an actual device
> yet... Care to test if I'm right, and do a v2 if necessry?
>
>> Cc: <[email protected]>
>
> This should be dropped for net. Ask David to queue it for stable
> instead. I usually do that by using a subject prefix like
>
> [PATCH net,stable v1] ...
>
>
>
>
> Bjørn
>
ok, thanks for feedback I will send v2 patch