Linux Virtualization - Mar 2023 - [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu
wrote:
> 
> 
> On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
> > > This patch series performs a clean up of the code in virtio_ring
and
> > > virtio_pci, modifying it to conform with the Linux kernel coding
style
> > > guidance [1]. The modifications ensure the code easy to read and
> > > understand. This small series does few short cleanups in the
code.
> > > 
> > > Patch-1 Allow non power of 2 sizes for virtqueues
> > > Patch-2 Avoid using inline for small functions.
> > > Patch-3 Use const to annotate read-only pointer params.
> > > 
> > > [1]
> > >
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease&data=05%7C01%7Cfeliu%40nvidia.com%7C08831607a6fb4f58881408db213f8638%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638140328946332918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FnD4GINUds2HLLo47aY5Ps%2B9nKWPW2XRI35z1Hp0yx4%3D&reserved=0
> > > 
> > > All of the patches have been verified based on the kernel code
> > > commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> > 
> > verified how?
> > 
> Hi Michael
> 1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
> install the kernel, and use iperf to test traffic
> 2. To validate this change, we tested various virtqueue sizes for packed
> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
> CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
> passed successfully.
Given split ring does not support non power of 2 how exactly
did you configure non power of 2?
> 
> > > Feng Liu (3):
> > >    virtio_pci_modern: Allow non power of 2 sizes for virtqueues
> > >    virtio_ring: Avoid using inline for small functions
> > >    virtio_ring: Use const to annotate read-only pointer params
> > > 
> > >   drivers/virtio/virtio_pci_modern.c |  5 ----
> > >   drivers/virtio/virtio_ring.c       | 48
+++++++++++++++---------------
> > >   include/linux/virtio.h             | 14 ++++-----
> > >   3 files changed, 31 insertions(+), 36 deletions(-)
> > > 
> > > --
> > > 2.34.1
> >

On 2023-03-11 p.m.2:06, Michael S. Tsirkin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
>>
>>
>> On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
>>>> This patch series performs a clean up of the code in
virtio_ring and
>>>> virtio_pci, modifying it to conform with the Linux kernel
coding style
>>>> guidance [1]. The modifications ensure the code easy to read
and
>>>> understand. This small series does few short cleanups in the
code.
>>>>
>>>> Patch-1 Allow non power of 2 sizes for virtqueues
>>>> Patch-2 Avoid using inline for small functions.
>>>> Patch-3 Use const to annotate read-only pointer params.
>>>>
>>>> [1]
>>>>
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease&data=05%7C01%7Cfeliu%40nvidia.com%7C6cd34740c4674c1892f608db2263b300%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638141583834629671%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IA0QCCKjHnYiEk2vPlZ5WjlXs1CMXDphyyqTYnbqQqo%3D&reserved=0
>>>>
>>>> All of the patches have been verified based on the kernel code
>>>> commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
>>>
>>> verified how?
>>>
>> Hi Michael
>> 1. Applied the patches on lastest kernel source(44889ba56cbb), compile
and
>> install the kernel, and use iperf to test traffic
>> 2. To validate this change, we tested various virtqueue sizes for
packed
>> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
>> CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all
tests
>> passed successfully.
> 
> Given split ring does not support non power of 2 how exactly
> did you configure non power of 2?
> 
Hi, Michael
We can implement the test by modifying qemu; 1. force the 
VIRTIO_F_RING_PACKED feature bit to be set, 2. set 
VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE and VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE to 
the value of non power_of_2, 3. remove the check of is_power_of 
virtqueue, then qemu can create the required virtual device (non power_2 
size , packed virtqueue device) ;In this way, any length and packed ring 
test can be performed;
remove the modified code, I can test split vq, and can see that the size 
of power_of_2 can load the driver normally, and the size of non 
power_of_2 will give an warning and fail to load the driver
>>
>>>> Feng Liu (3):
>>>>     virtio_pci_modern: Allow non power of 2 sizes for
virtqueues
>>>>     virtio_ring: Avoid using inline for small functions
>>>>     virtio_ring: Use const to annotate read-only pointer params
>>>>
>>>>    drivers/virtio/virtio_pci_modern.c |  5 ----
>>>>    drivers/virtio/virtio_ring.c       | 48
+++++++++++++++---------------
>>>>    include/linux/virtio.h             | 14 ++++-----
>>>>    3 files changed, 31 insertions(+), 36 deletions(-)
>>>>
>>>> --
>>>> 2.34.1
>>>
>