[PATCH v2 1/2] virtio-net: rephrase devconf fields description

2015-08-16 Thread Victor Kaplansky
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.
+
+\end{description}
+
 \devicenormative{\subsubsection}{Device configuration layout}{Device Types / 
Network Device / Device configuration layout}
 
 The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 
inclusive,
-- 
--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

2015-08-16 Thread Jason Wang


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


Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description

2015-08-19 Thread Victor Kaplansky
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

2015-08-19 Thread Jason Wang


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