Parav Pandit
2022-Jul-08 16:23 UTC
[PATCH V3 5/6] vDPA: answer num of queue pairs = 1 to userspace when VIRTIO_NET_F_MQ == 0
> From: Zhu, Lingshan <lingshan.zhu at intel.com> > Sent: Friday, July 8, 2022 2:21 AM > > > On 7/2/2022 6:07 AM, Parav Pandit wrote: > > > >> From: Zhu Lingshan <lingshan.zhu at intel.com> > >> Sent: Friday, July 1, 2022 9:28 AM > >> If VIRTIO_NET_F_MQ == 0, the virtio device should have one queue > >> pair, so when userspace querying queue pair numbers, it should return > >> mq=1 than zero. > >> > >> Function vdpa_dev_net_config_fill() fills the attributions of the > >> vDPA devices, so that it should call vdpa_dev_net_mq_config_fill() so > >> the parameter in vdpa_dev_net_mq_config_fill() should be > >> feature_device than feature_driver for the vDPA devices themselves > >> > >> Before this change, when MQ = 0, iproute2 output: > >> $vdpa dev config show vdpa0 > >> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs > >> 0 mtu 1500 > >> > > The fix belongs to user space. > > When a feature bit _MQ is not negotiated, vdpa kernel space will not add > attribute VDPA_ATTR_DEV_NET_CFG_MAX_VQP. > > When such attribute is not returned by kernel, max_vq_pairs should not > be shown by the iproute2. > I think userspace tool does not need to care whether MQ is offered or > negotiated, it just needs to read the number of queues there, so if no MQ, it > is not "not any queues", there are still 1 queue pair to be a virtio-net device, > means two queues. > > If not, how can you tell the user there are only 2 queues? The end users may > don't know this is default. They may misunderstand this as an error or > defects. > >When max_vq_pairs is not shown, it means that device didn?t expose MAX_VQ_PAIRS attribute to its guest users. (Because _MQ was not negotiated). It is not error or defect. It precisely shows what is exposed. User space will care when it wants to turn off/on _MQ feature bits and MAX_QP values. Showing max_vq_pairs of 1 even when _MQ is not negotiated, incorrectly says that max_vq_pairs is exposed to the guest, but it is not offered. So, please fix the iproute2 to not print max_vq_pairs when it is not returned by the kernel.> > We have many config space fields that depend on the feature bits and > some of them do not have any defaults. > > To keep consistency of existence of config space fields among all, we don't > want to show default like below. > > > > Please fix the iproute2 to not print max_vq_pairs when it is not returned > by the kernel. > > > >> After applying this commit, when MQ = 0, iproute2 output: > >> $vdpa dev config show vdpa0 > >> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs > >> 1 mtu 1500 > >> > >> Fixes: a64917bc2e9b (vdpa: Provide interface to read driver features) > >> Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com> > >> --- > >> drivers/vdpa/vdpa.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index > >> d76b22b2f7ae..846dd37f3549 100644 > >> --- a/drivers/vdpa/vdpa.c > >> +++ b/drivers/vdpa/vdpa.c > >> @@ -806,9 +806,10 @@ static int vdpa_dev_net_mq_config_fill(struct > >> vdpa_device *vdev, > >> u16 val_u16; > >> > >> if ((features & BIT_ULL(VIRTIO_NET_F_MQ)) == 0) > >> - return 0; > >> + val_u16 = 1; > >> + else > >> + val_u16 = __virtio16_to_cpu(true, config- > >>> max_virtqueue_pairs); > >> - val_u16 = le16_to_cpu(config->max_virtqueue_pairs); > >> return nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, > val_u16); > >> } > >> > >> @@ -842,7 +843,7 @@ static int vdpa_dev_net_config_fill(struct > >> vdpa_device *vdev, struct sk_buff *ms > >> VDPA_ATTR_PAD)) > >> return -EMSGSIZE; > >> > >> - return vdpa_dev_net_mq_config_fill(vdev, msg, features_driver, > >> &config); > >> + return vdpa_dev_net_mq_config_fill(vdev, msg, features_device, > >> +&config); > >> } > >> > >> static int > >> -- > >> 2.31.1