Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 15 Mar 2021 14:29:34 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Yunsheng Lin <linyunsheng@huawei.com>
Cc:     davem@davemloft.net, kuba@kernel.org, ast@kernel.org,
        daniel@iogearbox.net, andriin@fb.com, edumazet@google.com,
        weiwan@google.com, cong.wang@bytedance.com, ap420073@gmail.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        linuxarm@openeuler.org, mkl@pengutronix.de,
        linux-can@vger.kernel.org
Subject: Re: [RFC v2] net: sched: implement TCQ_F_CAN_BYPASS for lockless
 qdisc
Message-ID: <20210315122934.kano66d67uqmrr4t@skbuf>
References: <1615603667-22568-1-git-send-email-linyunsheng@huawei.com>
 <1615777818-13969-1-git-send-email-linyunsheng@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1615777818-13969-1-git-send-email-linyunsheng@huawei.com>
Precedence: bulk

On Mon, Mar 15, 2021 at 11:10:18AM +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 calls 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, see pfifo_fast_enqueue(), and is set to true when
> skb dequeuing return NULL, see pfifo_fast_dequeue(), a spinlock is
> added to avoid the race between enqueue/dequeue and qdisc->empty
> setting.
> 
> If there is requeued skb in q->gso_skb, and qdisc->empty is true,
> do not allow bypassing requeued skb. enqueuing and dequeuing in
> q->gso_skb is always protected by qdisc->seqlock, so is the access
> of q->gso_skb by skb_queue_empty();
> 
> 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>
> ---
> RFC V2: fix requeued skb out of order and data race problem.
> ---

This is great. Looks like requeued skbs bypassing the qdisc were indeed
the problem. I ran my stress test for 4:30 hours (7.7 million CAN frames)
and all were received in the same order as canfdtest enqueued them in
the socket.

I'll let the test run some more, just thought I'd let you know that
things are looking good so far. I'll leave you a Tested-by when you
submit the final version of the patch as non-RFC.