Re: [PATCH] virtio-net: Update the max_tx_vq description

On Sat, Apr 19, 2025 at 2:22 PM Akihiko Odaki <[email protected]> wrote:
>
> On 2025/04/16 16:38, Jason Wang wrote:
> > On Fri, Apr 11, 2025 at 3:41 PM Akihiko Odaki <[email protected]> wrote:
> >>
> >> On 2025/04/08 15:33, Jason Wang wrote:
> >>> On Mon, Apr 7, 2025 at 11:43 AM Akihiko Odaki <[email protected]> wrote:
> >>>>
> >>>> On 2025/04/07 11:41, Jason Wang wrote:
> >>>>> On Sat, Mar 29, 2025 at 4:32 PM Akihiko Odaki <[email protected]> wrote:
> >>>>>>
> >>>>>> On 2025/03/24 13:02, Jason Wang wrote:
> >>>>>>> On Fri, Mar 21, 2025 at 3:01 PM Akihiko Odaki <[email protected]> wrote:
> >>>>>>>>
> >>>>>>>> The previous description said:
> >>>>>>>>> A driver sets \field{max_tx_vq} to inform a device how many transmit
> >>>>>>>>> virtqueues it may use (transmitq1\ldots transmitq \field{max_tx_vq}).
> >>>>>>>>
> >>>>>>>> However, it also controls how many receive virtqueues a device MAY use.
> >>>>>>>> Copy the description of the virtqueue_pairs field of struct
> >>>>>>>> virtio_net_ctrl_mq_pairs_set, which also has the identical semantics, to
> >>>>>>>> clarify that.
> >>>>>>>
> >>>>>>> Thanks for doing this, I wonder if it's time to rename as "max_tx_vq"
> >>>>>>> sounds like a TX only thing.
> >>>>>>
> >>>>>> I'm no longer pursuing this change. Please see the discussion at:
> >>>>>> https://lore.kernel.org/r/[email protected]
> >>>>>>
> >>>>>> Please also review a change for QEMU based on the discussion:
> >>>>>> https://lore.kernel.org/qemu-devel/[email protected]
> >>>>>
> >>>>> Ok, so it seems that RSS implement a async RX/TX queue model instead
> >>>>> of the MQ queue pairs:
> >>>>>
> >>>>> 1) rx available queue were controlled by indirection table
> >>>>> 2) tx available queue were controlled by max_tx_vq
> >>>>>
> >>>>> So it introduces more "issues" like:
> >>>>>
> >>>>> 1) This calls a new uAPI as I suggested in another thread instead of
> >>>>> reusing TUN_SET_QUEUE
> >>>>
> >>>> QEMU applies the same approach for both VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET
> >>>> and VIRTIO_NET_CTRL_MQ_RSS_CONFIG, which is to parse the configuration
> >>>> passed by the guest in QEMU and to pass the parsed configuration to TUN
> >>>> via TUNSETQUEUE. Please see the patch for QEMU I mentioned in the
> >>>> previous email for the actual implementation.
> >>>
> >>> If this is true, even if guests want to have an async tx/rx queue
> >>> number, the underlayer TAP will still choose a combined queue pair
> >>> model.
> >>>
> >>>>
> >>>>> 2) If both MQ and RSS are negotiated, what happens if the driver is
> >>>>> using both max_tx_vq and virtqueue_pairs. This seems unclear in the
> >>>>> spec itself.
> >>>>
> >>>> The spec says the last one sent by the driver will be used. In version
> >>>> 1.2, 5.1.6.5.5 "Device operation in multiqueue mode" says:
> >>>>    > The driver enables multiqueue by sending a command using class
> >>>>    > VIRTIO_NET_CTRL_MQ. The command selects the mode of multiqueue
> >>>>    > operation, as follows:
> >>>>    >  #define VIRTIO_NET_CTRL_MQ    4
> >>>>    >  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0 (for automatic
> >>>>    >  receive steering)
> >>>>    >  #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1 (for configurable
> >>>>    >  receive steering)
> >>>>    >  #define VIRTIO_NET_CTRL_MQ_HASH_CONFIG         2 (for configurable
> >>>>    >  hash calculation)
> >>>>    >
> >>>>    > If more than one multiqueue mode is negotiated, the resulting device
> >>>>    > configuration is defined by the last command sent by the driver.
> >>>
> >>> I think it doesn't clarify all the cases. Consider we:
> >>>
> >>> 1) configure flow A to use rx queue 4
> >>> 2) set curr_queue_paris to 2
> >>>
> >>> How to choose the rx queue in flow A? This needs some clarification
> >>> e.g whether unclassified or automq or drop
> >>
> >> Such a configuration is not possible. If VIRTIO_NET_CTRL_MQ_RSS_CONFIG
> >> configures flow A to use rx queue 4, the resulting number of queue pairs
> >> will be at least 5.
> >
> > Yes, this could be prevented by a well formed driver. But I mean from
> > the device side, such commands could be sent from the driver that is
> > something both spec and the implementation (e.g TAP) needs to
> > consider.
>
> If VIRTIO_NET_CTRL_MQ_RSS_CONFIG is the last command, it must define the
> number of queue pairs since the spec says the resulting device
> configuration is defined by the last command sent by the driver. So if
> VIRTIO_NET_CTRL_MQ_RSS_CONFIG configures flow A to use rx queue 4, the
> command also define that the number of queue pairs to be at least 5.
> Therefore, it is not possible for a driver to define the number of queue
> pairs contradicting with the flow configuration.

Yes, but I meant what happens if there's a
VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET afterwards that sets queue pairs to 2.
In this case, should the flow go with which queue?

Thanks

>
> Regards,
> Akihiko Odaki
>
> >
> > Thanks
> >
> >
> >>
> >> Regards,
> >> Akihiko Odaki
> >>
> >>>
> >>> Thanks
> >>>
> >>>>
> >>>> Regards,
> >>>> Akihiko Odaki
> >>>>
> >>>>>
> >>>>> THanks
> >>>>>
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Signed-off-by: Akihiko Odaki <[email protected]>
> >>>>>>>> ---
> >>>>>>>>      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 76585b0..5178191 100644
> >>>>>>>> --- a/device-types/net/description.tex
> >>>>>>>> +++ b/device-types/net/description.tex
> >>>>>>>> @@ -1578,7 +1578,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> >>>>>>>>
> >>>>>>>>      Field \field{indirection_table} is an array of receive virtqueues ids.
> >>>>>>>>
> >>>>>>>> -A driver sets \field{max_tx_vq} to inform a device how many transmit virtqueues it may use (transmitq1\ldots transmitq \field{max_tx_vq}).
> >>>>>>>> +Field \field{max_tx_vq} specifies the number of the transmit and receive queues
> >>>>>>>> +to be used up to \field{max_virtqueue_pairs}; subsequently, transmitq1\ldots
> >>>>>>>> +transmitqn and receiveq1\ldots receiveqn where n=\field{max_tx_vq} MAY be used.
> >>>>>>>>
> >>>>>>>>      Fields \field{hash_key_length} and \field{hash_key_data} define the key to be used in hash calculation.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> ---
> >>>>>>>> base-commit: b495841a8e80d12c1130f8868f4128866291142d
> >>>>>>>> change-id: 20250321-tx-78e44c50a035
> >>>>>>>>
> >>>>>>>> Best regards,
> >>>>>>>> --
> >>>>>>>> Akihiko Odaki <[email protected]>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
>