Re: [virtio-dev] [PATCH v12] virtio-net: support device stats

2023-07-12 Thread Jason Wang
On Wed, Jul 12, 2023 at 5:19 PM Xuan Zhuo  wrote:
>
> On Wed, 12 Jul 2023 16:08:48 +0800, Jason Wang  wrote:
> > On Mon, Jul 10, 2023 at 3:32 PM Xuan Zhuo 
> > wrote:
> >
> > > hi, guys
> > >
> > > After a lot of internal discussions, I removed some unimportant counters.
> > > Based
> > > on this v12 patch I am repling to, most of the requirements have been met.
> > >
> > > The patch link
> > > https://lore.kernel.org/all/20220315032402.6088-1-xuanz...@linux.alibaba.com/
> > >
> > > We still have these counters, let's see if they can be standardized.
> > >
> > > ## limit
> > >
> > > * tx_bps_limit_drops
> > > * tx_pps_limit_drops
> > > * rx_bps_limit_drops
> > > * rx_pps_limit_drops
> > >
> > > In a cloud scenario, multiple users purchase different VMs, and these VMs
> > > share
> > > the capabilities of the same host. In order to ensure that each VM will 
> > > not
> > > affect others, the network card(virtio-net) capability of each VM is
> > > limited.
> > > These users purchase VMs, this limit has already been determined.
> > >
> >
> > This seems a feature beyond the counters but QOS. I think we virtio spec
> > need to support QOS before we can discuss any QOS related counters.
> >
> >
> > >
> > > So if the network card traffic of a vm exceeds the upper limit, packet
> > > loss will
> > > occur. It is necessary for us to count these packet losses. And the device
> > > should expose to the user.
> > >
> > > ## session
> > >
> > > * tx_session_full_drops The number of packet drops in the sending
> > > direction when
> > > the session is full
> > > * rx_session_full_drops The number of packets lost when the session is
> > > full in the receiving direction
> > >
> > > Our dpu supports tcp connection tracking, but there is an upper limit to
> > > the
> > > number of connections, and if it exceeds, packet loss will also occur.
> > >
> >
> > Similarly, if connect tracking offload is supported, it needs to be
> > implemented in the spec first then we can have related counters.
> >
> >
> > >
> > > ## ACL
> > >
> > > * tx_acl_drops ACL packet loss in the sending direction
> > > * rx_acl_drops The number of ACL packets lost in the receiving direction
> > >
> > > In our cloud service, for network cards, users are allowed to configure
> > > security
> > > rules,such as which IPs can access which ports of this machine.
> > >
> >
> > Same as above, ACL should be supported by the spec first then the counters.
> >
> > In conclusion, the features must be self contained. Otherwise you are doing
> > vDPA actually and those counters need to be accessed in a vendor specific
> > way.
>
> Yes that's the point, as we've discussed here.
>
> http://lore.kernel.org/all/1686550355.2503424-1-xuanz...@linux.alibaba.com
>
> There are many similar counters. There may not be many usage scenarios, so I
> didn't list them here.
>
> Our acl, seesion, and limit are not configured by the user at the driver 
> layer,
> and I don't think these should be configured at the driver.

Sounds like something related to OPI. But if the features were not
configured by driver, any reason to let it be noticeable by it?

Exposing things like tx_acl_drops may have security implications,
enabling software to probe the ACL rules?

Thanks

> So at least as
> far as our usage scenarios are concerned, these are some functions that have
> nothing to do with virtio spec.
>
> So what I want most is that the virto spec supports a vendor channel.
>
> Of course, I am ok with Parav's abstraction of "limit".
>
> Some implementations of txq are lossy and some are not creating 
> backpressure/flow control to driver so driver can rate limit it naturally.
> So a tx packet drop counter is needed to cover the lossy 
> implementations which is abstract enough regardless of reason on why device 
> dropped it.
> A more granular counter becomes vendor specific that we can possibly 
> avoid or place under different command.
>
> But acl and session may not be very good to abstract.
>
> Thanks.
>
>
> >
> > Thanks
> >
> >
> > >
> > >
> > > Thanks
> > >
> > >
> > > On Tue, 15 Mar 2022 11:24:02 +0800, Xuan Zhuo 
> > > wrote:
> > > > This patch allows the driver to obtain some statistics from the device.
> > > >
> > > > In the back-end implementation, we can count a lot of such information,
> > > > which can be used for debugging and judging the running status of the
> > > > back-end. We hope to directly display it to the user through ethtool.
> > > >
> > > > To get stats atomically, try to get stats for all queue pairs in one
> > > > command.
> > > >
> > > > Signed-off-by: Xuan Zhuo 
> > > > Suggested-by: Michael S. Tsirkin 
> > > > ---
> > > >  conformance.tex |   2 +
> > > >  content.tex | 406 +++-
> > > >  2 files changed, 405 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/conformance.tex b/conformance.tex
> > > > index 42f8537..c67f877 100644
> > > > --- a/conformance.tex
> 

[virtio-dev] RE: [PATCH 3/4] virtio-net: Use table to describe inner hash to rfc mapping

2023-07-12 Thread Parav Pandit



> From: Michael S. Tsirkin 
> Sent: Wednesday, July 12, 2023 6:41 PM
> Hmm. escapechar did not work then?

 Forgot to mention, I tried it, it didn't work.

> 
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Inner header hash type & Value & Reference \\ \hline \hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784 & (1 << 0) &
> > +\hyperref[intro:rfc2784]{RFC2784} \\ \hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890 & (1 << 1) &
> > +\hyperref[intro:rfc2784]{RFC2784} \\ \hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676  & (1 << 2) &
> > +\hyperref[intro:rfc7676]{RFC7676} \\ \hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP   & (1 << 3) &
> \hyperref[intro:rfc8086]{GRE in UDP} \\
> > +\hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN & (1 << 4) &
> \hyperref[intro:vxlan]{VXLAN} \\
> > +\hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE & (1 << 5) &
> > +\hyperref[intro:vxlan gpe]{VXLAN GPE} \\ \hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE& (1 << 6) &
> \hyperref[intro:geneve]{GENEVE} \\
> > +\hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP  & (1 << 7) &
> \hyperref[intro:ipip]{IPIP} \\
> > +\hline
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE & (1 << 8) &
> \hyperref[intro:nvgre]{NVGRE} \\
> > +\hline
> > +\end{tabular}
> >
> >  \subparagraph{Advice}
> >  Example uses of the inner header hash:
> > --
> > 2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [PATCH 3/4] virtio-net: Use table to describe inner hash to rfc mapping

2023-07-12 Thread Michael S. Tsirkin
On Thu, Jul 13, 2023 at 01:24:04AM +0300, Parav Pandit wrote:
> hyperlinks inside the C comments do not work well.
> Until we find out a way to represent it, lets present this in table
> form.
> 
> Signed-off-by: Parav Pandit 
> ---
>  device-types/net/description.tex | 35 ++--
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/device-types/net/description.tex 
> b/device-types/net/description.tex
> index 53c811f..68311ab 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1065,17 +1065,30 @@ \subsubsection{Processing of Incoming 
> Packets}\label{sec:Device Types / Network
>  Hash calculation for incoming packets / Encapsulation types 
> supported/enabled for inner header hash}
>  
>  Encapsulation types applicable for inner header hash:
> -\begin{lstlisting}
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
> \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
> \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
> \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
> \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
> \hyperref[intro:vxlan]{[VXLAN]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
> \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
> \hyperref[intro:geneve]{[GENEVE]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
> \hyperref[intro:ipip]{[IPIP]} */
> -#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
> \hyperref[intro:nvgre]{[NVGRE]} */
> -\end{lstlisting}

Hmm. escapechar did not work then?

> +
> +\begin{tabular}{|l|l|l|}
> +\hline
> +Inner header hash type & Value & Reference \\
> +\hline \hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784 & (1 << 0) & 
> \hyperref[intro:rfc2784]{RFC2784} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890 & (1 << 1) & 
> \hyperref[intro:rfc2784]{RFC2784} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676  & (1 << 2) & 
> \hyperref[intro:rfc7676]{RFC7676} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP   & (1 << 3) & 
> \hyperref[intro:rfc8086]{GRE in UDP} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN & (1 << 4) & 
> \hyperref[intro:vxlan]{VXLAN} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE & (1 << 5) & \hyperref[intro:vxlan 
> gpe]{VXLAN GPE} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE& (1 << 6) & 
> \hyperref[intro:geneve]{GENEVE} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP  & (1 << 7) & 
> \hyperref[intro:ipip]{IPIP} \\
> +\hline
> +VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE & (1 << 8) & 
> \hyperref[intro:nvgre]{NVGRE} \\
> +\hline
> +\end{tabular}
>  
>  \subparagraph{Advice}
>  Example uses of the inner header hash:
> -- 
> 2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] RE: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Parav Pandit

> From: Michael S. Tsirkin 
> Sent: Wednesday, July 12, 2023 6:31 PM

> There's no need to use undescores here. Let's just switch to a dash and be 
> done
> with it. E.g. GRE-rfc2784
To keep things like existing references and simple I removed the extra GRE- 
prefix too.
Please review the fixes at [1]

[1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00172.html


[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Michael S. Tsirkin
On Wed, Jul 12, 2023 at 04:52:16PM +, Parav Pandit wrote:
> 
> 
> > From: Cornelia Huck 
> > Sent: Wednesday, July 12, 2023 11:23 AM
> > 
> > On Wed, Jul 12 2023, Heng Qi  wrote:
> > 
> > > 在 2023/7/12 下午9:25, Cornelia Huck 写道:
> > >> ...while the changes above give me a fine pdf, the html generated is
> > >> broken. The problem seems to originate in the normative references
> > >> that are added in introduction.tex, but I don't see where it goes astray.
> > >>
> > >> [401] [402] [403] [404] (./virtio-v1.2-cs01.aux ! Missing \endcsname
> > >> inserted.
> > >> 
> > >> \unhbox
> > >> l.49 ...ction}Normative References}}{section.1}{}}
> > >>
> > >> ?
> > >> ! Emergency stop.
> > >> 
> > >> \unhbox
> > >> l.49 ...ction}Normative References}}{section.1}{}}
> > >>
> > >> (makehtml.sh on master seems to work ok, so it's something in this
> > >> patch...)
> > >>
> > >> Does anyone else manage to spot the problem?
> > >
> > > The problem is that underline cannot be added directly in \textbf, we
> > > need a little modification:
> > >
> > > \textbf{[GRE_rfc2784]} --> \textbf{[GRE\_rfc2784]}
> > > \textbf{[GRE_rfc2890]} --> \textbf{[GRE\_rfc2890]}
> > > \textbf{[GRE_rfc7676]} --> \textbf{[GRE\_rfc7676]}
> > >
> > > After this modification, I can successfully compile pdf on overleaf,
> > > please try this.
> > 
> > I'm afraid that's not the problem (I had tried this and similar changes 
> > without
> > success); the pdf generation is already fine, but html still fails with 
> > this change
> > (or similar).
> > 
> > [Trying on a standard Fedora 37 with a full texlive install...]
> 
> I will try in 2 hours.

There's no need to use undescores here. Let's just switch to a dash and
be done with it. E.g. GRE-rfc2784


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] RE: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Parav Pandit
Hi Cornelia, Michael,

> From: Parav Pandit 
> Sent: Wednesday, July 12, 2023 12:52 PM

> > I'm afraid that's not the problem (I had tried this and similar
> > changes without success); the pdf generation is already fine, but html
> > still fails with this change (or similar).
> >
> > [Trying on a standard Fedora 37 with a full texlive install...]
> 
> I will try in 2 hours.

Please see the fixes for these at [1].
I verified the pdf and html generation and links with it.
Patch 1 to 3 seems to be must.
Didn’t analyze/debug errors, but most seems logical following what rest of the 
spec is doing.

Please review.
[1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00171.html


[virtio-dev] [PATCH 3/4] virtio-net: Use table to describe inner hash to rfc mapping

2023-07-12 Thread Parav Pandit
hyperlinks inside the C comments do not work well.
Until we find out a way to represent it, lets present this in table
form.

Signed-off-by: Parav Pandit 
---
 device-types/net/description.tex | 35 ++--
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 53c811f..68311ab 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1065,17 +1065,30 @@ \subsubsection{Processing of Incoming 
Packets}\label{sec:Device Types / Network
 Hash calculation for incoming packets / Encapsulation types supported/enabled 
for inner header hash}
 
 Encapsulation types applicable for inner header hash:
-\begin{lstlisting}
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
\hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
\hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
\hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
\hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
\hyperref[intro:vxlan]{[VXLAN]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
\hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
\hyperref[intro:geneve]{[GENEVE]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
\hyperref[intro:ipip]{[IPIP]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
\hyperref[intro:nvgre]{[NVGRE]} */
-\end{lstlisting}
+
+\begin{tabular}{|l|l|l|}
+\hline
+Inner header hash type & Value & Reference \\
+\hline \hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784 & (1 << 0) & 
\hyperref[intro:rfc2784]{RFC2784} \\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890 & (1 << 1) & 
\hyperref[intro:rfc2784]{RFC2784} \\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676  & (1 << 2) & 
\hyperref[intro:rfc7676]{RFC7676} \\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP   & (1 << 3) & 
\hyperref[intro:rfc8086]{GRE in UDP} \\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN & (1 << 4) & 
\hyperref[intro:vxlan]{VXLAN} \\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE & (1 << 5) & \hyperref[intro:vxlan 
gpe]{VXLAN GPE} \\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE& (1 << 6) & 
\hyperref[intro:geneve]{GENEVE} \\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP  & (1 << 7) & \hyperref[intro:ipip]{IPIP} 
\\
+\hline
+VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE & (1 << 8) & 
\hyperref[intro:nvgre]{NVGRE} \\
+\hline
+\end{tabular}
 
 \subparagraph{Advice}
 Example uses of the inner header hash:
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] [PATCH 4/4] virtio-net: Use note instead of advice

2023-07-12 Thread Parav Pandit
Section like "Advice" is not the current practice. Lets capture such
advice as notes.

Signed-off-by: Parav Pandit 
---
 device-types/net/description.tex | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 68311ab..a8dff97 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1090,7 +1090,7 @@ \subsubsection{Processing of Incoming 
Packets}\label{sec:Device Types / Network
 \hline
 \end{tabular}
 
-\subparagraph{Advice}
+\begin{note}
 Example uses of the inner header hash:
 \begin{itemize}
 \item Legacy tunneling protocols, lacking the outer header entropy, can use 
RSS with the inner header hash to
@@ -1105,6 +1105,8 @@ \subsubsection{Processing of Incoming 
Packets}\label{sec:Device Types / Network
 Besides disabling the inner header hash, mitigations would depend on how the 
hash is used. When the hash
 use is limited to the RSS queue selection, the inner header hash may have 
quality of service (QoS) limitations.
 
+\end{note}
+
 \devicenormative{\subparagraph}{Inner Header Hash}{Device Types / Network 
Device / Device Operation / Control Virtqueue / Inner Header Hash}
 
 If the (outer) header of the received packet does not match any encapsulation 
types enabled
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] [PATCH 2/4] virtio-net: Avoid hyphen and extra braces

2023-07-12 Thread Parav Pandit
Avoid hyphen and replace it with white space like rest of the entries.
Also avoid unnecessary braces.
Name RFC as just RFC without special prefix about it.

This likely resolves the html generation errors.

Signed-off-by: Parav Pandit 
---
 device-types/net/description.tex |  2 +-
 introduction.tex | 15 +++
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 6fd4a20..53c811f 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -904,7 +904,7 @@ \subsubsection{Processing of Incoming 
Packets}\label{sec:Device Types / Network
 \end{itemize}
 
 The per-packet hash calculation can depend on the IP packet type. See
-\hyperref[intro:IP]{[IP]}, \hyperref[intro:UDP]{[UDP]} and 
\hyperref[intro:TCP]{[TCP]}.
+\hyperref[intro:IP]{IP}, \hyperref[intro:UDP]{UDP} and 
\hyperref[intro:TCP]{TCP}.
 
 \subparagraph{Supported/enabled hash types}
 \label{sec:Device Types / Network Device / Device Operation / Processing of 
Incoming Packets / Hash calculation for incoming packets / Supported/enabled 
hash types}
diff --git a/introduction.tex b/introduction.tex
index 81f07a4..028ec17 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -101,26 +101,25 @@ \section{Normative References}\label{sec:Normative 
References}
\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
 Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve 
Cryptography'', Version 1.0, September 2000.
\newline\url{https://www.secg.org/sec1-v2.pdf}\\
-
-   \phantomsection\label{intro:gre_rfc2784}\textbf{[GRE_rfc2784]} &
+   \phantomsection\label{intro:rfc2784}\textbf{[RFC2784]} &
 Generic Routing Encapsulation. This protocol is only specified for IPv4 
and used as either the payload or delivery protocol.
\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
-   \phantomsection\label{intro:gre_rfc2890}\textbf{[GRE_rfc2890]} &
-Key and Sequence Number Extensions to GRE \ref{intro:gre_rfc2784}. This 
protocol describes extensions by which two fields, Key and
-Sequence Number, can be optionally carried in the GRE Header 
\ref{intro:gre_rfc2784}.
+   \phantomsection\label{intro:rfc2890}\textbf{[RFC2890]} &
+Key and Sequence Number Extensions to GRE \ref{intro:rfc2784}. This 
protocol describes extensions by which two fields, Key and
+Sequence Number, can be optionally carried in the GRE Header 
\ref{intro:rfc2784}.
\newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
-   \phantomsection\label{intro:gre_rfc7676}\textbf{[GRE_rfc7676]} &
+   \phantomsection\label{intro:rfc7676}\textbf{[RFC7676]} &
 IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is 
specified for IPv6 and used as either the payload or
 delivery protocol. Note that this does not change the GRE header format or 
any behaviors specified by RFC 2784 or RFC 2890.
\newline\url{https://datatracker.ietf.org/doc/rfc7676/}\\
-   \phantomsection\label{intro:gre_in_udp_rfc8086}\textbf{[GRE-in-UDP]} &
+   \phantomsection\label{intro:rfc8086}\textbf{[GRE in UDP]} &
 GRE-in-UDP Encapsulation. This specifies a method of encapsulating network 
protocol packets within GRE and UDP headers.
 This protocol is specified for IPv4 and IPv6, and used as either the 
payload or delivery protocol.
\newline\url{https://www.rfc-editor.org/rfc/rfc8086}\\
\phantomsection\label{intro:vxlan}\textbf{[VXLAN]} &
 Virtual eXtensible Local Area Network.
\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
-   \phantomsection\label{intro:vxlan-gpe}\textbf{[VXLAN-GPE]} &
+   \phantomsection\label{intro:vxlan gpe}\textbf{[VXLAN GPE]} &
 Generic Protocol Extension for VXLAN. This protocol describes extending 
Virtual eXtensible Local Area Network (VXLAN) via changes to the VXLAN header.

\newline\url{https://www.ietf.org/archive/id/draft-ietf-nvo3-vxlan-gpe-12.txt}\\
\phantomsection\label{intro:geneve}\textbf{[GENEVE]} &
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] [PATCH 0/4] Short document fixes to inner hash feature

2023-07-12 Thread Parav Pandit
This short patches fixes the editing errors that stops the pdf and html
generation.

These 3 fixes are for the patch [1] for the github issue [2].

[1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00024.html
[2] https://github.com/oasis-tcs/virtio-spec/issues/173

Patch summary:
patch-1 place C code under listing
patch-2 avoid hyphen and extra braces
patch-3 use table as hyperlink do not work well in C code listing
patch-4 refer 'advice' as 'note'

Patch 1 to 3 appears to be must in the testing.
Patch 4 is not a fix and can be done later if it requires discussion.

Parav Pandit (4):
  virtio-net: Place C code under listing
  virtio-net: Avoid hyphen and extra braces
  virtio-net: Use table to describe inner hash to rfc mapping
  virtio-net: Use note instead of advice

 device-types/net/description.tex | 45 ++--
 introduction.tex | 15 +--
 2 files changed, 38 insertions(+), 22 deletions(-)

-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] [PATCH 1/4] virtio-net: Place C code under listing

2023-07-12 Thread Parav Pandit
With extra white space for the define, pdf generation fails.
Also place the C code under listing.

Signed-off-by: Parav Pandit 
---
 device-types/net/description.tex | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 206020d..6fd4a20 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1024,12 +1024,14 @@ \subsubsection{Processing of Incoming 
Packets}\label{sec:Device Types / Network
 If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the 
command
 VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner 
header hash.
 
+\begin{lstlisting}
 struct virtnet_hash_tunnel {
 le32 enabled_tunnel_types;
 };
 
 #define VIRTIO_NET_CTRL_HASH_TUNNEL 7
- #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
+#define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
+\end{lstlisting}
 
 Field \field{enabled_tunnel_types} contains the bitmask of encapsulation types 
enabled for inner header hash.
 See \ref{sec:Device Types / Network Device / Device Operation / Processing of 
Incoming Packets /
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] RE: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Parav Pandit


> From: Cornelia Huck 
> Sent: Wednesday, July 12, 2023 11:23 AM
> 
> On Wed, Jul 12 2023, Heng Qi  wrote:
> 
> > 在 2023/7/12 下午9:25, Cornelia Huck 写道:
> >> ...while the changes above give me a fine pdf, the html generated is
> >> broken. The problem seems to originate in the normative references
> >> that are added in introduction.tex, but I don't see where it goes astray.
> >>
> >> [401] [402] [403] [404] (./virtio-v1.2-cs01.aux ! Missing \endcsname
> >> inserted.
> >> 
> >> \unhbox
> >> l.49 ...ction}Normative References}}{section.1}{}}
> >>
> >> ?
> >> ! Emergency stop.
> >> 
> >> \unhbox
> >> l.49 ...ction}Normative References}}{section.1}{}}
> >>
> >> (makehtml.sh on master seems to work ok, so it's something in this
> >> patch...)
> >>
> >> Does anyone else manage to spot the problem?
> >
> > The problem is that underline cannot be added directly in \textbf, we
> > need a little modification:
> >
> > \textbf{[GRE_rfc2784]} --> \textbf{[GRE\_rfc2784]}
> > \textbf{[GRE_rfc2890]} --> \textbf{[GRE\_rfc2890]}
> > \textbf{[GRE_rfc7676]} --> \textbf{[GRE\_rfc7676]}
> >
> > After this modification, I can successfully compile pdf on overleaf,
> > please try this.
> 
> I'm afraid that's not the problem (I had tried this and similar changes 
> without
> success); the pdf generation is already fine, but html still fails with this 
> change
> (or similar).
> 
> [Trying on a standard Fedora 37 with a full texlive install...]

I will try in 2 hours.


[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Cornelia Huck
On Wed, Jul 12 2023, Heng Qi  wrote:

> 在 2023/7/12 下午9:25, Cornelia Huck 写道:
>> ...while the changes above give me a fine pdf, the html generated is
>> broken. The problem seems to originate in the normative references that
>> are added in introduction.tex, but I don't see where it goes astray.
>>
>> [401] [402] [403] [404] (./virtio-v1.2-cs01.aux
>> ! Missing \endcsname inserted.
>> 
>> \unhbox
>> l.49 ...ction}Normative References}}{section.1}{}}
>>
>> ?
>> ! Emergency stop.
>> 
>> \unhbox
>> l.49 ...ction}Normative References}}{section.1}{}}
>>
>> (makehtml.sh on master seems to work ok, so it's something in this
>> patch...)
>>
>> Does anyone else manage to spot the problem?
>
> The problem is that underline cannot be added directly in \textbf, we 
> need a little modification:
>
> \textbf{[GRE_rfc2784]} --> \textbf{[GRE\_rfc2784]}
> \textbf{[GRE_rfc2890]} --> \textbf{[GRE\_rfc2890]}
> \textbf{[GRE_rfc7676]} --> \textbf{[GRE\_rfc7676]}
>
> After this modification, I can successfully compile pdf on overleaf, 
> please try this.

I'm afraid that's not the problem (I had tried this and similar changes
without success); the pdf generation is already fine, but html still
fails with this change (or similar).

[Trying on a standard Fedora 37 with a full texlive install...]


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Cornelia Huck
On Wed, Jul 12 2023, Heng Qi  wrote:

> 在 2023/7/12 下午8:42, Cornelia Huck 写道:
>> On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:
>>
>>> On Wed, Jul 12, 2023 at 02:22:26PM +0200, Cornelia Huck wrote:
 On Mon, Jul 03 2023, Heng Qi  wrote:

 (...)

> +\paragraph{Inner Header Hash}
> +\label{sec:Device Types / Network Device / Device Operation / Processing 
> of Incoming Packets / Inner Header Hash}
> +
> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the 
> command
> +VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the 
> inner header hash.
> +
> +struct virtnet_hash_tunnel {
> +le32 enabled_tunnel_types;
> +};
> +
> +#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
> + #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
 This needs to be wrapped in \begin{lstlisting}..\end{lstlisting}, can do
 so when applying.

 (...)

> +Encapsulation types applicable for inner header hash:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
> \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
> \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
> \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
> \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
> \hyperref[intro:vxlan]{[VXLAN]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
> \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
> \hyperref[intro:geneve]{[GENEVE]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
> \hyperref[intro:ipip]{[IPIP]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
> \hyperref[intro:nvgre]{[NVGRE]} */
> +\end{lstlisting}
 I'm afraid this one doesn't come out quite as intended, we'll end up
 with verbatim "\hyperref" text instead of a link. Anyone have a good
 idea on how to fix that?

 I'd prefer to push this now with the first issue addressed and to do an
 (editorial) patch on top to deal with the second issue (unless someone
 can come up with a really trivial fix for it, then I can apply that
 straightaway.)
>>> Someone suggested using escapechar:
>>> https://tex.stackexchange.com/questions/314903/inline-links-in-code-listings
>>>
>>> Didn't try.
>> Looks reasonable (and also revealed a typo for VXLAN-GPE). I think I'll
>> go ahead with this one.
>
> Yes. "intro:vxlan_gpe" -> "intro:vxlan-gpe".
>
> May I ask if the fix to these two problems is for me to make fix patches 
> or for you to solve it when editing?

I'd fix it myself while applying, but...

...while the changes above give me a fine pdf, the html generated is
broken. The problem seems to originate in the normative references that
are added in introduction.tex, but I don't see where it goes astray.

[401] [402] [403] [404] (./virtio-v1.2-cs01.aux
! Missing \endcsname inserted.
 
   \unhbox 
l.49 ...ction}Normative References}}{section.1}{}}
  
? 
! Emergency stop.
 
   \unhbox 
l.49 ...ction}Normative References}}{section.1}{}}

(makehtml.sh on master seems to work ok, so it's something in this
patch...)

Does anyone else manage to spot the problem?


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Cornelia Huck
On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:

> On Wed, Jul 12, 2023 at 02:22:26PM +0200, Cornelia Huck wrote:
>> On Mon, Jul 03 2023, Heng Qi  wrote:
>> 
>> (...)
>> 
>> > +\paragraph{Inner Header Hash}
>> > +\label{sec:Device Types / Network Device / Device Operation / Processing 
>> > of Incoming Packets / Inner Header Hash}
>> > +
>> > +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the 
>> > command
>> > +VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner 
>> > header hash.
>> > +
>> > +struct virtnet_hash_tunnel {
>> > +le32 enabled_tunnel_types;
>> > +};
>> > +
>> > +#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
>> > + #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
>> 
>> This needs to be wrapped in \begin{lstlisting}..\end{lstlisting}, can do
>> so when applying.
>> 
>> (...)
>> 
>> > +Encapsulation types applicable for inner header hash:
>> > +\begin{lstlisting}
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
>> > \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
>> > \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
>> > \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
>> > \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
>> > \hyperref[intro:vxlan]{[VXLAN]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
>> > \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
>> > \hyperref[intro:geneve]{[GENEVE]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
>> > \hyperref[intro:ipip]{[IPIP]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
>> > \hyperref[intro:nvgre]{[NVGRE]} */
>> > +\end{lstlisting}
>> 
>> I'm afraid this one doesn't come out quite as intended, we'll end up
>> with verbatim "\hyperref" text instead of a link. Anyone have a good
>> idea on how to fix that?
>> 
>> I'd prefer to push this now with the first issue addressed and to do an
>> (editorial) patch on top to deal with the second issue (unless someone
>> can come up with a really trivial fix for it, then I can apply that
>> straightaway.)
>
> Someone suggested using escapechar:
> https://tex.stackexchange.com/questions/314903/inline-links-in-code-listings
>
> Didn't try.

Looks reasonable (and also revealed a typo for VXLAN-GPE). I think I'll
go ahead with this one.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev] Re: [PATCH v12] virtio-net: support device stats

2023-07-12 Thread Michael S. Tsirkin
On Wed, Jul 12, 2023 at 02:24:32PM +0200, Cornelia Huck wrote:
> On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:
> 
> > On Wed, Jul 12, 2023 at 07:44:01PM +0800, Xuan Zhuo wrote:
> >> On Wed, 12 Jul 2023 07:34:38 -0400, "Michael S. Tsirkin"  
> >> wrote:
> >> > On Tue, Mar 15, 2022 at 11:24:02AM +0800, Xuan Zhuo wrote:
> >> > > This patch allows the driver to obtain some statistics from the device.
> >> > >
> >> > > In the back-end implementation, we can count a lot of such information,
> >> > > which can be used for debugging and judging the running status of the
> >> > > back-end. We hope to directly display it to the user through ethtool.
> >> > >
> >> > > To get stats atomically, try to get stats for all queue pairs in one
> >> > > command.
> >> > >
> >> > > Signed-off-by: Xuan Zhuo 
> >> > > Suggested-by: Michael S. Tsirkin 
> >> >
> >> > Functionally, I think this is close to ok now.  But the text needs
> >> > of work.  Are you trying to target 1.3 with this?
> >> 
> >> I personally worry about that 1.3 may be too late, if possible, I would 
> >> like to
> >> respond as soon as possible.
> >> 
> >> I really don't really care if it can get into 1.3 or not but I do hope 
> >> that this
> >> can be done as soon as possible.
> >> 
> >> Thanks.
> >
> > Yes we were supposed to freeze for 1.3. This change can be merged on a
> > main branch after 1.3 forks and is under review.
> 
> Nod, that's what I would prefer to do. Being merged on virtio-next
> should be enough for including device/driver implementations.

Except I'd prefer a v1.3 branch instead of a next branch - adding things
on the branch should be harder, not easier.

-- 
MST


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev] Re: [PATCH v12] virtio-net: support device stats

2023-07-12 Thread Cornelia Huck
On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:

> On Wed, Jul 12, 2023 at 07:44:01PM +0800, Xuan Zhuo wrote:
>> On Wed, 12 Jul 2023 07:34:38 -0400, "Michael S. Tsirkin"  
>> wrote:
>> > On Tue, Mar 15, 2022 at 11:24:02AM +0800, Xuan Zhuo wrote:
>> > > This patch allows the driver to obtain some statistics from the device.
>> > >
>> > > In the back-end implementation, we can count a lot of such information,
>> > > which can be used for debugging and judging the running status of the
>> > > back-end. We hope to directly display it to the user through ethtool.
>> > >
>> > > To get stats atomically, try to get stats for all queue pairs in one
>> > > command.
>> > >
>> > > Signed-off-by: Xuan Zhuo 
>> > > Suggested-by: Michael S. Tsirkin 
>> >
>> > Functionally, I think this is close to ok now.  But the text needs
>> > of work.  Are you trying to target 1.3 with this?
>> 
>> I personally worry about that 1.3 may be too late, if possible, I would like 
>> to
>> respond as soon as possible.
>> 
>> I really don't really care if it can get into 1.3 or not but I do hope that 
>> this
>> can be done as soon as possible.
>> 
>> Thanks.
>
> Yes we were supposed to freeze for 1.3. This change can be merged on a
> main branch after 1.3 forks and is under review.

Nod, that's what I would prefer to do. Being merged on virtio-next
should be enough for including device/driver implementations.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-12 Thread Cornelia Huck
On Mon, Jul 03 2023, Heng Qi  wrote:

(...)

> +\paragraph{Inner Header Hash}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of 
> Incoming Packets / Inner Header Hash}
> +
> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the 
> command
> +VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner 
> header hash.
> +
> +struct virtnet_hash_tunnel {
> +le32 enabled_tunnel_types;
> +};
> +
> +#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
> + #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0

This needs to be wrapped in \begin{lstlisting}..\end{lstlisting}, can do
so when applying.

(...)

> +Encapsulation types applicable for inner header hash:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
> \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
> \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
> \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
> \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
> \hyperref[intro:vxlan]{[VXLAN]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
> \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
> \hyperref[intro:geneve]{[GENEVE]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
> \hyperref[intro:ipip]{[IPIP]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
> \hyperref[intro:nvgre]{[NVGRE]} */
> +\end{lstlisting}

I'm afraid this one doesn't come out quite as intended, we'll end up
with verbatim "\hyperref" text instead of a link. Anyone have a good
idea on how to fix that?

I'd prefer to push this now with the first issue addressed and to do an
(editorial) patch on top to deal with the second issue (unless someone
can come up with a really trivial fix for it, then I can apply that
straightaway.)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [PATCH v12] virtio-net: support device stats

2023-07-12 Thread Michael S. Tsirkin
On Wed, Jul 12, 2023 at 07:44:01PM +0800, Xuan Zhuo wrote:
> On Wed, 12 Jul 2023 07:34:38 -0400, "Michael S. Tsirkin"  
> wrote:
> > On Tue, Mar 15, 2022 at 11:24:02AM +0800, Xuan Zhuo wrote:
> > > This patch allows the driver to obtain some statistics from the device.
> > >
> > > In the back-end implementation, we can count a lot of such information,
> > > which can be used for debugging and judging the running status of the
> > > back-end. We hope to directly display it to the user through ethtool.
> > >
> > > To get stats atomically, try to get stats for all queue pairs in one
> > > command.
> > >
> > > Signed-off-by: Xuan Zhuo 
> > > Suggested-by: Michael S. Tsirkin 
> >
> > Functionally, I think this is close to ok now.  But the text needs
> > of work.  Are you trying to target 1.3 with this?
> 
> I personally worry about that 1.3 may be too late, if possible, I would like 
> to
> respond as soon as possible.
> 
> I really don't really care if it can get into 1.3 or not but I do hope that 
> this
> can be done as soon as possible.
> 
> Thanks.

Yes we were supposed to freeze for 1.3. This change can be merged on a
main branch after 1.3 forks and is under review.


> 
> >
> > Feedback from other hw vendors would be nice. Parav could you take
> > a look and say whether the list of counters seems rich enough for
> > you? Any counters you'd like to add?
> >
> >
> > > ---
> > >  conformance.tex |   2 +
> > >  content.tex | 406 +++-
> > >  2 files changed, 405 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/conformance.tex b/conformance.tex
> > > index 42f8537..c67f877 100644
> > > --- a/conformance.tex
> > > +++ b/conformance.tex
> > > @@ -142,6 +142,7 @@ \section{Conformance Targets}\label{sec:Conformance / 
> > > Conformance Targets}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device 
> > > Operation / Control Virtqueue / Automatic receive steering in multiqueue 
> > > mode}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device 
> > > Operation / Control Virtqueue / Offloads State Configuration / Setting 
> > > Offloads State}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device 
> > > Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > +\item \ref{drivernormative:Device Types / Network Device / Device 
> > > Operation / Control Virtqueue / Device Stats}
> > >  \end{itemize}
> > >
> > >  \conformance{\subsection}{Block Driver 
> > > Conformance}\label{sec:Conformance / Driver Conformance / Block Driver 
> > > Conformance}
> > > @@ -401,6 +402,7 @@ \section{Conformance Targets}\label{sec:Conformance / 
> > > Conformance Targets}
> > >  \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}
> > >  \item \ref{devicenormative:Device Types / Network Device / Device 
> > > Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS 
> > > processing}
> > > +\item \ref{devicenormative:Device Types / Network Device / Device 
> > > Operation / Control Virtqueue / Device Stats}
> > >  \end{itemize}
> > >
> > >  \conformance{\subsection}{Block Device 
> > > Conformance}\label{sec:Conformance / Device Conformance / Block Device 
> > > Conformance}
> > > diff --git a/content.tex b/content.tex
> > > index c6f116c..81f325d 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / 
> > > Network Device / Feature bits
> > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > >  channel.
> > >
> > > +\item[VIRTIO_NET_F_CTRL_STATS(55)] Device can provide device-level 
> > > statistics
> > > +to the driver through the control channel.
> > > +
> > >  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike 
> > > UFO
> > >   (fragmenting the packet) the USO splits large UDP packet
> > >   to several segments when each of these smaller packets has UDP header.
> > > @@ -3137,6 +3140,7 @@ \subsubsection{Feature bit 
> > > requirements}\label{sec:Device Types / Network Device
> > >  \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > > +\item[VIRTIO_NET_F_CTRL_STATS] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or 
> > > VIRTIO_NET_F_HOST_TSO6.
> > >  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \end{description}
> > > @@ -4015,6 +4019,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device 
> > > Types / Network Device / Devi
> > >  u8 command;
> > >  u8 command-specific-data[];
> > >  u8 ack;

[virtio-dev] Re: [PATCH v12] virtio-net: support device stats

2023-07-12 Thread Michael S. Tsirkin
On Tue, Mar 15, 2022 at 11:24:02AM +0800, Xuan Zhuo wrote:
> This patch allows the driver to obtain some statistics from the device.
> 
> In the back-end implementation, we can count a lot of such information,
> which can be used for debugging and judging the running status of the
> back-end. We hope to directly display it to the user through ethtool.
> 
> To get stats atomically, try to get stats for all queue pairs in one
> command.
> 
> Signed-off-by: Xuan Zhuo 
> Suggested-by: Michael S. Tsirkin 

Functionally, I think this is close to ok now.  But the text needs
of work.  Are you trying to target 1.3 with this?

Feedback from other hw vendors would be nice. Parav could you take
a look and say whether the list of counters seems rich enough for
you? Any counters you'd like to add?


> ---
>  conformance.tex |   2 +
>  content.tex | 406 +++-
>  2 files changed, 405 insertions(+), 3 deletions(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index 42f8537..c67f877 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -142,6 +142,7 @@ \section{Conformance Targets}\label{sec:Conformance / 
> Conformance Targets}
>  \item \ref{drivernormative:Device Types / Network Device / Device Operation 
> / Control Virtqueue / Automatic receive steering in multiqueue mode}
>  \item \ref{drivernormative:Device Types / Network Device / Device Operation 
> / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
>  \item \ref{drivernormative:Device Types / Network Device / Device Operation 
> / Control Virtqueue / Receive-side scaling (RSS) }
> +\item \ref{drivernormative:Device Types / Network Device / Device Operation 
> / Control Virtqueue / Device Stats}
>  \end{itemize}
>  
>  \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / 
> Driver Conformance / Block Driver Conformance}
> @@ -401,6 +402,7 @@ \section{Conformance Targets}\label{sec:Conformance / 
> Conformance Targets}
>  \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}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation 
> / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> +\item \ref{devicenormative:Device Types / Network Device / Device Operation 
> / Control Virtqueue / Device Stats}
>  \end{itemize}
>  
>  \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / 
> Device Conformance / Block Device Conformance}
> diff --git a/content.tex b/content.tex
> index c6f116c..81f325d 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / 
> Network Device / Feature bits
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>  channel.
>  
> +\item[VIRTIO_NET_F_CTRL_STATS(55)] Device can provide device-level statistics
> +to the driver through the control channel.
> +
>  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
>   (fragmenting the packet) the USO splits large UDP packet
>   to several segments when each of these smaller packets has UDP header.
> @@ -3137,6 +3140,7 @@ \subsubsection{Feature bit 
> requirements}\label{sec:Device Types / Network Device
>  \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> +\item[VIRTIO_NET_F_CTRL_STATS] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or 
> VIRTIO_NET_F_HOST_TSO6.
>  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
>  \end{description}
> @@ -4015,6 +4019,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device 
> Types / Network Device / Devi
>  u8 command;
>  u8 command-specific-data[];
>  u8 ack;
> +u8 command-specific-data-reply[];
>  };
>  
>  /* ack values */
> @@ -4023,9 +4028,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device 
> Types / Network Device / Devi
>  \end{lstlisting}
>  
>  The \field{class}, \field{command} and command-specific-data are set by the
> -driver, and the device sets the \field{ack} byte. There is little it can
> -do except issue a diagnostic if \field{ack} is not
> -VIRTIO_NET_OK.
> +driver, and the device sets the \field{ack} byte and optionally
> +\field{command-specific-data-reply}. There is little the driver can
> +do except issue a diagnostic if \field{ack} is not VIRTIO_NET_OK.
> +
> +The command VIRTIO_NET_CTRL_STATS_GET contains 
> \field{command-specific-data-reply}.
>  
>  \paragraph{Packet Receive Filtering}\label{sec:Device Types / Network Device 
> / Device Operation / Control Virtqueue / Packet Receive Filtering}
>  

[virtio-dev] [VIRTIO GPU PATCH v2 1/1] virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZING

2023-07-12 Thread Jiqian Chen
When we suspend/resume guest on Xen, the display can't come back.
This is because when guest suspended, it called into Qemu. Then
Qemu destroyed all resources which is used for display. So that
guest's display can't come back to the time when it was suspended.

To solve above problem, I added a new mechanism that when guest is
suspending, it will notify Qemu, and then Qemu will not destroy
resources.

Due to that mechanism needs cooperation between guest and host,
I need to add a new feature flag, so that guest and host can
negotiate whenever freezing is supported or not.

Signed-off-by: Jiqian Chen 
---
 device-types/gpu/description.tex | 35 
 1 file changed, 35 insertions(+)

diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
index 4435248..729dde5 100644
--- a/device-types/gpu/description.tex
+++ b/device-types/gpu/description.tex
@@ -37,6 +37,8 @@ \subsection{Feature bits}\label{sec:Device Types / GPU Device 
/ Feature bits}
   resources is supported.
 \item[VIRTIO_GPU_F_CONTEXT_INIT (4)] multiple context types and
   synchronization timelines supported.  Requires VIRTIO_GPU_F_VIRGL.
+\item[VIRTIO_GPU_F_FREEZING (5)] freezing virtio-gpu and keeping resources
+  alive is supported.
 \end{description}
 
 \subsection{Device configuration layout}\label{sec:Device Types / GPU Device / 
Device configuration layout}
@@ -228,6 +230,9 @@ \subsubsection{Device Operation: Request 
header}\label{sec:Device Types / GPU De
 VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
 VIRTIO_GPU_CMD_MOVE_CURSOR,
 
+/* status */
+VIRTIO_GPU_CMD_STATUS_FREEZING = 0x0400,
+
 /* success responses */
 VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
 VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
@@ -838,6 +843,36 @@ \subsubsection{Device Operation: cursorq}\label{sec:Device 
Types / GPU Device /
 
 \end{description}
 
+\subsubsection{Device Operation: status}\label{sec:Device Types / GPU Device / 
Device Operation / Device Operation: cursorq}
+
+\begin{lstlisting}
+struct virtio_gpu_status_freezing {
+  struct virtio_gpu_ctrl_hdr hdr;
+  __u32 freezing;
+};
+\end{lstlisting}
+
+\begin{description}
+
+\item[VIRTIO_GPU_CMD_STATUS_FREEZING]
+Notify freezing status through controlq.
+Request data is \field{struct virtio_gpu_status_freezing}.
+Response type is VIRTIO_GPU_RESP_OK_NODATA.
+
+This is added for S3 function in guest with virtio-gpu. When guest does
+S3, let it notify QEMU if virtio-gpu is in freezing status or not in
+\field{freezing}. Zero means it is not in freezing status, none-zero
+is the opposite. When virtio-gpu is in freezing status, QEMU will not
+destroy resources which are created by using commands
+VIRTIO_GPU_CMD_RESOURCE_CREATE_*, so that guest can use those resources
+to resume display.
+
+Note: this change is not enough to solve the problems of S4 function.
+QEMU may lose resources after hibernation. It needs more research and
+development.
+
+\end{description}
+
 \subsection{VGA Compatibility}\label{sec:Device Types / GPU Device / VGA 
Compatibility}
 
 Applies to Virtio Over PCI only.  The GPU device can come with and
-- 
2.34.1


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] [VIRTIO GPU PATCH v2 0/1] Add new feature flag VIRTIO_GPU_F_FREEZING

2023-07-12 Thread Jiqian Chen
v2:

Hi all,
Thanks to Gerd Hoffmann for his suggestions. V2 makes below changes:
* Elaborate on the types of resources.
* Add some descriptions for S3 and S4.


v1:

Hi all,
I am working to implement virtgpu S3 function on Xen.

Currently on Xen, if we start a guest through Qemu with enabling virtgpu,
and then suspend and s3resume guest. We can find that the guest kernel
comes back, but the display doesn't. It just shown a black screen.

That is because when guest was during suspending, it called into Qemu and
Qemu destroyed all resources and reset renderer. This made the display
gone after guest resumed.

So, I add a mechanism that when guest is suspending, it will notify Qemu,
and then Qemu will not destroy resources. That can help guest's display
come back.

As discussed and suggested by Robert Beckett and Gerd Hoffmann on v1
qemu's mailing list. Due to that mechanism needs cooperation between
guest and host. What's more, as virtio drivers by design paravirt
drivers, it is reasonable for guest to accept some cooperation with host
to manage suspend/resume. So I request to add a new feature flag, so that
guest and host can negotiate whenever freezing is supported or not.

Attach the patches of Qemu and kernel:
Qemu
v1: 
https://lore.kernel.org/qemu-devel/20230608025655.1674357-2-jiqian.c...@amd.com/
v2: 
https://lore.kernel.org/qemu-devel/20230630070016.841459-1-jiqian.c...@amd.com/T/#t
Kernel
v1: 
https://lore.kernel.org/lkml/20230608063857.1677973-1-jiqian.c...@amd.com/
v2: 
https://lore.kernel.org/lkml/20230630073448.842767-1-jiqian.c...@amd.com/T/#t

Best regards,
Jiqian Chen.

Jiqian Chen (1):
  virtio-gpu: Add new feature flag VIRTIO_GPU_F_FREEZING

 device-types/gpu/description.tex | 35 
 1 file changed, 35 insertions(+)

-- 
2.34.1


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev] [PATCH v12] virtio-net: support device stats

2023-07-12 Thread Jason Wang
On Mon, Jul 10, 2023 at 3:32 PM Xuan Zhuo 
wrote:

> hi, guys
>
> After a lot of internal discussions, I removed some unimportant counters.
> Based
> on this v12 patch I am repling to, most of the requirements have been met.
>
> The patch link
> https://lore.kernel.org/all/20220315032402.6088-1-xuanz...@linux.alibaba.com/
>
> We still have these counters, let's see if they can be standardized.
>
> ## limit
>
> * tx_bps_limit_drops
> * tx_pps_limit_drops
> * rx_bps_limit_drops
> * rx_pps_limit_drops
>
> In a cloud scenario, multiple users purchase different VMs, and these VMs
> share
> the capabilities of the same host. In order to ensure that each VM will not
> affect others, the network card(virtio-net) capability of each VM is
> limited.
> These users purchase VMs, this limit has already been determined.
>

This seems a feature beyond the counters but QOS. I think we virtio spec
need to support QOS before we can discuss any QOS related counters.


>
> So if the network card traffic of a vm exceeds the upper limit, packet
> loss will
> occur. It is necessary for us to count these packet losses. And the device
> should expose to the user.
>
> ## session
>
> * tx_session_full_drops The number of packet drops in the sending
> direction when
> the session is full
> * rx_session_full_drops The number of packets lost when the session is
> full in the receiving direction
>
> Our dpu supports tcp connection tracking, but there is an upper limit to
> the
> number of connections, and if it exceeds, packet loss will also occur.
>

Similarly, if connect tracking offload is supported, it needs to be
implemented in the spec first then we can have related counters.


>
> ## ACL
>
> * tx_acl_drops ACL packet loss in the sending direction
> * rx_acl_drops The number of ACL packets lost in the receiving direction
>
> In our cloud service, for network cards, users are allowed to configure
> security
> rules,such as which IPs can access which ports of this machine.
>

Same as above, ACL should be supported by the spec first then the counters.

In conclusion, the features must be self contained. Otherwise you are doing
vDPA actually and those counters need to be accessed in a vendor specific
way.

Thanks


>
>
> Thanks
>
>
> On Tue, 15 Mar 2022 11:24:02 +0800, Xuan Zhuo 
> wrote:
> > This patch allows the driver to obtain some statistics from the device.
> >
> > In the back-end implementation, we can count a lot of such information,
> > which can be used for debugging and judging the running status of the
> > back-end. We hope to directly display it to the user through ethtool.
> >
> > To get stats atomically, try to get stats for all queue pairs in one
> > command.
> >
> > Signed-off-by: Xuan Zhuo 
> > Suggested-by: Michael S. Tsirkin 
> > ---
> >  conformance.tex |   2 +
> >  content.tex | 406 +++-
> >  2 files changed, 405 insertions(+), 3 deletions(-)
> >
> > diff --git a/conformance.tex b/conformance.tex
> > index 42f8537..c67f877 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -142,6 +142,7 @@ \section{Conformance Targets}\label{sec:Conformance
> / Conformance Targets}
> >  \item \ref{drivernormative:Device Types / Network Device / Device
> Operation / Control Virtqueue / Automatic receive steering in multiqueue
> mode}
> >  \item \ref{drivernormative:Device Types / Network Device / Device
> Operation / Control Virtqueue / Offloads State Configuration / Setting
> Offloads State}
> >  \item \ref{drivernormative:Device Types / Network Device / Device
> Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > +\item \ref{drivernormative:Device Types / Network Device / Device
> Operation / Control Virtqueue / Device Stats}
> >  \end{itemize}
> >
> >  \conformance{\subsection}{Block Driver
> Conformance}\label{sec:Conformance / Driver Conformance / Block Driver
> Conformance}
> > @@ -401,6 +402,7 @@ \section{Conformance Targets}\label{sec:Conformance
> / Conformance Targets}
> >  \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}
> >  \item \ref{devicenormative:Device Types / Network Device / Device
> Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > +\item \ref{devicenormative:Device Types / Network Device / Device
> Operation / Control Virtqueue / Device Stats}
> >  \end{itemize}
> >
> >  \conformance{\subsection}{Block Device
> Conformance}\label{sec:Conformance / Device Conformance / Block Device
> Conformance}
> > diff --git a/content.tex b/content.tex
> > index c6f116c..81f325d 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types /
> Network Device / Feature bits
> >