Re: [PATCH 1/2] vdpa/mlx5: Make VIRTIO_NET_F_MRG_RXBUF off by default - Si-Wei Liu

From: Si-Wei Liu <si-wei.liu@oracle.com>
To: Parav Pandit <parav@mellanox.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	 Eli Cohen <elic@nvidia.com>
Cc: Eugenio Perez Martin <eperezma@redhat.com>,
	"perezma@redhat.com" <perezma@redhat.com>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>
Subject: Re: [PATCH 1/2] vdpa/mlx5: Make VIRTIO_NET_F_MRG_RXBUF off by default
Date: Mon, 13 Mar 2023 15:19:19 -0700	[thread overview]
Message-ID: <f269688b-8392-406c-e070-369b25acc252@oracle.com> (raw)
In-Reply-To: <PH0PR12MB54816C24BAA322567BED21A0DCB89@PH0PR12MB5481.namprd12.prod.outlook.com>


On 3/12/2023 6:28 AM, Parav Pandit wrote:
>
>> From: Michael S. Tsirkin <mst@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@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.
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization