Re: [virtio-comment] [PATCH] virtio-net: Add an optional device control over the receive buffers length

[Vitaly Mireyno <vmireyno@marvell.com>]

>-----Original Message-----
>From: Jason Wang <jasowang@redhat.com>
>Sent: Thursday, 23 January, 2020 9:11
>To: Michael S. Tsirkin <mst@redhat.com>; Vitaly Mireyno
><vmireyno@marvell.com>
>Cc: virtio-comment@lists.oasis-open.org
>Subject: Re: [EXT] Re: [virtio-comment] [PATCH] virtio-net: Add an optional
>device control over the receive buffers length
>
>
>On 2020/1/23 äå2:55, Michael S. Tsirkin wrote:
>> On Wed, Jan 22, 2020 at 04:06:30PM +0000, Vitaly Mireyno wrote:
>>>
>>>> -----Original Message-----
>>>> From: Michael S. Tsirkin <mst@redhat.com>
>>>> Sent: Monday, 20 January, 2020 18:25
>>>> To: Vitaly Mireyno <vmireyno@marvell.com>
>>>> Cc: virtio-comment@lists.oasis-open.org; Jason Wang
>>>> <jasowang@redhat.com>
>>>> Subject: [EXT] Re: [virtio-comment] [PATCH] virtio-net: Add an
>>>> optional device control over the receive buffers length
>>>>
>>>> --------------------------------------------------------------------
>>>> -- On Mon, Dec 30, 2019 at 01:59:17PM +0000, Vitaly Mireyno wrote:
>>>>> This patch gives devices some level of control over the receive
>>>>> buffers
>>>> length.
>>>>> The driver declares the minimum receive buffer length, and the
>>>>> device
>>>> requests max/min buffer length ratio.
>>>>> This is a follow-up to the "[PATCH] virtio-net: Add equal-sized
>>>>> receive buffers feature" discussion:
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.oasis-2D
>>>>> ope
>>>>> n.org_archives_virtio-
>>>> 2Dcomment_201912_msg00007.html&d=DwIFAg&c=nKjWec
>>>>
>2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&
>>>> m=jrQgZ
>>>>> -
>>>>
>iN3QnvFqxQQkoPTmztJRTNCXTF2dz68TEV0cA&s=pDtISMJpw1N9ohot9fluo1N
>>>> j1X2s1
>>>>> J_sEVJGg2uwkZk&e=
>>>>>
>>>>>
>>>>> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
>>>>> ---
>>>>>   content.tex | 41 +++++++++++++++++++++++++++++++++++++++--
>>>>>   1 file changed, 39 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/content.tex b/content.tex index d68cfaf..0a4cfba
>>>>> 100644
>>>>> --- a/content.tex
>>>>> +++ b/content.tex
>>>>> @@ -2815,6 +2815,13 @@ \subsection{Feature bits}\label{sec:Device
>>>>> Types / Network Device / Feature bits
>>>> \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through
>>>> control
>>>>>       channel.
>>>>>
>>>>> +\item[VIRTIO_NET_F_RXBUF_LEN_RATIO(57)] Device requests to limit
>the
>>>>> +    maximum/minimum receive buffers length ratio.
>>>>> +
>>>>> +\item[VIRTIO_NET_F_RXBUF_MIN_LEN(58)] Device requests to know
>the
>>>> minimum
>>>>> +    receive buffers length. Driver declares the minimum receive buffers
>>>>> +    length.
>>>>> +
>>>>>   \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the
>exact
>>>> \field{hdr_len}
>>>>>       value. Device benefits from knowing the exact header length.
>>>>>
>>>> So I am wondering, when we are switching from regular skbs to XDP,
>>>> all buffers become 4K. When we switch back, we get variability again.
>>>> All the while some old buffers can be outstanding.
>>>>
>>>> How will we handle this?
>>>>
>>> The feature limits the flexibility of the driver to set the descriptors length.
>>> It doesn't limit the actual buffer allocation size. So if XDP requires a larger
>headroom, I don't see a problem. Please excuse my ignorance, but what do
>you mean by "all buffers become 4K"?
>> So linux can switch between skb and XDP mode. In skb mode buffer size
>> varies, it works by merging multiple buffers. In XDP a single buffer
>> is made big enough to hold the whole packet. Length is fixed.
>>
>> If we are in XDP mode but buffer was added while in skb mode, or vice
>> versa, we recover generally by copying data to the correct buffer.
>> This is a temporary slowdown - better than dropping packets.
>>
>> So let's assume the device ratio is 1.
>> I guess while in XDP mode, we'll write XDP buffer length into this
>> field. But in skb mode, we can make the buffer smaller.
>> This implies driver needs to change the min_rx_buf_len?
>
>
>I'm not sure I get here. The header room should be invisible from device point
>of view I think.
>
>Thanks
>

When the device ratio is 1 (meaning that the device requests constant size descriptors length), I'd suggest that the driver set all descriptors length to the MTU-based size, such that a single descriptor could be enough to hold any received packet, and this is regardless of SKB or XDP mode.
A buffer headroom is indeed invisible to the device, and the driver is free to allocate any buffer size. Only the descriptors length is restricted in this mode.
Does this make sense?