OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [virtio-dev] [PATCH] network device: xmit/receive cleanup




On Thu, Apr 30, 2015 at 4:02 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
On Thu, Apr 30, 2015 at 03:45:18PM +0800, Jason Wang wrote:
On Thu, Apr 30, 2015 at 3:22 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
 >Fix up multiple issues in xmit/receive sections:
 >- drop MAY/MUST/SHOULD outside normative statements
 >- spell out conformance requirements for both drivers and
 >  devices, for xmit and receive paths
 >- document the missing VIRTIO_NET_HDR_F_DATA_VALID
 >- document handling of unrecognized flag bits so we can extend
 >  flags in the future, similar to VIRTIO_NET_HDR_F_DATA_VALID
 >
 >This fixes VIRTIO-123
 >
 >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
 >---
 > conformance.tex |   4 +-
 > content.tex     | 193
 >++++++++++++++++++++++++++++++++++++++++++++++++++------
 > 2 files changed, 176 insertions(+), 21 deletions(-)
 >
 >diff --git a/conformance.tex b/conformance.tex
 >index 279b5cb..0ba2240 100644
 >--- a/conformance.tex
 >+++ b/conformance.tex
>@@ -92,6 +92,7 @@ A network driver MUST conform to the following normative
 >statements:
 > \item \ref{drivernormative:Device Types / Network Device / Device
 >configuration layout}
 > \item \ref{drivernormative:Device Types / Network Device / Device
 >Operation / Packet Transmission}
 > \item \ref{drivernormative:Device Types / Network Device / Device
 >Operation / Setting Up Receive Buffers}
 >+\item \ref{drivernormative:Device Types / Network Device / Device
 >Operation / Processing of Incoming Packets}
 > \item \ref{drivernormative:Device Types / Network Device / Device
 >Operation / Control Virtqueue / Setting MAC Address Filtering}
 > \item \ref{drivernormative:Device Types / Network Device / Device
 >Operation / Control Virtqueue / Gratuitous Packet Sending}
 > \item \ref{drivernormative:Device Types / Network Device / Device
>Operation / Control Virtqueue / Automatic receive steering in multiqueue
 >mode}
 >@@ -208,8 +209,9 @@ A network device MUST conform to the following
 >normative statements:
 > \begin{itemize}
 > \item \ref{devicenormative:Device Types / Network Device / Device
 >configuration layout}
 >+\item \ref{devicenormative:Device Types / Network Device / Device
 >Operation / Packet Transmission}
 > \item \ref{devicenormative:Device Types / Network Device / Device
 >Operation / Setting Up Receive Buffers}
 >-\item \ref{devicenormative:Device Types / Network Device / Device
 >Operation / Processing of Packets}
 >+\item \ref{devicenormative:Device Types / Network Device / Device
 >Operation / Processing of Incoming Packets}
 > \item \ref{devicenormative:Device Types / Network Device / Device
 >Operation / Control Virtqueue / Setting MAC Address Filtering}
 > \item \ref{devicenormative:Device Types / Network Device / Device
 >Operation / Control Virtqueue / Gratuitous Packet Sending}
 > \item \ref{devicenormative:Device Types / Network Device / Device
>Operation / Control Virtqueue / Automatic receive steering in multiqueue
 >mode}
 >diff --git a/content.tex b/content.tex
 >index 7bca4fa..b287f5f 100644
 >--- a/content.tex
 >+++ b/content.tex
 >@@ -3215,7 +3215,12 @@ if both guests are amenable.}
 >   the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
 >   VIRTIO_NET_F_GUEST_UFO and VIRTIO_NET_F_GUEST_ECN are the input
 >   equivalents of the features described above.
>- See \ref{sec:Device Types / Network Device / Device Operation / Setting >Up Receive Buffers}~\nameref{sec:Device Types / Network Device / Device
 >Operation / Setting Up Receive Buffers} and \ref{sec:Device Types /
 >Network Device / Device Operation / Processing of
>Packets}~\nameref{sec:Device Types / Network Device / Device Operation /
 >Processing of Packets} below.
 >+  See \ref{sec:Device Types / Network Device / Device Operation /
 >+Setting Up Receive Buffers}~\nameref{sec:Device Types / Network
 >+Device / Device Operation / Setting Up Receive Buffers} and
 >+\ref{sec:Device Types / Network Device / Device Operation /
 >+Processing of Incoming Packets}~\nameref{sec:Device Types /
>+Network Device / Device Operation / Processing of Incoming Packets}
 >below.
 > \end{enumerate}
> A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore >@@ -3274,10 +3279,10 @@ Transmitting a single packet is simple, but varies
 >depending on
 > the different features the driver negotiated.
 > \begin{enumerate}
>-\item The driver MAY send a completely checksummed packet. In this case, >+\item The driver can send a completely checksummed packet. In this case,
 >   \field{flags} will be zero, and \field{gso_type} will be
 >VIRTIO_NET_HDR_GSO_NONE.
 >-\item If the driver negotiated VIRTIO_NET_F_CSUM, it MAY skip
 >+\item If the driver negotiated VIRTIO_NET_F_CSUM, it can skip
 >   checksumming the packet:
 >   \begin{itemize}
 >   \item \field{flags} has the VIRTIO_NET_HDR_F_NEEDS_CSUM set,
 >@@ -3336,18 +3341,83 @@ specifically in the protocol.}.
 >   \drivernormative{\paragraph}{Packet Transmission}{Device Types /
 >Network Device / Device Operation / Packet Transmission}
> -If a driver has not negotiated VIRTIO_NET_F_CSUM, \field{flags} MUST be
 >zero and
 >-the packet MUST be fully checksummed.
 >-
 > The driver MUST set \field{num_buffers} to zero.
> -A driver SHOULD NOT send TCP packets requiring segmentation offload
 >which have the Explicit Congestion Notification bit set, unless the
 >VIRTIO_NET_F_HOST_ECN feature is
>-negotiated\footnote{This is a common restriction in real, older network
 >cards.}, in
 >-which case it MUST set the VIRTIO_NET_HDR_GSO_ECN bit in
 >\field{gso_type}.
 >+If VIRTIO_NET_F_CSUM is not negotiated, the driver MUST set
 >+\field{flags} to zero and SHOULD supply a fully checksummed
 >+packet to the device.
 >+
 >+If VIRTIO_NET_F_HOST_TSO4 is negotiated, the driver MAY set
 >+\field{gso_type} to VIRTIO_NET_HDR_GSO_TCPV4 to request TCPv4
 >+segmentation, otherwise the driver MUST NOT set
 >+\field{gso_type} to VIRTIO_NET_HDR_GSO_TCPV4.
 >+
 >+If VIRTIO_NET_F_HOST_TSO6 is negotiated, the driver MAY set
 >+\field{gso_type} to VIRTIO_NET_HDR_GSO_TCPV6 to request TCPv6
 >+segmentation, otherwise the driver MUST NOT set
 >+\field{gso_type} to VIRTIO_NET_HDR_GSO_TCPV6.
 >+
 >+If VIRTIO_NET_F_HOST_UFO is negotiated, the driver MAY set
 >+\field{gso_type} to VIRTIO_NET_HDR_GSO_UDP to request UDP
 >+segmentation, otherwise the driver MUST NOT set
 >+\field{gso_type} to VIRTIO_NET_HDR_GSO_UDP.
 >+
 >+The driver SHOULD NOT send to the device TCP packets requiring
 >segmentation offload
>+which have the Explicit Congestion Notification bit set, unless the
 >+VIRTIO_NET_F_HOST_ECN feature is negotiated, in which case the
 >+driver MUST set the VIRTIO_NET_HDR_GSO_ECN bit in
 >+\field{gso_type}.
 >+
 >+If the VIRTIO_NET_F_CSUM feature has been negotiated, the
 >+driver MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
 >+\field{flags}, if so:
 >+\begin{enumerate}
 >+\item the driver MUST validate the packet checksum at
 >+	offset \field{csum_offset} from \field{csum_start} as well as all
 >+	preceding offsets;
 >+\item the driver MUST set the packet checksum stored in the
 >+	buffer to the TCP/UDP pseudo header;
 >+\item the driver MUST set \field{csum_start} and
 >+	\field{csum_offset} such that calculating a ones'
 >+	complement checksum from \field{csum_start} up until the end of
 >+	the packet and storing the result at offset \field{csum_offset}
 >+	from  \field{csum_start} will result in a fully checksummed
 >+	packet;
 >+\end{enumerate}
 >+
 >+If none of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have
 >+been negotiated, the driver MUST set \field{gso_type} to
 >+VIRTIO_NET_HDR_GSO_NONE.
 >+
 >+If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, then
 >+the driver MUST also set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
 >+\field{flags} and MUST set \field{gso_size} to indicate the
 >+desired MSS.
 >+
 >+If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have
 >+been negotiated, the driver SHOULD set \field{hdr_len} to a value
 >+not less than the length of the headers, including the transport
 >+header.
 >+
 >+The driver MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID bit in
 >+\field{flags}.
Maybe we should allow this. Consider one example is guest is forwarding a
 packet with this to another interface.

Well we don't use this now, as neither linux nor windows can use this
at the moment. I'm wary of features that are there just in case. I did add text specifying that devices must ignore bits they don't recognize,
so we'll be able to lift the restriction if we find a use for it.
What do you think?

Make sense.
Thanks



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]