On Fri, Feb 03 2023, Parav Pandit <[email protected]> wrote: > Currently some fields of the virtio_net_config structure are defined > before introducing the structure and some are defined after > introducing virtio_net_config. > Better to define the configuration layout first followed by > description of all the fields.
I see that some other devices (e.g. block) list the config layout _after_ all of the descriptions, although I think listing first and then describing is the better approach. However, in-between is the worst order, and just cleaning up this one right now makes sense. > > Device configuration fields are described in the section. Change wording > from 'listed' to 'described' as suggested in patch [1]. > > [1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00004.html > > Signed-off-by: Parav Pandit <[email protected]> > --- > device-types/net/description.tex | 39 +++++++++++++++++--------------- > 1 file changed, 21 insertions(+), 18 deletions(-) > > diff --git a/device-types/net/description.tex > b/device-types/net/description.tex > index dedd6b1..d4f598b 100644 > --- a/device-types/net/description.tex > +++ b/device-types/net/description.tex > @@ -154,11 +154,27 @@ \subsubsection{Legacy Interface: Feature > bits}\label{sec:Device Types / Network > \subsection{Device configuration layout}\label{sec:Device Types / Network > Device / Device configuration layout} > \label{sec:Device Types / Block Device / Feature bits / Device configuration > layout} > > -Device configuration fields are listed below, they are read-only for a > driver. The \field{mac} address field > -always exists (though is only valid if VIRTIO_NET_F_MAC is set), and > -\field{status} only exists if VIRTIO_NET_F_STATUS is set. Two > -read-only bits (for the driver) are currently defined for the status field: > -VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE. > +Device configuration fields are described below, they are read-only for a > driver. Maybe replace that with: "The network device uses the following device configuration layout. The fields are read-only for the driver." > + > +\begin{lstlisting} > +struct virtio_net_config { > + u8 mac[6]; > + le16 status; > + le16 max_virtqueue_pairs; > + le16 mtu; > + le32 speed; > + u8 duplex; > + u8 rss_max_key_size; > + le16 rss_max_indirection_table_length; > + le32 supported_hash_types; > +}; > +\end{lstlisting} > + > +The \field{mac} address field always exists (though is only valid > +if VIRTIO_NET_F_MAC is set), and \field{status} only exists if > +VIRTIO_NET_F_STATUS is set. Two read-only bits (for the driver) > +are currently defined for the status field: VIRTIO_NET_S_LINK_UP > +and VIRTIO_NET_S_ANNOUNCE. As you are touching this anyway, maybe break it up? "The \field{mac} address field always exists (although it is only valid if VIRTIO_NET_F_MAC is set). \field{status} only exists if VIRTIO_NET_F_STATUS is set. Two read-only bits (for the driver) are currently defined for the status field: VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE." > > \begin{lstlisting} > #define VIRTIO_NET_S_LINK_UP 1 > @@ -188,19 +204,6 @@ \subsection{Device configuration > layout}\label{sec:Device Types / Network Device > is expected to re-read these values after receiving a > configuration change notification. > > -\begin{lstlisting} > -struct virtio_net_config { > - u8 mac[6]; > - le16 status; > - le16 max_virtqueue_pairs; > - le16 mtu; > - le32 speed; > - u8 duplex; > - u8 rss_max_key_size; > - le16 rss_max_indirection_table_length; > - le32 supported_hash_types; > -}; > -\end{lstlisting} > The following field, \field{rss_max_key_size} only exists if > VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set. > It specifies the maximum supported length of RSS key in bytes. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
