* [PATCH v1] net: cdc_ncm: update datagram size after changing mtu
@ 2016-04-06 11:41 Robert Dobrowolski
2016-04-06 14:36 ` Bjørn Mork
0 siblings, 1 reply; 3+ messages in thread
From: Robert Dobrowolski @ 2016-04-06 11:41 UTC (permalink / raw)
To: linux-usb
Cc: robert.dobrowolski, rafal.f.redzimski, stable, oliver, netdev,
linux-kernel
From: Rafal Redzimski <rafal.f.redzimski@intel.com>
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: <stable@vger.kernel.org>
Signed-off-by: Rafal Redzimski <rafal.f.redzimski@intel.com>
Signed-off-by: Robert Dobrowolski <robert.dobrowolski@linux.intel.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1] net: cdc_ncm: update datagram size after changing mtu
2016-04-06 11:41 [PATCH v1] net: cdc_ncm: update datagram size after changing mtu Robert Dobrowolski
@ 2016-04-06 14:36 ` Bjørn Mork
2016-04-11 12:00 ` Robert Dobrowolski
0 siblings, 1 reply; 3+ messages in thread
From: Bjørn Mork @ 2016-04-06 14:36 UTC (permalink / raw)
To: Robert Dobrowolski
Cc: linux-usb, rafal.f.redzimski, stable, oliver, netdev, linux-kernel
Robert Dobrowolski <robert.dobrowolski@linux.intel.com> writes:
> From: Rafal Redzimski <rafal.f.redzimski@intel.com>
>
> 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: <stable@vger.kernel.org>
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] net: cdc_ncm: update datagram size after changing mtu
2016-04-06 14:36 ` Bjørn Mork
@ 2016-04-11 12:00 ` Robert Dobrowolski
0 siblings, 0 replies; 3+ messages in thread
From: Robert Dobrowolski @ 2016-04-11 12:00 UTC (permalink / raw)
To: Bjørn Mork
Cc: Robert Dobrowolski, linux-usb, rafal.f.redzimski, stable, oliver,
netdev, linux-kernel
> Robert Dobrowolski <robert.dobrowolski@linux.intel.com> writes:
>
>> From: Rafal Redzimski <rafal.f.redzimski@intel.com>
>>
>> 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: <stable@vger.kernel.org>
>
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-11 11:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-06 11:41 [PATCH v1] net: cdc_ncm: update datagram size after changing mtu Robert Dobrowolski
2016-04-06 14:36 ` Bjørn Mork
2016-04-11 12:00 ` Robert Dobrowolski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).