Re: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash

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

On Wed, Jun 28, 2023 at 05:38:29PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, June 28, 2023 1:24 PM
> 
> 
> > > Because when device has two ways to access config space, it always have to
> > account and build the interface that, hey some driver will not use DMA.
> > > Hence have it always in the MMIO accessible area.
> > 
> > Maybe I get it. You want to use the new features as a carrot to force drivers to
> > implement DMA? You suspect they will ignore the spec requirement just
> > because things seem to work?
> >
> Right because it is not a must normative.

Well SHOULD also does not mean "ok to just ignore".

	This word, or the adjective "RECOMMENDED", mean that there
	   may exist valid reasons in particular circumstances to ignore a
	   particular item, but the full implications must be understood and
	   carefully weighed before choosing a different course.



> > There's some logic here, for sure. you just might be right.
> > 
> > However, surely we can discuss this small tweak in 1.4 timeframe?
> 
> Sure, if we prefer the DMA approach I don't have a problem in adding temporary one field to config space.
> 
> I propose to add a line to the spec " Device Configuration Space" section, something like,
> 
> Note: Any new device configuration space fields additional MUST consider accessing such fields via a DMA interface.
> 
> And this will guide the new patches of what to do instead of last moment rush.

Yea, except again I'd probably make it a SHOULD: e.g. I can see how
switching to MMIO might be an option for qemu helping us debug DMA
issues.

The time to discuss this detail would be around when proposal for the DMA
access to config space is on list though: I feel this SHOULD vs MUST
is a small enough detail.

Going back to inner hash. If we move supported_tunnels back to config space,
do you feel we still need GET or just drop it? I note we do not
have GET for either hash or rss config.

And if we no longer have GET is there still a reason for a separate
command as opposed to a field in virtio_net_hash_config?
I know this was done in v11 but there it was misaligned.
We went with a command because we needed it for supported_tunnels
but now that is no longer the case and
there are reserved words in virtio_net_hash_config ...

Let me know how you feel it about that, not critical for me.


-- 
MST