On Mon, Mar 13, 2023 at 09:14:38PM +0000, Parav Pandit wrote: > > > > From: Michael S. Tsirkin <mst@xxxxxxxxxx> > > Sent: Sunday, March 12, 2023 12:24 PM > > > > On Sun, Mar 12, 2023 at 01:28:06PM +0000, 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 0x300cb982b > > > > > > > > > > > > > > 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. > > > > > > > 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. > > > > OK. And which commit changes that? > vdpa dev add command [1] has the ability to set the desired features. > The commit log of this patch has an example too. > > [1] https://elixir.bootlin.com/linux/v6.3-rc2/C/ident/vdpa_nl_cmd_dev_add_set_doit I mean if this is claiming to fix a performance regression it should have a Fixes: tag with the commit that introduced the regression. -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
