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

[Alvaro Karsz <alvaro.karsz@solid-run.com>]

> The description here looks more like GUEST_RSC instead of GUEST_GSO.
> The difference is GUEST_GSO doesn't coalesce packets.

If I understand correctly, VIRTIO_NET_F_GUEST_TSO/UFO means that the
device should coalesce ethernet frames into packets, and that's what I
meant.
Something like GRO.
Which part is not clear in the patch?
Maybe the "received frames" part?
I meant received from the "outside world" and not from the host.


>
> So we need clarify the "packet" definition:
>
> 1) is it the packet that the device saw on the wire (before coalescing)
>
> or
> 2) it's the packet that has been coalesced by the the device and put
> in the buffer
> And when the device is expected to increase the counter


I think that it should be based on ethernet frames (1).
So, the driver should increase the packet counter after
receiving/sending an ethernet frame.
Meaning that the counter may be increased many times handling just one
driver buffer (if GSO/GRO are negotiated).
This is why I added the following part:

> +\item The device will check if at least one descriptor was used from the descriptor area, if not, it will continue to accumulate frames until one descriptor is used.\\
> +An example is if any of the VIRTIO_NET_F_GUEST_TSO/UFO features are negotiated, a device could receive more than 15 frames, and write all in the same buffer.
> +\item The device will reset its internal coalescing counters.




> I think we need first understand what's the advantage of using packet
> based interrupt coalescing over event index?


I think that there are some advantages:
- More flexible, It's easier to configure the device to send
notifications every X packets/events.
- Allows you to set a timeout using the rx/tx-usecs parameters.
- For a HW device, it is more easy to get control commands through the
control virtqueue than polling the driver area using PCI transactions.

> And if the answer is yes, is it worthwhile to make coalescing and
> event index work in parallel?
>
> The notification is sent when any of the following condition is met:
>
> - If the idx field in the used ring (which determined where that
> descriptor index was placed) was equal to used_event
> - If we hit the coalescing limit
>
> For the driver that want to use interrupt coalescing only, it can choose:
>
> 1) avoid negotiating event index
>
> or
>
> 2) do not publish new used_event
>
> Thanks


In this case, the coalescing parameters could be set to 1
(tx/rx_frames_max) and it will neutralize the event_idx effect, since
a notification will be sent for every packet.
This could be done the other way around, the driver could set
event_idx to X, then increase it by 1 every notification, and it will
neutralize the coalescing.

In my opinion, the user should have control over the coalescing
parameters using ethtool, and the driver could suppress all
notifications, but I can't see what we'll benefit from allowing both
coalescing and EVENT_IDX to work in parallel.

What do you think?