Re: [PATCH] xsk: add cq event

From: "Björn Töpel" <bjorn.topel@intel.com>
To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
	"Karlsson, Magnus" <magnus.karlsson@intel.com>
Cc: Magnus Karlsson <magnus.karlsson@intel.com>,
	Jonathan Lemon <jonathan.lemon@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH] xsk: add cq event
Date: Tue, 17 Nov 2020 11:00:05 +0100	[thread overview]
Message-ID: <3306b4d8-8689-b0e7-3f6d-c3ad873b7093@intel.com> (raw)
In-Reply-To: <964677c6-442c-485e-9268-3a801dbd4bd3@orsmsx607.amr.corp.intel.com>

On 2020-11-16 17:12, Xuan Zhuo wrote:
> On Mon, 16 Nov 2020 15:31:20 +0100, =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn.topel@intel.com> wrote:
> 
>> On 2020-11-16 09:10, Xuan Zhuo wrote:
> 
>>> When we write all cq items to tx, we have to wait for a new event based
> 
>>> on poll to indicate that it is writable. But the current writability is
> 
>>> triggered based on whether tx is full or not, and In fact, when tx is
> 
>>> dissatisfied, the user of cq's item may not necessarily get it, because it
> 
>>> may still be occupied by the network card. In this case, we need to know
> 
>>> when cq is available, so this patch adds a socket option, When the user
> 
>>> configures this option using setsockopt, when cq is available, a
> 
>>> readable event is generated for all xsk bound to this umem.
> 
>>>
> 
>>> I can't find a better description of this event,
> 
>>> I think it can also be 'readable', although it is indeed different from
> 
>>> the 'readable' of the new data. But the overhead of xsk checking whether
> 
>>> cq or rx is readable is small.
> 
>>>
> 
>>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> 
>>
> 
>> Thanks for the patch!
> 
>>
> 
>> I'm not a fan of having two different "readable" event (both Rx and cq).
> 
>> Could you explain a bit what the use case is, so I get a better
> 
>> understanding.
> 
>>
> 
>> The Tx queues has a back-pressure mechanism, determined of the number of
> 
>> elements in cq. Is it related to that?
> 
>>
> 
>> Please explain a bit more what you're trying to solve, and maybe we can
> 
>> figure out a better way forward!
> 
>>
> 
>>
> 
>> Thanks!
> 
>> Björn
> 
> I want to implement a tool for mass sending. For example, the size of cq is
> 
> 1024, and I set the size of tx also to 1024, so that I will put all cq in tx at
> 
> once, and then I have to wait for an event, come Indicates that there is new
> 
> write space or new cq is available.
> 
> 
> 
> At present, we can only monitor the event of write able. This indicates whether
> 
> tx is full, but in fact, tx is basically not full, because the full state is
> 
> very short, and those tx items are being used by the network card. And
> 
> epoll_wait will be awakened directly every time, without waiting, but I cannot
> 
> get the cq item, so I still cannot successfully send the package again.
> 
> 
> 
> Of course, I don't like the "readable" event very much. This is a suitable
> 
> one I found in the existing epoll event. ^_^
>

More questions! By "Mass sending" do you mean maximum throughput, or
does that mean "in very large batches"?

For the latter to do 1k batches, you could increase the Tx/cq buffer
size to say 4k.

For maximum thoughput it's better to use smaller batches (e.g. what the
txpush scenario in samples/xdpsock does).

You're right that even if there's space in the Tx ring, it wont be sent
unless there's sufficient space in the cq ring. Maybe it would make
sense to be more restrictive when triggering the "writable" socket
event? E.g. only trigger it when there's space in Tx *and* sufficient cq
space?

Björn

> 
> 
> Thanks.
> 