* [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets
@ 2022-12-21 10:17 Alvaro Karsz
2022-12-21 11:08 ` [virtio-comment] " Xuan Zhuo
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Alvaro Karsz @ 2022-12-21 10:17 UTC (permalink / raw)
To: virtio-comment, virtio-dev; +Cc: jasowang, mst, Alvaro Karsz
This patch adds a new feature VIRTIO_NET_F_NOTF_COAL_LOW_HIGH,
and adds 2 new commands to VIRTIO_NET_CTRL_NOTF_COAL class:
* VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET - create a low-rate set of
coalescing parameters. the device should switch to this set when
the packet rate (packets per second) reaches the pkt_rate_low threshold.
* VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET - create a high-rate set of
coalescing parameters. the device should switch to this set when
the packet rate (packets per second) reaches the pkt_rate_high threshold.
A device may have up to 3 sets, and should switch between them based on
the packet rate.
Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
---
content.tex | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/content.tex b/content.tex
index 96f4723..969f945 100644
--- a/content.tex
+++ b/content.tex
@@ -3088,6 +3088,8 @@ \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_NOTF_COAL_LOW_HIGH(50)] Device supports notifications coalescing low rate and high rate sets.
+
\item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
\item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
@@ -3142,6 +3144,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
\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_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
+\item[VIRTIO_NET_F_NOTF_COAL_LOW_HIGH] Requires VIRTIO_NET_F_NOTF_COAL.
\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}
@@ -4504,9 +4507,25 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
le32 tx_usecs;
};
+//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
+struct virtio_net_ctrl_coal_low {
+ le32 pkt_rate_low;
+ struct virtio_net_ctrl_coal_tx tx;
+ struct virtio_net_ctrl_coal_rx rx;
+};
+
+//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
+struct virtio_net_ctrl_coal_high {
+ le32 pkt_rate_high;
+ struct virtio_net_ctrl_coal_tx tx;
+ struct virtio_net_ctrl_coal_rx rx;
+};
+
#define VIRTIO_NET_CTRL_NOTF_COAL 6
#define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
#define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
+ #define VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET 2 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
+ #define VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET 3 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
\end{lstlisting}
Coalescing parameters:
@@ -4515,13 +4534,17 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
\item \field{tx_usecs}: Maximum number of usecs to delay a TX notification.
\item \field{rx_max_packets}: Maximum number of packets to receive before a RX notification.
\item \field{tx_max_packets}: Maximum number of packets to send before a TX notification.
+\item \field{pkt_rate_low}: Threshold for low packet rate set (packets per second).
+\item \field{pkt_rate_high}: Threshold for high packet rate set (packets per second).
\end{itemize}
-The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
+The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
\begin{enumerate}
\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and \field{tx_max_packets} parameters.
\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and \field{rx_max_packets} parameters.
+\item VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_low} or less packets per second.
+\item VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_high} or more packets per second.
\end{enumerate}
\subparagraph{RX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Notifications}
@@ -4554,18 +4577,54 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
\item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
\end{itemize}
+\subparagraph{Low/High Rate Notifications Sets}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Low/High Rate Notifications Sets}
+
+If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature is negotiated, the driver may send VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands to the device.
+
+The virtio_net_ctrl_coal_low struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or lower than \field{pkt_rate_low} packets per second.
+
+The virtio_net_ctrl_coal_high struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or higher than \field{pkt_rate_high} packets per second.
+
+A device may have up to 3 sets of coalescing parameters, and should switch between the sets based on the packet rate.
+
+A device may have only a low rate set, in this case, it should coalesce notifications only when the packet rate crosses down the \field{pkt_rate_low} threshold.
+
+A device may have only a high rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold.
+
+A device may have only a high rate set and a low rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold, or crosses down the \field{pkt_rate_low} threshold.
+
\drivernormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
If the VIRTIO_NET_F_NOTF_COAL feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL commands.
+If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands.
+
+The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 in order to remove the low rate set.
+
+The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 in order to remove the high rate set.
+
+The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET was issued.
+
+The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET was issued.
+
\devicenormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
A device SHOULD respond to the VIRTIO_NET_CTRL_NOTF_COAL commands with VIRTIO_NET_ERR if it was not able to change the parameters.
A device SHOULD NOT send used buffer notifications to the driver, if the notifications are suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, even if the coalescing counters expired.
+The device MUST remove the low rate set if a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 is received.
+
+The device MUST remove the high rate set if a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 is received.
+
+The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if a high rate set is being used.
+
+The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if a low rate set is being used.
+
Upon reset, a device MUST initialize all coalescing parameters to 0.
+Upon reset, a device MUST not have a low rate and high rate sets.
+
\subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
Types / Network Device / Legacy Interface: Framing Requirements}
--
2.32.0
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [virtio-comment] Re: [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets
2022-12-21 10:17 [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets Alvaro Karsz
@ 2022-12-21 11:08 ` Xuan Zhuo
2022-12-21 11:48 ` Alvaro Karsz
2023-01-04 14:27 ` [virtio-dev] " Alvaro Karsz
2023-01-04 15:13 ` [virtio-comment] " Michael S. Tsirkin
2 siblings, 1 reply; 9+ messages in thread
From: Xuan Zhuo @ 2022-12-21 11:08 UTC (permalink / raw)
To: Alvaro Karsz
Cc: jasowang, mst, Alvaro Karsz, virtio-comment, virtio-dev, hengqi
On Wed, 21 Dec 2022 12:17:29 +0200, Alvaro Karsz <alvaro.karsz@solid-run.com> wrote:
> This patch adds a new feature VIRTIO_NET_F_NOTF_COAL_LOW_HIGH,
> and adds 2 new commands to VIRTIO_NET_CTRL_NOTF_COAL class:
> * VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET - create a low-rate set of
> coalescing parameters. the device should switch to this set when
> the packet rate (packets per second) reaches the pkt_rate_low threshold.
>
> * VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET - create a high-rate set of
> coalescing parameters. the device should switch to this set when
> the packet rate (packets per second) reaches the pkt_rate_high threshold.
>
> A device may have up to 3 sets, and should switch between them based on
> the packet rate.
I want to know which one is better than NetDim(Coalesce Adaptive) in driver.
I know Heng Qi's work in this.
Thanks
>
> Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
> ---
> content.tex | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/content.tex b/content.tex
> index 96f4723..969f945 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3088,6 +3088,8 @@ \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_NOTF_COAL_LOW_HIGH(50)] Device supports notifications coalescing low rate and high rate sets.
> +
> \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>
> \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -3142,6 +3144,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> \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_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
> +\item[VIRTIO_NET_F_NOTF_COAL_LOW_HIGH] Requires VIRTIO_NET_F_NOTF_COAL.
> \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}
> @@ -4504,9 +4507,25 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> le32 tx_usecs;
> };
>
> +//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> +struct virtio_net_ctrl_coal_low {
> + le32 pkt_rate_low;
> + struct virtio_net_ctrl_coal_tx tx;
> + struct virtio_net_ctrl_coal_rx rx;
> +};
> +
> +//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> +struct virtio_net_ctrl_coal_high {
> + le32 pkt_rate_high;
> + struct virtio_net_ctrl_coal_tx tx;
> + struct virtio_net_ctrl_coal_rx rx;
> +};
> +
> #define VIRTIO_NET_CTRL_NOTF_COAL 6
> #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> + #define VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET 2 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> + #define VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET 3 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> \end{lstlisting}
>
> Coalescing parameters:
> @@ -4515,13 +4534,17 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> \item \field{tx_usecs}: Maximum number of usecs to delay a TX notification.
> \item \field{rx_max_packets}: Maximum number of packets to receive before a RX notification.
> \item \field{tx_max_packets}: Maximum number of packets to send before a TX notification.
> +\item \field{pkt_rate_low}: Threshold for low packet rate set (packets per second).
> +\item \field{pkt_rate_high}: Threshold for high packet rate set (packets per second).
> \end{itemize}
>
>
> -The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> \begin{enumerate}
> \item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and \field{tx_max_packets} parameters.
> \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and \field{rx_max_packets} parameters.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_low} or less packets per second.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_high} or more packets per second.
> \end{enumerate}
>
> \subparagraph{RX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Notifications}
> @@ -4554,18 +4577,54 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
> \end{itemize}
>
> +\subparagraph{Low/High Rate Notifications Sets}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Low/High Rate Notifications Sets}
> +
> +If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature is negotiated, the driver may send VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands to the device.
> +
> +The virtio_net_ctrl_coal_low struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or lower than \field{pkt_rate_low} packets per second.
> +
> +The virtio_net_ctrl_coal_high struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or higher than \field{pkt_rate_high} packets per second.
> +
> +A device may have up to 3 sets of coalescing parameters, and should switch between the sets based on the packet rate.
> +
> +A device may have only a low rate set, in this case, it should coalesce notifications only when the packet rate crosses down the \field{pkt_rate_low} threshold.
> +
> +A device may have only a high rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold.
> +
> +A device may have only a high rate set and a low rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold, or crosses down the \field{pkt_rate_low} threshold.
> +
> \drivernormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>
> If the VIRTIO_NET_F_NOTF_COAL feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL commands.
>
> +If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands.
> +
> +The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 in order to remove the low rate set.
> +
> +The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 in order to remove the high rate set.
> +
> +The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET was issued.
> +
> +The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET was issued.
> +
> \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>
> A device SHOULD respond to the VIRTIO_NET_CTRL_NOTF_COAL commands with VIRTIO_NET_ERR if it was not able to change the parameters.
>
> A device SHOULD NOT send used buffer notifications to the driver, if the notifications are suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, even if the coalescing counters expired.
>
> +The device MUST remove the low rate set if a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 is received.
> +
> +The device MUST remove the high rate set if a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 is received.
> +
> +The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if a high rate set is being used.
> +
> +The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if a low rate set is being used.
> +
> Upon reset, a device MUST initialize all coalescing parameters to 0.
>
> +Upon reset, a device MUST not have a low rate and high rate sets.
> +
> \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> Types / Network Device / Legacy Interface: Framing Requirements}
>
> --
> 2.32.0
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-comment] Re: [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets
2022-12-21 11:08 ` [virtio-comment] " Xuan Zhuo
@ 2022-12-21 11:48 ` Alvaro Karsz
2022-12-21 12:45 ` Heng Qi
2022-12-21 13:58 ` Xuan Zhuo
0 siblings, 2 replies; 9+ messages in thread
From: Alvaro Karsz @ 2022-12-21 11:48 UTC (permalink / raw)
To: Xuan Zhuo; +Cc: jasowang, mst, virtio-comment, virtio-dev, hengqi
Hi,
> I want to know which one is better than NetDim(Coalesce Adaptive) in driver.
>
> I know Heng Qi's work in this.
>
> Thanks
Why choose? we can have both.
ethtool can handle both pkt_rate_low/pkt_rate_high and
use_adaptive_rx_coalesce/use_adaptive_tx_coalesce.
The adaptive algorithm can even use this feature to set low and high
coalescing sets.
Alvaro
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-comment] Re: [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets
2022-12-21 11:48 ` Alvaro Karsz
@ 2022-12-21 12:45 ` Heng Qi
2022-12-21 13:58 ` Xuan Zhuo
1 sibling, 0 replies; 9+ messages in thread
From: Heng Qi @ 2022-12-21 12:45 UTC (permalink / raw)
To: Alvaro Karsz, Xuan Zhuo; +Cc: jasowang, mst, virtio-comment, virtio-dev
在 2022/12/21 下午7:48, Alvaro Karsz 写道:
> Hi,
>
>> I want to know which one is better than NetDim(Coalesce Adaptive) in driver.
>>
>> I know Heng Qi's work in this.
>>
>> Thanks
>
> Why choose? we can have both.
> ethtool can handle both pkt_rate_low/pkt_rate_high and
> use_adaptive_rx_coalesce/use_adaptive_tx_coalesce.
>
> The adaptive algorithm can even use this feature to set low and high
> coalescing sets.
Hi, all.
NetDIM is currently a mature library in the kernel. It uses the number
of bytes, PPS and interrupt rate
as samples to make an action, and it is performed independently in tx or
rx direction. Also, There will
be an extra worker to help us send the configuration based on the
control queue to avoid interrupting
the softirq. Although the method of pkt_rate_{low, high} params does not
conflict with dim, I seem
to have some doubts that the colasce parameters of the rx and tx
directions are determined at the
same time based on pkt_rate alone, will this be a problem?
Thanks.
>
> Alvaro
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-comment] Re: [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets
2022-12-21 11:48 ` Alvaro Karsz
2022-12-21 12:45 ` Heng Qi
@ 2022-12-21 13:58 ` Xuan Zhuo
1 sibling, 0 replies; 9+ messages in thread
From: Xuan Zhuo @ 2022-12-21 13:58 UTC (permalink / raw)
To: Alvaro Karsz; +Cc: jasowang, mst, virtio-comment, virtio-dev, hengqi
On Wed, 21 Dec 2022 13:48:59 +0200, Alvaro Karsz <alvaro.karsz@solid-run.com> wrote:
> Hi,
>
> > I want to know which one is better than NetDim(Coalesce Adaptive) in driver.
> >
> > I know Heng Qi's work in this.
> >
> > Thanks
>
>
> Why choose? we can have both.
> ethtool can handle both pkt_rate_low/pkt_rate_high and
> use_adaptive_rx_coalesce/use_adaptive_tx_coalesce.
>
> The adaptive algorithm can even use this feature to set low and high
> coalescing sets.
Yes, I understand.
But if a function can be implemented on driver, we don't need to let the device
support it.
I hope that the device is simpler, if it is possible.
Thanks.
>
> Alvaro
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_net: support low and high rate of notification coalescing sets
2022-12-21 10:17 [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets Alvaro Karsz
2022-12-21 11:08 ` [virtio-comment] " Xuan Zhuo
@ 2023-01-04 14:27 ` Alvaro Karsz
2023-01-04 15:13 ` [virtio-comment] " Michael S. Tsirkin
2 siblings, 0 replies; 9+ messages in thread
From: Alvaro Karsz @ 2023-01-04 14:27 UTC (permalink / raw)
To: virtio-comment, virtio-dev; +Cc: jasowang, mst
Ping
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-comment] Re: [PATCH] virtio_net: support low and high rate of notification coalescing sets
2022-12-21 10:17 [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets Alvaro Karsz
2022-12-21 11:08 ` [virtio-comment] " Xuan Zhuo
2023-01-04 14:27 ` [virtio-dev] " Alvaro Karsz
@ 2023-01-04 15:13 ` Michael S. Tsirkin
2023-01-04 18:40 ` Alvaro Karsz
2 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2023-01-04 15:13 UTC (permalink / raw)
To: Alvaro Karsz; +Cc: virtio-comment, virtio-dev, jasowang
On Wed, Dec 21, 2022 at 12:17:29PM +0200, Alvaro Karsz wrote:
> This patch adds a new feature VIRTIO_NET_F_NOTF_COAL_LOW_HIGH,
> and adds 2 new commands to VIRTIO_NET_CTRL_NOTF_COAL class:
> * VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET - create a low-rate set of
> coalescing parameters. the device should switch to this set when
> the packet rate (packets per second) reaches the pkt_rate_low threshold.
>
> * VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET - create a high-rate set of
> coalescing parameters. the device should switch to this set when
> the packet rate (packets per second) reaches the pkt_rate_high threshold.
>
> A device may have up to 3 sets, and should switch between them based on
> the packet rate.
>
> Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
there seems to be a concept of a "set of coalescing parameters".
if we use it we'll have to define it, I don't think it's a standard
thing. I personally don't think we should bother, we currently
talk about "coalescing parameters".
I propose unifying virtio_net_ctrl_coal_rx and virtio_net_ctrl_coal_tx
since they are the same anyway. Then reuse.
> ---
> content.tex | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/content.tex b/content.tex
> index 96f4723..969f945 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3088,6 +3088,8 @@ \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_NOTF_COAL_LOW_HIGH(50)] Device supports notifications coalescing low rate and high rate sets.
> +
> \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>
> \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -3142,6 +3144,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> \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_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
> +\item[VIRTIO_NET_F_NOTF_COAL_LOW_HIGH] Requires VIRTIO_NET_F_NOTF_COAL.
> \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}
> @@ -4504,9 +4507,25 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> le32 tx_usecs;
> };
>
> +//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
space after // pls. It's also unclear to put this comment
before struct. a struct is just a struct, a comment such as this
can only refer to fields in structs.
> +struct virtio_net_ctrl_coal_low {
> + le32 pkt_rate_low;
> + struct virtio_net_ctrl_coal_tx tx;
> + struct virtio_net_ctrl_coal_rx rx;
> +};
is it really true we always want the same rate for tx and rx?
why not separate commands for each? this is the way we went
for general coalescing at least.
> +
> +//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> +struct virtio_net_ctrl_coal_high {
> + le32 pkt_rate_high;
> + struct virtio_net_ctrl_coal_tx tx;
> + struct virtio_net_ctrl_coal_rx rx;
> +};
> +
given these are identical, do we want to unify these?
maybe virtio_net_ctrl_coal_threshold ?
> #define VIRTIO_NET_CTRL_NOTF_COAL 6
> #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> + #define VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET 2 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> + #define VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET 3 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> \end{lstlisting}
>
> Coalescing parameters:
> @@ -4515,13 +4534,17 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> \item \field{tx_usecs}: Maximum number of usecs to delay a TX notification.
> \item \field{rx_max_packets}: Maximum number of packets to receive before a RX notification.
> \item \field{tx_max_packets}: Maximum number of packets to send before a TX notification.
> +\item \field{pkt_rate_low}: Threshold for low packet rate set (packets per second).
what does "set" mean here?
and I'd take (packets per second) out of ():
low packet rate, in units of packets per second.
> +\item \field{pkt_rate_high}: Threshold for high packet rate set (packets per second).
> \end{itemize}
>
>
> -The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> \begin{enumerate}
> \item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and \field{tx_max_packets} parameters.
> \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and \field{rx_max_packets} parameters.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_low} or less packets per second.
set repeated twice here. confusing to the point where I couldn't figure out the meaning exactly.
for a rate of \field{pkt_rate_low} or less packets per second
do you mean
when packet rate is \field{pkt_rate_low} or less?
> +\item VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_high} or more packets per second.
> \end{enumerate}
>
> \subparagraph{RX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Notifications}
> @@ -4554,18 +4577,54 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
> \end{itemize}
>
> +\subparagraph{Low/High Rate Notifications Sets}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Low/High Rate Notifications Sets}
> +
> +If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature is negotiated, the driver may send VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands to the device.
> +
> +The virtio_net_ctrl_coal_low struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or lower than \field{pkt_rate_low} packets per second.
packets rate -> packet rate
here and elsewhere
> +
> +The virtio_net_ctrl_coal_high struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or higher than \field{pkt_rate_high} packets per second.
equal to or higher than
> +
> +A device may have up to 3 sets of coalescing parameters, and should switch between the sets based on the packet rate.
what are the 3 sets?
avoid "may" outside conformance sections please.
> +A device may have only a low rate set, in this case, it should coalesce notifications only when the packet rate crosses down the \field{pkt_rate_low} threshold.
> +
> +A device may have only a high rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold.
> +
> +A device may have only a high rate set and a low rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold, or crosses down the \field{pkt_rate_low} threshold.
in this case it's not both.
> +
> \drivernormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>
> If the VIRTIO_NET_F_NOTF_COAL feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL commands.
>
> +If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands.
> +
> +The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 in order to remove the low rate set.
> +
> +The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 in order to remove the high rate set.
> +
> +The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET was issued.
> +
> +The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET was issued.
> +
> \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>
> A device SHOULD respond to the VIRTIO_NET_CTRL_NOTF_COAL commands with VIRTIO_NET_ERR if it was not able to change the parameters.
>
> A device SHOULD NOT send used buffer notifications to the driver, if the notifications are suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, even if the coalescing counters expired.
>
> +The device MUST remove the low rate set if a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 is received.
> +
> +The device MUST remove the high rate set if a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 is received.
> +
> +The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if a high rate set is being used.
> +
> +The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if a low rate set is being used.
> +
> Upon reset, a device MUST initialize all coalescing parameters to 0.
>
> +Upon reset, a device MUST not have a low rate and high rate sets.
> +
how about documenting how the coalescing is used.
E.g. is it best effort or mandatory?
I also noticed that coalescing is currently under-specified with
respect to delivering interrupts when config changes.
For example, what happens if I get 1 packet then lower max packets to 1?
Do I get an interrupt immediately or does the counter get reset?
> \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> Types / Network Device / Legacy Interface: Framing Requirements}
>
> --
> 2.32.0
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-comment] Re: [PATCH] virtio_net: support low and high rate of notification coalescing sets
2023-01-04 15:13 ` [virtio-comment] " Michael S. Tsirkin
@ 2023-01-04 18:40 ` Alvaro Karsz
2023-01-05 7:32 ` Alvaro Karsz
0 siblings, 1 reply; 9+ messages in thread
From: Alvaro Karsz @ 2023-01-04 18:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-comment, virtio-dev, jasowang
Thanks for your comments.
> On Wed, Dec 21, 2022 at 12:17:29PM +0200, Alvaro Karsz wrote:
> > This patch adds a new feature VIRTIO_NET_F_NOTF_COAL_LOW_HIGH,
> > and adds 2 new commands to VIRTIO_NET_CTRL_NOTF_COAL class:
> > * VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET - create a low-rate set of
> > coalescing parameters. the device should switch to this set when
> > the packet rate (packets per second) reaches the pkt_rate_low threshold.
> >
> > * VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET - create a high-rate set of
> > coalescing parameters. the device should switch to this set when
> > the packet rate (packets per second) reaches the pkt_rate_high threshold.
> >
> > A device may have up to 3 sets, and should switch between them based on
> > the packet rate.
> >
> > Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
>
> there seems to be a concept of a "set of coalescing parameters".
> if we use it we'll have to define it, I don't think it's a standard
> thing. I personally don't think we should bother, we currently
> talk about "coalescing parameters".
ok, I'll replace the set concept with "coalescing parameters".
> I propose unifying virtio_net_ctrl_coal_rx and virtio_net_ctrl_coal_tx
> since they are the same anyway. Then reuse.
Sure, but since both structs are already defined in the spec,
shouldn't we fix it in a separate patch?
Do you want me to propose a new patch that unifies the structs?
> > ---
> > content.tex | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 60 insertions(+), 1 deletion(-)
> >
> > diff --git a/content.tex b/content.tex
> > index 96f4723..969f945 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -3088,6 +3088,8 @@ \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_NOTF_COAL_LOW_HIGH(50)] Device supports notifications coalescing low rate and high rate sets.
> > +
> > \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
> >
> > \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> > @@ -3142,6 +3144,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > \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_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
> > +\item[VIRTIO_NET_F_NOTF_COAL_LOW_HIGH] Requires VIRTIO_NET_F_NOTF_COAL.
> > \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}
> > @@ -4504,9 +4507,25 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > le32 tx_usecs;
> > };
> >
> > +//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
>
> space after // pls. It's also unclear to put this comment
> before struct. a struct is just a struct, a comment such as this
> can only refer to fields in structs.
ok, I'll drop the comment from its current place
> > +struct virtio_net_ctrl_coal_low {
> > + le32 pkt_rate_low;
> > + struct virtio_net_ctrl_coal_tx tx;
> > + struct virtio_net_ctrl_coal_rx rx;
> > +};
>
> is it really true we always want the same rate for tx and rx?
> why not separate commands for each? this is the way we went
> for general coalescing at least.
I actually had ethtool in mind while writing the patch.
AFAIK ethtool has a single rate for both rx and tx low/high, so using
ethtool to manage the parameters won't be possible.
I know that we should worry about that when trying to implement the
feature and not while proposing it to the spec, but I think that we
will benefit the most from this feature if it can be used with an
existing, well known tool such as ethtool.
What do you think?
> > +
> > +//Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> > +struct virtio_net_ctrl_coal_high {
> > + le32 pkt_rate_high;
> > + struct virtio_net_ctrl_coal_tx tx;
> > + struct virtio_net_ctrl_coal_rx rx;
> > +};
> > +
>
> given these are identical, do we want to unify these?
> maybe virtio_net_ctrl_coal_threshold ?
ok.
> > #define VIRTIO_NET_CTRL_NOTF_COAL 6
> > #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> > #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> > + #define VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET 2 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> > + #define VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET 3 //Only if VIRTIO_NET_F_NOTF_COAL_LOW_HIGH negotiated
> > \end{lstlisting}
> >
> > Coalescing parameters:
> > @@ -4515,13 +4534,17 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > \item \field{tx_usecs}: Maximum number of usecs to delay a TX notification.
> > \item \field{rx_max_packets}: Maximum number of packets to receive before a RX notification.
> > \item \field{tx_max_packets}: Maximum number of packets to send before a TX notification.
> > +\item \field{pkt_rate_low}: Threshold for low packet rate set (packets per second).
>
> what does "set" mean here?
I'll replace it with" coalescing parameters", as suggested in your
first comment.
> and I'd take (packets per second) out of ():
> low packet rate, in units of packets per second.
ok.
> > +\item \field{pkt_rate_high}: Threshold for high packet rate set (packets per second).
> > \end{itemize}
> >
> >
> > -The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
> > +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> > \begin{enumerate}
> > \item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and \field{tx_max_packets} parameters.
> > \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and \field{rx_max_packets} parameters.
> > +\item VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_low} or less packets per second.
>
> set repeated twice here. confusing to the point where I couldn't figure out the meaning exactly.
> for a rate of \field{pkt_rate_low} or less packets per second
> do you mean
> when packet rate is \field{pkt_rate_low} or less?
Yes, I'll fix the wording and replace the second "set" with
"coalescing parameters".
> > +\item VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET: set the \field{rx_usecs}, \field{rx_max_packets}, \field{tx_usecs} and \field{tx_max_packets} set for a rate of \field{pkt_rate_high} or more packets per second.
> > \end{enumerate}
> >
> > \subparagraph{RX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Notifications}
> > @@ -4554,18 +4577,54 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
> > \end{itemize}
> >
> > +\subparagraph{Low/High Rate Notifications Sets}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Low/High Rate Notifications Sets}
> > +
> > +If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature is negotiated, the driver may send VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands to the device.
> > +
> > +The virtio_net_ctrl_coal_low struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or lower than \field{pkt_rate_low} packets per second.
>
> packets rate -> packet rate
> here and elsewhere
ok.
> > +
> > +The virtio_net_ctrl_coal_high struct defines a set of coalescing parameters (\field{rx_usecs}, \field{tx_usecs}, \field{rx_max_packets} and \field{tx_max_packets}) that should be used when the packets rate is equal or higher than \field{pkt_rate_high} packets per second.
>
> equal to or higher than
ok.
> > +
> > +A device may have up to 3 sets of coalescing parameters, and should switch between the sets based on the packet rate.
>
> what are the 3 sets?
The high set, the low set and the normal set, I'll add it to the sentence.
> avoid "may" outside conformance sections please.
ok.
> > +A device may have only a low rate set, in this case, it should coalesce notifications only when the packet rate crosses down the \field{pkt_rate_low} threshold.
> > +
> > +A device may have only a high rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold.
> > +
> > +A device may have only a high rate set and a low rate set, in this case, it should coalesce notifications only when the packet rate crosses up the \field{pkt_rate_high} threshold, or crosses down the \field{pkt_rate_low} threshold.
>
> in this case it's not both.
Do you mean that I should replace "A device may have only a high rate
set and a low rate set" with "A device may have both a high rate set
and a low rate set"?
I wanted to emphasize that it may have a low set and a high set,
without a normal set, this is why I wrote "only", but I can fix the
wording.
> > +
> > \drivernormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
> >
> > If the VIRTIO_NET_F_NOTF_COAL feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL commands.
> >
> > +If the VIRTIO_NET_F_NOTF_COAL_LOW_HIGH feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET/VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET commands.
> > +
> > +The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 in order to remove the low rate set.
> > +
> > +The driver SHOULD issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 in order to remove the high rate set.
> > +
> > +The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET was issued.
> > +
> > +The driver MUST NOT issue a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET was issued.
> > +
> > \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
> >
> > A device SHOULD respond to the VIRTIO_NET_CTRL_NOTF_COAL commands with VIRTIO_NET_ERR if it was not able to change the parameters.
> >
> > A device SHOULD NOT send used buffer notifications to the driver, if the notifications are suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, even if the coalescing counters expired.
> >
> > +The device MUST remove the low rate set if a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} 0 is received.
> > +
> > +The device MUST remove the high rate set if a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} 0 is received.
> > +
> > +The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_LOW_SET command with \field{pkt_rate_low} equal or higher than \field{pkt_rate_high}, if a high rate set is being used.
> > +
> > +The device MUST respond with VIRTIO_NET_ERR to a VIRTIO_NET_CTRL_NOTF_COAL_HIGH_SET command with \field{pkt_rate_high} equal or lower than \field{pkt_rate_low}, if a low rate set is being used.
> > +
> > Upon reset, a device MUST initialize all coalescing parameters to 0.
> >
> > +Upon reset, a device MUST not have a low rate and high rate sets.
> > +
>
>
> how about documenting how the coalescing is used.
> E.g. is it best effort or mandatory?
ok, I can add a line stating that this is a best effort feature.
> I also noticed that coalescing is currently under-specified with
> respect to delivering interrupts when config changes.
> For example, what happens if I get 1 packet then lower max packets to 1?
> Do I get an interrupt immediately or does the counter get reset?
I think that the device should check the counters when coalescing
parameters change, and should deliver an interrupt if needed.
But since this affects an existing feature, shouldn't it be fixed in a
separate patch?
> > \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > Types / Network Device / Legacy Interface: Framing Requirements}
> >
> > --
> > 2.32.0
>
Alvaro
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-comment] Re: [PATCH] virtio_net: support low and high rate of notification coalescing sets
2023-01-04 18:40 ` Alvaro Karsz
@ 2023-01-05 7:32 ` Alvaro Karsz
0 siblings, 0 replies; 9+ messages in thread
From: Alvaro Karsz @ 2023-01-05 7:32 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-comment, virtio-dev, jasowang
> > is it really true we always want the same rate for tx and rx?
> > why not separate commands for each? this is the way we went
> > for general coalescing at least.
>
> I actually had ethtool in mind while writing the patch.
> AFAIK ethtool has a single rate for both rx and tx low/high, so using
> ethtool to manage the parameters won't be possible.
>
> I know that we should worry about that when trying to implement the
> feature and not while proposing it to the spec, but I think that we
> will benefit the most from this feature if it can be used with an
> existing, well known tool such as ethtool.
>
> What do you think?
ok, we actually could separate tx and rx, and send both commands when
ethtool is used to set low/high rate.
I will fix the patch to have different rx and tx commands.
Alvaro
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-01-05 7:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-21 10:17 [virtio-dev] [PATCH] virtio_net: support low and high rate of notification coalescing sets Alvaro Karsz
2022-12-21 11:08 ` [virtio-comment] " Xuan Zhuo
2022-12-21 11:48 ` Alvaro Karsz
2022-12-21 12:45 ` Heng Qi
2022-12-21 13:58 ` Xuan Zhuo
2023-01-04 14:27 ` [virtio-dev] " Alvaro Karsz
2023-01-04 15:13 ` [virtio-comment] " Michael S. Tsirkin
2023-01-04 18:40 ` Alvaro Karsz
2023-01-05 7:32 ` Alvaro Karsz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox