Re: RE: [RFC] virtio-net: support access and control the member devices

[Xuan Zhuo <xuanzhuo@linux.alibaba.com>]

On Thu, 3 Aug 2023 10:23:32 +0000, Parav Pandit <parav@nvidia.com> wrote:
>
> > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Sent: Thursday, August 3, 2023 2:02 PM
> > To: virtio-dev@lists.oasis-open.org
> > Cc: jasowang@redhat.com; Michael S. Tsirkin <mst@redhat.com>; Parav Pandit
> > <parav@nvidia.com>; virtio-comment@lists.oasis-open.org
> > Subject: [RFC] virtio-net: support access and control the member devices
> >
> > Now we have the concept of the device group. This allows us to support sr-iov
> > and siov more conveniently.
> >
> > The following description uses pf, vf as an example. The scene with sf should be
> > similar.
> >
> > According to my understanding, the management at the level of virtqueue
> > should be handled by admin queue. And the net related management is still
> > managed by cvq of pf. So this patch works with the cvq of the pf.
> >
> Admin command is preferred
>
> > The problem I am trying to solve is when there are multiple vf virtio-net devices
> > under one pf.
> > It is necessary for administrators to query the status and information of each vf
> > and configure mac.
> This patch enables to configure also, please add it in the commit log.
>
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  device-types/net/description.tex        | 72 +++++++++++++++++++++++--
> >  device-types/net/device-conformance.tex |  1 +
> >  2 files changed, 70 insertions(+), 3 deletions(-)
> >
> > diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> > index 76585b0..1494a18 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -88,6 +88,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_ACCESS_MEMBER_DEVICE(51)] Device can access and
> > control the
> > +    member devices.
> > +
> Once admin command is used, patch will be simpler without new feature bit.
>
> >  \item[VIRTIO_NET_F_HASH_TUNNEL(51)] Device supports inner header hash
> > for encapsulated packets.
> >
> >  \item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue
> > notification coalescing.
> > @@ -1156,6 +1159,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> > Types / Network Device / Devi
> >          u8 command;
> >          u8 command-specific-data[];
> >          u8 ack;
> > +        u8 command-specific-data-reply[];
> >  };
> >
> >  /* ack values */
> > @@ -1164,9 +1168,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> > Types / Network Device / Devi  \end{lstlisting}
> >
> >  The \field{class}, \field{command} and command-specific-data are set by the -
> > driver, and the device sets the \field{ack} byte. There is little it can -do except
> > issue a diagnostic if \field{ack} is not -VIRTIO_NET_OK.
> > +driver, and the device sets the \field{ack} byte and optionally
> > +\field{command-specific-data-reply}. There is little the driver can do
> > +except issue a diagnostic if \field{ack} is not VIRTIO_NET_OK.
> > +
> > +The command VIRTIO_NET_CTRL_MEMBER_GET_INFO contains
> > \field{command-specific-data-reply}.
> >
> >  \paragraph{Packet Receive Filtering}\label{sec:Device Types / Network Device
> > / Device Operation / Control Virtqueue / Packet Receive Filtering}
> > \label{sec:Device Types / Network Device / Device Operation / Control
> > Virtqueue / Setting Promiscuous Mode}%old label for latexdiff @@ -1805,6
> > +1811,66 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types /
> > Network Device / Devi
> >
> >  Upon reset, a device MUST initialize all coalescing parameters to 0.
> >
> > +\paragraph{Member Device}\label{sec:Device Types / Network Device /
> > +Device Operation / Control Virtqueue / Member Info}
> > +
> > +If a deivce is the own deivce inside one device group and the feature
> > +VIRTIO_NET_F_ACCESS_MEMBER_DEVICE is negotiated, the own driver can
> > +obtain the info and change the configuration of the member devices of
> > +this device group.
> > +
> > +\begin{lstlisting}
> > +struct virtio_net_ctrl_member {
> > +        le64 group_member_id;
> > +};
> > +
> > +struct virtio_net_ctrl_member_info_reply {
> We need a bitmap to indicate which fields are valid.
> We did a nice work of it in vdpa tool to describe both, the fields and bitmap.
>
> Also, when you change to admin command, please change suffix to from _reply to _result to align to its terminology.


OK.


>
> > +        u8 mac[6];
> > +        le16 status;
> > +        le16 max_virtqueue_pairs;
> > +        le16 mtu;
> > +        le32 speed;
> > +        u8 duplex;
>
> We should above split constant config space fields and below run time fields meant for debug purpose.
> We are working on the member device migration proposal and below fields belong to "device context" should be available through such separate command.
> But if you do it before, please cover the whole device state and we will be able to reuse for multiple use-cases.


I am not sure I got fully.

I will try.

Thanks.