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

[Parav Pandit <parav@nvidia.com>]

> 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.

> +        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.