Re: [virtio-comment] Re: [PATCH] virtio-net: introduce admin control virtqueue

[Jason Wang <jasowang@redhat.com>]

On 2021/1/22 äå8:14, Michael S. Tsirkin wrote:
> On Fri, Jan 22, 2021 at 04:14:19AM -0500, Jason Wang wrote:
> > ----- Original Message -----
> > > On Fri, Jan 22, 2021 at 03:41:23AM -0500, Jason Wang wrote:
> > > > ----- Original Message -----
> > > > > On Fri, Jan 22, 2021 at 04:23:25PM +0800, Jason Wang wrote:
> > > > > > When implementing multiple (sub)functions virtio device like SRIOV or
> > > > > > sub-function. We're suffering from several issues:
> > > > > >
> > > > > > - There's no management interface for management function (PF) to
> > > > > >    configure features, attributes for a (sub)function
> > > > > > - Per (sub)function control virtqueue could be too expensive as the
> > > > > >    number of (sub)functions could be very large
> > > > > > - Virtualize per (sub)function control virtqueue could be very
> > > > > >    challenge as we need the support of DMA isolation at queue level
> > > > > >
> > > > > > So this patch introduces the feature of VIRTIO_NET_CTRL_ADMIN_VQ. This
> > > > > > allows the device to implement a single admin control virtqueue to
> > > > > > manage per (sub)function features and attributes.
> > > > > >
> > > > > > The idea is simple, a new function id is introduced on top of the
> > > > > > exist virtio_net_ctrl structure. This id is transport or device
> > > > > > specific to uniquely identify a (sub)function.
> > > > > >
> > > > > > With this, we get a way of using management function (PF) to configure
> > > > > > per (sub)function features and attributes. And since the admin control
> > > > > > virtqueue belongs to management function (PF), the DMA is naturally
> > > > > > isolated at (sub)function level instead of the queue level for per
> > > > > > (sub)function control vq.
> > > > > >
> > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > > > Let's specify how this works at least for PCI? What about sub-functions
> > > > > under VF level? How are these specified?
> > > > Ok, will do.
> > > >
> > > > > > ---
> > > > > >   content.tex | 23 +++++++++++++++++++++++
> > > > > >   1 file changed, 23 insertions(+)
> > > > > >
> > > > > > diff --git a/content.tex b/content.tex
> > > > > > index 620c0e2..98592a4 100644
> > > > > > --- a/content.tex
> > > > > > +++ b/content.tex
> > > > > > @@ -2940,6 +2940,9 @@ \subsection{Feature bits}\label{sec:Device Types
> > > > > > /
> > > > > > Network Device / Feature bits
> > > > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > > > > >       channel.
> > > > > >   
> > > > > > +\item[VIRTIO_NET_F_ADMIN_CTRL_VQ(56)] Admin control channel is
> > > > > > +    available.
> > > > > > +
> > > > > >   \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
> > > > > >       value and a type of calculated hash.
> > > > > >   
> > > > > > @@ -3864,6 +3867,26 @@ \subsubsection{Control
> > > > > > Virtqueue}\label{sec:Device
> > > > > > Types / Network Device / Devi
> > > > > >   do except issue a diagnostic if \field{ack} is not
> > > > > >   VIRTIO_NET_OK.
> > > > > >   
> > > > > > +\subsubsection{Admin Control Virtqueue}\label{sec:Device Types /
> > > > > > Network
> > > > > > Device / Device Operation / Control Virtqueue}
> > > > > > +
> > > > > > +When VIRTIO_NET_F_ADMIN_CTRL_VQ is negotiated, the driver can use the
> > > > > > +control virtqueue to manipulate features of individual function where
> > > > > manipulate features? You don't mean feature bits, do you?
> > > > I copy from the control virtqueue paragraph above:
> > > >
> > > > """
> > > > The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
> > > > negotiated) to send commands to manipulate various features of
> > > > the device which would not easily map into the configuration
> > > > space.
> > > > """
> > > >
> > > > Do we need to tweak that part as well?
> > > >
> > > > Thanks
> > > Maybe, but let's be a bit more specific. For example,
> > > is it legal to manipulate the MAC of a VF?
> > Right, so we can clarify that the admin control virtqueue should be
> > only used/advertised for the management function like PF. When
> > neogiated, it's always legal to manipulte the MAC of VF through it.
> >
> > > When is it legal?
> > Or do you mean some like "trust" command for iproute2 that allows VF
> > to change its own mac?
> >
> > When PF has admin cvq and VF has normal cvq, management software needs
> > to synrhonize the setting of mac address. E.g management won't set mac
> > address after VM is launched.
> >
> > Thanks
> Consider.
> VF has VIRTIO_NET_F_CTRL_MAC_ADDR. This means guest can set mac address.
> This is something PF needs ability to control. How is this done?


This requires a new command to allow/deny the mac address setting from 
the VF.


> Also, does PF allow setting guest MAC?


I think the answer is yes.


> when?


It will work as how ip set vf mac work. Usually, it should be done 
before launching the guest. Management should make sure there's no 
conflict in mac address updating.


>   when PF has
> VIRTIO_NET_F_CTRL_MAC_ADDR?


Yes, without this feature. We can't set mac address through admin 
control virtqueue.


> Do we differentiate between
> that and ability to set MAC for pf?


I think not. E.g PF should have a dedicated function_id (or virtual 
device id) like 0. Then we can set mac address of PF through admin cvq.

Thanks

>