[virtio-dev] Re: [virtio-comment] Re: [PATCH v2] virtio-net: support setting coalescing params for multiple vqs
Heng Qi <[email protected]>
| Newsgroups | dev.linux.lists.virtio-dev,dev.linux.lists.virtio-comment |
|---|---|
| Message-ID | <[email protected]> |
在 2024/1/24 下午6:52, Michael S. Tsirkin 写道: > On Wed, Jan 24, 2024 at 05:50:28AM -0500, Michael S. Tsirkin wrote: >> On Mon, Jan 15, 2024 at 09:06:02PM +0800, Heng Qi wrote: >>> Currently, when each time the driver attempts to update the coalescing parameters >>> for a vq, it needs to kick the device and wait for the ctrlq response to return. >>> The following path is observed: 1. Driver kicks the device; 2. After the device >>> receives the kick, CPU scheduling occurs and DMA multiple buffers multiple times; >>> 3. The device completes processing and replies with a response. >>> >>> When large-queue devices issue multiple requests and kick the device frequently, >>> this often interrupt the work of the device-side CPU. In addition, each vq request >>> is processed separately, causing more delays for the CPU to wait for the DMA >>> request to complete. >>> >>> These interruptions and overhead will strain the CPU responsible for controlling >>> the path of the DPU, especially in multi-device and large-queue scenarios. >>> >>> To solve the above problems, we internally tried batch request, which merges >>> requests from multiple queues and sends them at once. We conservatively tested >>> 8 queue commands and sent them together. The DPU processing efficiency can be >>> improved by 8 times, which greatly eases the DPU's support for multi-device >>> and multi-queue DIM. >>> >>> Maintainers may be concerned about whether the batch command method can optimize >>> the above problems: accumulate multiple request commands to kick the device once, >>> and obtain the processing results of the corresponding commands asynchronously. >>> The batch command method is used by us to optimize the CPU overhead of the DIM >>> worker caused by the guest being busy waiting for the command response result. >>> This is a different focus than batch request to solve the problem. >>> >>> Suggested-by: Xiaoming Zhao <[email protected]> >>> Signed-off-by: Heng Qi <[email protected]> >>> --- >>> v1->v2: Updated commit log. Due to sensitivity, sorry that can not give the >>> absolute value directly. @Michael >>> >>> device-types/net/description.tex | 26 ++++++++++++++++++++------ >>> 1 file changed, 20 insertions(+), 6 deletions(-) >>> >>> diff --git a/device-types/net/description.tex b/device-types/net/description.tex >>> index aff5e08..b3766c4 100644 >>> --- a/device-types/net/description.tex >>> +++ b/device-types/net/description.tex >>> @@ -1667,8 +1667,8 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi >>> for notification coalescing. >>> >>> If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated, the driver can >>> -send commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET >>> -for virtqueue notification coalescing. >>> +send commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET and >>> +VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET for virtqueue notification coalescing. >>> >>> \begin{lstlisting} >>> struct virtio_net_ctrl_coal { >>> @@ -1682,11 +1682,17 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi >>> struct virtio_net_ctrl_coal coal; >>> }; >>> >>> +struct virtio_net_ctrl_mrg_coal_vq { >>> + le16 num_entries; /* indicates number of valid entries */ >>> + struct virtio_net_ctrl_coal_vq entries[]; >>> +}; >>> + >>> #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_VQ_SET 2 >>> #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3 >>> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET 4 >>> \end{lstlisting} >>> >>> Coalescing parameters: >>> @@ -1706,6 +1712,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi >>> \item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, the structure virtio_net_ctrl_coal_vq is write-only for the driver. >>> \item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vq_index} and \field{reserved} are write-only >>> for the driver, and the structure virtio_net_ctrl_coal is read-only for the driver. >>> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET, the structure virtio_net_ctrl_mrg_coal_vq is write-only for the driver. >>> \end{itemize} >>> >>> The class VIRTIO_NET_CTRL_NOTF_COAL has the following commands: >>> @@ -1716,6 +1723,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi >>> for an enabled transmit/receive virtqueue whose index is \field{vq_index}. >>> \item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets} parameters >>> for an enabled transmit/receive virtqueue whose index is \field{vq_index}. >>> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET: use the structure virtio_net_ctrl_mrg_coal_vq to set the \field{max_usecs} and \field{max_packets} parameters >>> + for \field{num_entries} enabled transmit/receive virtqueues. The corresponding index value >>> + of each configured virtqueue is \field{vq_index}. >>> \end{enumerate} >>> >>> The device may generate notifications more or less frequently than specified by set commands of the VIRTIO_NET_CTRL_NOTF_COAL class. >>> @@ -1782,9 +1792,13 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi >>> >>> The driver MUST set \field{vq_index} to the virtqueue index of an enabled transmit or receive virtqueue. >>> >>> +The driver MUST set \field{num_entries} to a non-zero value and MUST NOT set \field{num_entries} to >>> +a value greater than the number of enabled transmit and receive virtqueues. >>> + >>> The driver MUST have negotiated the VIRTIO_NET_F_NOTF_COAL feature when issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET. >>> >>> -The driver MUST have negotiated the VIRTIO_NET_F_VQ_NOTF_COAL feature when issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET. >>> +The driver MUST have negotiated the VIRTIO_NET_F_VQ_NOTF_COAL feature when issuing commands >>> +VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET. >>> >>> The driver MUST ignore the values of coalescing parameters received from the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if the device responds with VIRTIO_NET_ERR. >>> >>> @@ -1794,10 +1808,10 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi >>> >>> The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was not able to change the parameters. >>> >>> -The device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command with VIRTIO_NET_ERR if it was not able to change the parameters. >>> +The device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET commands with VIRTIO_NET_ERR if it was not able to change the parameters. >> This one however is vague. >> In fact the addition of VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET made >> VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET >> also vague we just missed this. >> >> >> >> So, it is not clear whether if the device responds with VIRTIO_NET_ERR - it can change >> parameters for some VQs but not others, or does it have to either >> change parameters for all VQs or none at all. >> >> I think a better sematic is all or nothing. >> >> So I suggest combining this statement with one of >> VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, >> and adding something like >> "in this case, coalescing parameters MUST remain unchanged, >> for all VQs". >> >> This bug bothers me. Maybe we should fix it before release? >> >> I also don't know why we repeat same text for VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, >> VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET. > I figured it out, TX/RX are SHOULD. > I will fix TX/RX and you can add the text to this one. Sorry Michael, it seems I missed this email yesterday. I will add this. Thanks, Heng > > >>> >>> -The device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with >>> -VIRTIO_NET_ERR if the designated virtqueue is not an enabled transmit or receive virtqueue. >>> +The device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET >>> +commands with VIRTIO_NET_ERR if the designated virtqueue is not an enabled transmit or receive virtqueue. >>> >>> Upon disabling and re-enabling a transmit virtqueue, the device MUST set the coalescing parameters of the virtqueue >>> to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or, if the driver did not set any TX coalescing parameters, to 0. >>> -- >>> 1.8.3.1 > > 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: [email protected] > Unsubscribe: [email protected] > List help: [email protected] > 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/ --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]