Oliver Hartkopp wrote:
> Matthias Fuchs wrote:
>
>> But I ran into some issues. Perhaps some of you can help:
>> Q1: Under high TX load the driver runs out of free tx_contexts.
>> (see "couldn't find free context"). I expected the socket can core
>> to take care of this so that the lower driver cannot be overrun
>> by tx jobs. Either this expectation is wrong or there's a bug in
>> the driver :-)
>
> Hi Matthias,
>
> i wonder, why you do not stop the netdev tx queue immediately at the
> beginning of
>
>> +static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
>> + struct net_device *netdev)
>
> but at this (late) point:
>
>> + if (err) {
>> + can_free_echo_skb(netdev, context->echo_index);
>> +
>> + atomic_dec(&priv->active_tx_urbs);
>> + usb_unanchor_urb(urb);
>> +
>> + if (err == -ENODEV)
>> + netif_device_detach(netdev);
>> + else
>> + dev_warn(ND2D(netdev), "failed tx_urb %d\n", err);
>> +
>> + goto releasebuf;
>> + } else {
>> + netdev->trans_start = jiffies;
>> +
>> + /* Slow down tx path */
>> + if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
>> + netif_stop_queue(netdev);
>> + }
The driver queues more than one TX packet for the sake of throughput.
Sebastian mentioned that there is a big improvement with 10 TX URBs.
> The netdev queues in Linux will give you skbs until you say 'stop' :-)
> This is not SocketCAN specific.
>
> I assume the handling of netif_[wake|stop]_queue() should be reworked.
>
> E.g. invoke netif_stop_queue() at the beginning of esd_usb2_start_xmit() and
> when everything is really fine, you can invoke netif_wake_queue() at the and
> of esd_usb2_start_xmit() again.
>
> Btw. did you check the above code with
>
> if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS - 1)
>
> to test for an off-by-one problem in the tx_urbs?
So far and in the EMS USB driver, the stop/wakeup is handled based on
the "priv->active_tx_urbs" count. If there are no more tx_urbs, netdev
will be stopped and restarted later in esd_usb2_write_bulk_callback().
At a first glance, I think there's the problem. The context needs to be
stored until esd_usb2_tx_done_msg() is called. That function must also
wakeup the netdev queue.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
