Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
On 08/19/2015 07:54 PM, Victor Kaplansky wrote: > On Mon, Aug 17, 2015 at 10:43:46AM +0800, Jason Wang wrote: >> >> On 08/16/2015 09:42 PM, Victor Kaplansky wrote: >>> Clarify general description of the mac, status and >>> max_virtqueue_pairs fields. Specifically, the old description is >>> vague about configuration layout and fields offsets when some of >>> the fields are non valid. >>> >>> Also clarify that validity of two status bits depends on two >>> different feature flags. >>> >>> Signed-off-by: Victor Kaplansky >>> --- >>> + >>> +\item [\field{max_virtqueue_pairs}] tells the driver the maximum >>> +number of each of virtqueues (receiveq1\ldots receiveqN and >>> +transmitq1\ldots transmitqN respectively) that can be configured >>> +on the device once VIRTIO_NET_F_MQ is negotiated. >>> +\field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is >>> +set and can be read by the driver. >>> + >> >> I don't get the point that adding "can be read by the driver". Looks >> like it's hard for hypervisor to detect this? > AFAIU, if the device sets VIRTIO_NET_F_MQ, the device also sets > the value of 'max_virtqueue_pairs' even before driver negotiated > VIRTIO_NET_F_MQ. If so, the driver can read the value of > 'max_virtqueue_pairs' during negotiation and potentially this > value can even affect negotiation decision of the driver. Looks not, it's legal to negotiate VIRTIO_NET_F_MQ with max_virtqueue_pairs = 1. > > If above is correct, I'll change the description to make this > point more clear. > > Thanks, > -- Victor Driver also sets 'mac' before driver negotiated VIRTIO_NET_F_MAC, so I'm not sure this is really needed. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
On Mon, Aug 17, 2015 at 10:43:46AM +0800, Jason Wang wrote: > > > On 08/16/2015 09:42 PM, Victor Kaplansky wrote: > > Clarify general description of the mac, status and > > max_virtqueue_pairs fields. Specifically, the old description is > > vague about configuration layout and fields offsets when some of > > the fields are non valid. > > > > Also clarify that validity of two status bits depends on two > > different feature flags. > > > > Signed-off-by: Victor Kaplansky > > --- > > + > > +\item [\field{max_virtqueue_pairs}] tells the driver the maximum > > +number of each of virtqueues (receiveq1\ldots receiveqN and > > +transmitq1\ldots transmitqN respectively) that can be configured > > +on the device once VIRTIO_NET_F_MQ is negotiated. > > +\field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is > > +set and can be read by the driver. > > + > > > I don't get the point that adding "can be read by the driver". Looks > like it's hard for hypervisor to detect this? AFAIU, if the device sets VIRTIO_NET_F_MQ, the device also sets the value of 'max_virtqueue_pairs' even before driver negotiated VIRTIO_NET_F_MQ. If so, the driver can read the value of 'max_virtqueue_pairs' during negotiation and potentially this value can even affect negotiation decision of the driver. If above is correct, I'll change the description to make this point more clear. Thanks, -- Victor -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
On 08/16/2015 09:42 PM, Victor Kaplansky wrote: > Clarify general description of the mac, status and > max_virtqueue_pairs fields. Specifically, the old description is > vague about configuration layout and fields offsets when some of > the fields are non valid. > > Also clarify that validity of two status bits depends on two > different feature flags. > > Signed-off-by: Victor Kaplansky > --- > content.tex | 42 +++--- > 1 file changed, 31 insertions(+), 11 deletions(-) > > diff --git a/content.tex b/content.tex > index d989d98..342183b 100644 > --- a/content.tex > +++ b/content.tex > @@ -3115,23 +3115,14 @@ were needed. > \subsection{Device configuration layout}\label{sec:Device Types / Network > Device / Device configuration layout} > \label{sec:Device Types / Block Device / Feature bits / Device configuration > layout} > > -Three driver-read-only configuration fields are currently defined. 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. > + > +The following configuration fields are currently defined: > > \begin{lstlisting} > #define VIRTIO_NET_S_LINK_UP 1 > #define VIRTIO_NET_S_ANNOUNCE2 > \end{lstlisting} > > -The following driver-read-only field, \field{max_virtqueue_pairs} only > exists if > -VIRTIO_NET_F_MQ is set. This field specifies the maximum number > -of each of transmit and receive virtqueues (receiveq1\ldots receiveqN > -and transmitq1\ldots transmitqN respectively) that can be configured once > VIRTIO_NET_F_MQ > -is negotiated. > - > \begin{lstlisting} > struct virtio_net_config { > u8 mac[6]; > @@ -3140,6 +3131,35 @@ struct virtio_net_config { > }; > \end{lstlisting} > > +\field{mac}, \field{status}, \field{max_virtqueue_pairs} are > +driver-read-only. \field{default_mtu} can also be written > +by the driver after negotiation. > + > +\begin{description} > +\item [\field{mac}] is MAC address assigned by the device and is valid > +only if VIRTIO_NET_F_MAC is offered. > + > +\item [\field{status}] consists of two driver-read-only status bits: > + > +\begin{description} > + > +\item VIRTIO_NET_S_LINK_UP is valid if VIRTIO_NET_F_STATUS is > +offered. > + > +\item VIRTIO_NET_S_ANNOUNCE is valid if > +VIRTIO_NET_F_GUEST_ANNOUNCE is offered. > + > +\end{description} > + > +\item [\field{max_virtqueue_pairs}] tells the driver the maximum > +number of each of virtqueues (receiveq1\ldots receiveqN and > +transmitq1\ldots transmitqN respectively) that can be configured > +on the device once VIRTIO_NET_F_MQ is negotiated. > +\field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is > +set and can be read by the driver. > + Hi: I don't get the point that adding "can be read by the driver". Looks like it's hard for hypervisor to detect this? Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html