On 2021/3/14 18:15, Marc Kleine-Budde wrote: > Cc += linux-can@xxxxxxxxxxxxxxx > > 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@xxxxxxxxxx> >>> --- >> >> 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@xxxxxxxxxxxxxx/ 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 >
