On 2021/3/14 18:15, Marc Kleine-Budde wrote:
Cc += linux-can@vger.kernel.org
On 3/14/21 1:03 AM, Vladimir Oltean wrote:
On Sat, Mar 13, 2021 at 10:47:47AM +0800, Yunsheng Lin wrote:
Currently pfifo_fast has both TCQ_F_CAN_BYPASS and TCQ_F_NOLOCK flag set, but queue discipline by-pass does not work for lockless qdisc because skb is always enqueued to qdisc even when the qdisc is empty, see __dev_xmit_skb().
This patch calles sch_direct_xmit() to transmit the skb directly to the driver for empty lockless qdisc too, which aviod enqueuing and dequeuing operation. qdisc->empty is set to false whenever a skb is enqueued, and is set to true when skb dequeuing return NULL, see pfifo_fast_dequeue().
Also, qdisc is scheduled at the end of qdisc_run_end() when q->empty is false to avoid packet stuck problem.
The performance for ip_forward test increases about 10% with this patch.
Signed-off-by: Yunsheng Lin linyunsheng@huawei.com
I can confirm the ~10% IP forwarding throughput improvement brought by this patch, but as you might be aware, there was a previous attempt to add qdisc bypass to pfifo_fast by Paolo Abeni: https://lore.kernel.org/netdev/661cc33a-5f65-2769-cc1a-65791cb4b131@pengutro...
Thanks for mention the previous attempt to add qdisc bypass to pfifo_fast.
It was reverted because TX reordering was observed with SocketCAN (although, presumably it should also be seen with Ethernet and such).
When writing this patch, I was more foucusing on packet stuck problem when TCQ_F_CAN_BYPASS is added for lockless qdisc.
When I am looking at flexcan_start_xmit() used by the can driver you mentioned, it calls netif_stop_queue() to disable the queue when sending each skb, which may cuause other skb to be requeued, see dev_requeue_skb() called by sch_direct_xmit(), and q->empty is still true when this happens, so other cpu may send skb directly bypassing the requeued skb, causing an out of order problem.
I will try to deal with the above requeued skb problem, and see if there are other timing issus beside the requeued skb problem.
Thanks for the testing again.
Thanks for testing that, I just stumbled over this patch by accident.
Marc