Re: [PATCH net-next v4] net: virtio_net: notifications coalescing support

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jul 20, 2022 at 10:07:11AM +0300, Alvaro Karsz wrote:
> > Hmm. we currently (ab)use tx_max_coalesced_frames values 0 and 1 to mean tx
> napi on/off.
> > However I am not sure we should treat any value != 1 as napi on.
> >
> > I don't really have good ideas - I think abusing coalescing might
> > have been a mistake. But now that we are there, I feel we need
> > a way for userspace to at least be able to figure out whether
> > setting coalescing to 0 will have nasty side effects.
> 
> 
> So, how can I proceed from here?
> Maybe we don't need to use tx napi when this feature is negotiated (like Jakub
> suggested in prev. versions)?
> It makes sense, since the number of TX notifications can be reduced by setting
> tx_usecs/tx_max_packets with ethtool.


Hmm Jason had some ideas about extensions in mind when he
coded the current UAPI, let's see if he has ideas.
I'll ruminate on compatibility a bit too.

> > It's also a bit of a spec defect that it does not document corner cases
> > like what do 0 values do, are they different from 1? or what are max values.
> > Not too late to fix?
> 
> 
> I think that some of the corner cases can be understood from the coalescing
> values.
> For example:
> if rx_usecs=0 we should wait for 0 usecs, meaning that we should send a
> notification immediately.
> But if rx_usecs=1 we should wait for 1 usec.
> The case with max_packets is a little bit unclear for the values 0/1, and it
> seems that in both cases we should send a notification immediately after
> receiving/sending a packet.
> 
> 
> > So the spec says
> >         Device supports notifications coalescing.
> >
> > which makes more sense - there's not a lot guest needs to do here.
> 
> 
> Noted.
> 
> > parameters?
> 
>  
> I'll change it to "settings".
> 
> > why with dash here? And why not just put the comments near the fields
> > themselves?
> 
> 
> Noted.