Virtualization - Jul 2022 - [PATCH V3 5/6] vDPA: answer num of queue pairs = 1 to userspace when VIRTIO_NET_F_MQ == 0

> From: Michael S. Tsirkin <mst at redhat.com>
> Sent: Wednesday, July 13, 2022 1:27 AM
> 
> On Fri, Jul 01, 2022 at 10:07:59PM +0000, 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.
> >
> > 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.
> 
> Parav I read the discussion and don't get your argument. From
driver's POV
> _MQ with 1 VQ pair and !_MQ are exactly functionally equivalent.
But we are talking from user POV here.
> 
> It's true that iproute probably needs to be fixed too, to handle old
kernels.
> But iproute is not the only userspace, why not make it's life easier by
fixing
> the kernel?
Because it cannot be fixed for other config space fields which are control by
feature bits those do not have any defaults.
So better to treat all in same way from user POV.

On Tue, Jul 26, 2022 at 03:54:06PM +0000, Parav Pandit
wrote:
> 
> > From: Michael S. Tsirkin <mst at redhat.com>
> > Sent: Wednesday, July 13, 2022 1:27 AM
> > 
> > On Fri, Jul 01, 2022 at 10:07:59PM +0000, 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.
> > >
> > > 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.
> > 
> > Parav I read the discussion and don't get your argument. From
driver's POV
> > _MQ with 1 VQ pair and !_MQ are exactly functionally equivalent.
> But we are talking from user POV here.
>From spec POV there's just driver and device, user would be part of
driver here.
> > 
> > It's true that iproute probably needs to be fixed too, to handle
old kernels.
> > But iproute is not the only userspace, why not make it's life
easier by fixing
> > the kernel?
> Because it cannot be fixed for other config space fields which are control
by feature bits those do not have any defaults.
> So better to treat all in same way from user POV.
Consistency is good for sure. What are these other fields though?
Can you give examples so I understand please?

-- 
MST