On Sat, Mar 11, 2023 at 05:25:04PM -0500, Feng Liu wrote: > > > On 2023-03-11 p.m.2:05, Michael S. Tsirkin wrote: > > External email: Use caution opening links or attachments > > > > > > On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote: > > > > > > > > > On 2023-03-10 a.m.8:36, Parav Pandit wrote: > > > > > > > > > > > > > From: Feng Liu <feliu@xxxxxxxxxx> > > > > > Sent: Friday, March 10, 2023 12:34 AM > > > > > > > > > > > > > > - if (!is_power_of_2(num)) { > > > > > - dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", > > > > > num); > > > > > - return ERR_PTR(-EINVAL); > > > > > - } > > > > > - > > > > > > > > The check is still valid for split q. > > > > Maybe the right place for such a check is not the pci transport driver. > > > > But layer below where split vs packed q knowledge resides and hence, power of 2 check can be omitted for packed vq. > > > > > > Hi, Parav > > > I think you are right, I checked the virtio spec, only packed virtqueue > > > can use queue size which is not power_of_2; so, I think the check can be > > > reserved only for split queue here, something like > > > > > > struct virtio_device *vdev = &vp_dev->vdev; > > > if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED) > > > && !is_power_of_2(num)){ > > > dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num); > > > return ERR_PTR(-EINVAL); > > > } > > > > > > I will fix it in next version > > > > > > Hi, Michael and Jason > > > Do you have any comments on this? > > > > > > > Hmm good point. Which raises the question: so how did you test it then? > > > Hi Michael > > I will construct a non power of 2 size packed virtqueue device to test > whether the driver is loaded successfully and whether the traffic is normal; > at the same time, I will also construct a non power of 2 size split > virtqueue device for testing to see if an warning is given and the driver is > loaded fail > > The method of constructing the device, such as the reply steps in the > previous email Okay but previously you said you tested ring sizes 100 and 200 with iperf. How did you construct these? > > > > -- > > MST > > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization