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


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