Linux Virtualization - Mar 2023 - [PATCH 1/2] vdpa/mlx5: Make VIRTIO_NET_F_MRG_RXBUF off by default

On 3/12/2023 6:28 AM, Parav Pandit wrote:
>
>> From: Michael S. Tsirkin <mst at redhat.com>
>> Sent: Sunday, March 12, 2023 6:25 AM
>>
>> On Sun, Mar 12, 2023 at 11:10:25AM +0200, Eli Cohen wrote:
>>> On 12/03/2023 10:58, Michael S. Tsirkin wrote:
>>>> On Sun, Mar 12, 2023 at 10:39:19AM +0200, Eli Cohen wrote:
>>>>> One can still enable it when creating the vdpa device using
vdpa
>>>>> tool by providing features that include it.
>>>>>
>>>>> For example:
>>>>> $ vdpa dev add name vdpa0 mgmtdev pci/0000:86:00.2
device_features
>>>>> 0x300cb982b
>>>>>
>>>>> Signed-off-by: Eli Cohen <elic at nvidia.com>
>>>> What's the reason to turn it off by default? It's
generally a
>>>> performance win isn't it?
>>> It has negative impact on packet rate so we want to keep it off by
default.
> The performance characteristics is very workload specific.
> It is less of interest given the primary reason is backward compatibility,
more below.
Actually there's no such burden or requirement to maintain backward 
compatibility for the default 'vdpa dev add' behavior if dedicated 
device_features is not specified. Historically the default vdpa creation 
on mlx5 ever got changed from single queue to 8 queue pairs when 
VIRTIO_NET_F_MQ feature was first introduced to mlx5_vdpa, then the 
default switched back to 1 data queue pair again when max_vqp attribute 
was added to the vdpa tool. Essentially, every addition of new feature 
to mlx5_vdpa, e.g. CTRL_VQ, CTRL_VLAN, and et al, effectively changed 
the default "vdpa dev add" behavior not just only once: the backward 
compatibility guarantee is simply just not there and ever.

-Siwei
>
>> Interesting.  I feel this would benefit from a bit more analysis.
>> Packet rate with dpdk? With linux? Is there a chance this will regress
some
>> workloads?
>> VIRTIO_NET_F_MRG_RXBUF was designed to save memory, which is good for
>> small tcp buffers.
> Eli,
> Please update the commit message.
> This change is to avoid regression in existing systems.
> The device previously didn't report MRG_RXBUF cap and it was not in
use.
> Lately, certain devices are reporting this feature bit and it is breaking
the backward compatibility.
> So the driver keeps it disabled by default.
> User should enable it when user prefers to.
>