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 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?

Regards,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to