Re: [virtio-comment] [PATCH v2] Introduction of Virtio Network device notifications coalescing feature

[Jason Wang <jasowang@redhat.com>]

On Wed, May 18, 2022 at 8:08 PM Alvaro Karsz <alvaro.karsz@solid-run.com> wrote:
>
> > I don't want to
> > distract you though, I think the virtio-net-level coalescing makes sense
> > since it counts Ethernet frames rather than virtqueue buffers.
>
> I'm not sure it should count ethernet frames, I think that it should
> count descriptors.
> Assume the following example:
>
> tx-max-frames = 5
> The driver wants to send a 64k packet, and VIRTIO_NET_F_HOST_TSO4 is negotiated.
> The driver will write a 64K description on the descriptor area.
> The device will segment the payload and will send more than 40 frames
> (with standard MTU) in a very short time (so the timer is not a
> factor).
> The device will send more than 8 notifications to the driver for just
> 1 used descriptor.

I think you meant 1 used buffer actually, the notification was checked
and raised when the device had a new used buffer. In your case without
coalsing we may have at most 1 notification.

Having 8 notifications doesn't make much sense since there's no used
buffer until the last notification.

>
> I think that speaking of descriptors instead of ethernet frames makes
> more sense.

So here is the doc for ethtool_coalesce:

 * @rx_max_coalesced_frames: Maximum number of packets to receive
 *      before an RX interrupt.

It counted by packets. So counting the packets makes more sense.
Especially considering we have mergeable rx buffers where a single
packet can consume multiple buffers.

>
> This is relevant for RX as well.
> Assume that VIRTIO_NET_F_GUEST_TSO4 is negotiated, and
> VIRTIO_NET_F_MRG_RXBUF is not.
> We could receive many frames that will be written in the same
> descriptor chain, and we are in the same situation as described for
> TX.
>
>
> I think that this feature should be at a "higher level".
>
> The device operates normally, ignoring the coalescing parameters.
>
> If the device "decides" that it should send a notification according
> to VirtIO "rules" (before checking if the events are suppressed by the
> driver), it will then increase the internal coalescing counters, and
> decide if it should send the notification.
> So, in the case of GSO, it will increase the internal tx-fames counter
> by 1 if the descriptor is of a 64B packet, or a 64KB  packet.

I'm not sure I understand here. I think the right one is to increase
the counter by 1 after the device receives a packet no matter how many
descriptors/buffers are used for this packet.

Thanks

>
>
> What do you think?
>