Si-Wei Liu
2022-Sep-07 21:39 UTC
[virtio-dev] RE: [PATCH v5 2/2] virtio-net: use mtu size as buffer length for big packets
On 9/7/2022 12:51 PM, Parav Pandit wrote:>> And I'd like commit log to include results of perf testing >> - with indirect feature on > Which device do you suggest using for this test? >You may use software vhost-net backend with and without fix to compare. Since this driver fix effectively lowers down the buffer size for the indirect=on case as well, it's a natural request to make sure no perf degradation is seen on devices with indirect descriptor enabled. I don't expect degradation though and think this patch should improve efficiency with less memory foot print. Cheers, -Siwei
Parav Pandit
2022-Sep-07 22:11 UTC
[virtio-dev] RE: [PATCH v5 2/2] virtio-net: use mtu size as buffer length for big packets
> From: Si-Wei Liu <si-wei.liu at oracle.com> > Sent: Wednesday, September 7, 2022 5:40 PM > > > On 9/7/2022 12:51 PM, Parav Pandit wrote: > >> And I'd like commit log to include results of perf testing > >> - with indirect feature on > > Which device do you suggest using for this test? > > > You may use software vhost-net backend with and without fix to compare. > Since this driver fix effectively lowers down the buffer size for the > indirect=on case as well,Do you have sample example for this?> it's a natural request to make sure no perf > degradation is seen on devices with indirect descriptor enabled. I don't > expect degradation though and think this patch should improve efficiency > with less memory foot print. >Any specific reason to discount test for the packed vq here because the change applies to packed vq too? It is counter intuitive to see degradation with smaller size buffers. But sure, code reviews can miss things that can bring regression for which it should be tested. I am not against the test itself. It is good thing to do more test coverage. What is puzzling me is, I fail to see test results not available in previous commits and cover letters, which is making this patch special for test coverage. Or a new trend begins with this specific patch onwards? For example, I don?t see a test results in commit [1], [2], [3] to indicate that no degradation is seen which heavily touches the lock in core data path. So want to know test reporting guidelines in the commit logs for this and other patches. [1] commit 5a2f966d0f [2] commit a7766ef18b33 [3] commit 22bc63c58e876