Re: [PATCH 1/2] vdpa/mlx5: Make VIRTIO_NET_F_MRG_RXBUF off by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 3/12/2023 6:28 AM, Parav Pandit wrote:

From: Michael S. Tsirkin <mst@xxxxxxxxxx>
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

Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
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.


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
VIRTIO_NET_F_MRG_RXBUF was designed to save memory, which is good for
small tcp buffers.
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

[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux