Oliver Neukum <oneukum@xxxxxxx> writes:
> looking at cdc-ncm it seeems to me that cdc-ncm is forced to play
> very dirty games because usbnet doesn't have a notion about aggregating
> packets for a single transfer.
>
> It seems to me that this can be fixed introducing a method for bundling,
> which tells usbnet how packets have been aggregated. To have performance
> usbnet strives to always keep at least two transfers in flight.
>
> The code isn't complete and I need to get a device for testing, but to get
> your opinion, I ask you to comment on what I have now.
I haven't tested this on any device either, but it looks like the right
direction to me. Note that there are several other existing minidrivers
which could take advantage of this code. A quick look found that both
sierra_net and rndis_host have some unbundling support in their
rx_fixups, but both ignore tx bundling. rndis_host has the following
explanation in tx_fixup:
/* fill out the RNDIS header. we won't bother trying to batch
* packets; Linux minimizes wasted bandwidth through tx queues.
*/
Although that may be true for the host, I am not sure it will be true
for every device?
> @@ -874,71 +840,37 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
> /* return skb */
> ctx->tx_curr_skb = NULL;
> ctx->netdev->stats.tx_packets += ctx->tx_curr_frame_num;
Maybe all the statistics could be moved back to usbnet now that it will
see all packets again?
> index f87cf62..bb2f622 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -33,6 +33,7 @@ struct usbnet {
> wait_queue_head_t *wait;
> struct mutex phy_mutex;
> unsigned char suspend_count;
> + atomic_t tx_in_flight;
>
> /* i/o info: pipes etc */
> unsigned in, out;
So you don't keep any timer here? The idea is that you always will
transmit immediately unless there are two transmit's in flight? I
believe that is a change which has to be tested against real devices.
They may be optimized for receiving bundles.
Not really related, but I am still worrying how MBIM is going to fit
into the usbnet model where you have a strict relation between one
netdev and one USB interface. Do you see any way usbnet could be
extended to manage a list of netdevs?
All the netdev related functions doing
struct usbnet *dev = netdev_priv(net);
would still work. But all the USB related functions using dev->net to
get _the_ netdev would fail. Maybe this will be too difficult to
implement within the usbnet framework at all?
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
