Re: [PATCH v4] Introduction of Virtio Network device notifications coalescing feature.

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

> Considering RX may use multiple buffers per packet, I wonder if it's
> better to stick to "packet" here.

Noted.

> Does this work like: if we set rx_max_buffers to 10, it means after 1
> buffers is used, we still need another 10 buffers?

No, if  rx_max_buffers=10, we should send a notification on the #10
packet. I will clarify that.

> I think we should explicitly say mergeable rx buffers here. And that's
> why I think mentioning "packet is sent/received" is probably better
> than the buffer used here.

I meant when the host writes a packet in several buffers, and the NEXT
flag is set on the descriptor.
But you're right, I should mention the mergeable rx buffers as well.

> It's better to clarify it in the part of coalescing parameters, or we
> can just borrow the comments of kernel's ethtool_coalesce, then it
> should explain itself like:
>
>
> @rx_usecs: How many usecs to delay an RX notification after a packet arrives
> @rx_max_buffers: Maximum number of packets to receive before an RX notification
>
>
> Not a native speaker but then rx_usecs=0 it means the notification is
> sent immediately and by saying "maximum number" it means best effort.

Noted. I will move the coalescing parameters explanation to this subparagraph.

> Let's clarify what happens if notification is suppressed.

I will add that in this case, the device should not send a notification.

> I think there's probably no need to mention the implementation details
> like counters here. The definition of rx_usecs and rx_max_buffers
> should be sufficient.

Noted.

> What's the reason for this?

Because if (tx/rx)_buffers_max is bigger than the virtqueue size, this
condition will never be met.
Do you think that we should allow this to happen?

> I think we need use MUST here, something like:
>
>
> Upon reset, the device must present all zero values ...

Noted.