Re: [virtio-comment] Re: [PATCH v6 2/5] virtio-net: Add flow filter capabilities read commands

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

On Mon, Nov 27, 2023 at 02:06:27PM +0100, Cornelia Huck wrote:
> >> > > > 4. provides consistent structures that provisioning side will be
> >> > > > able to use
> >> > >
> >> > > Problem for provisioning is extra definitions will be needed, in a
> >> > > device specific way.
> >> > In vdpa tool and other OS tools of iproute2 developed, setting and getting
> >> those device specific values are useful.
> >> > It is ok.
> >> 
> >> It does not become ok just by saying so. You are taking a single RO value and
> >> instead of it having an address there are now 2 other ways to address it. And
> >> you fail to see the problem and the pain you are inflicting on software
> >> developers. Just stick with an address if you can.
> > There is zero problem with sw.
> > Sw just need to issue send_command() and done with it, like rest of the commands.
> > A pain would be create yet another DMA interface.
> >
> >> 
> >> > >
> >> > > > > > Nor do I see any enforcement, single method via cvq still holds strong.
> >> > > > >
> >> > > > > You don't need to enforce things, if people want to put a lot of
> >> > > > > RAM on device and put it in a register let them.
> >> > > > >
> >> > > > Not enforced. It uses the CVQ for flow group and flow filter life
> >> > > > cycles and for
> >> > > the sharing this config as well.
> >> > > > Also aligns with stats that rest also agreed on.
> >> > >
> >> > > I am talking about your attempt to generally say "no more config
> >> > > fields everything must be in CVQ".
> >> > Config fields for initialization time is fine as the spec allows it today.
> >> > Things which can differ, it is ok to use cvq interface.
> >> 
> >> I don't know what does "Things that can differ" means. Generally device caps
> >> are perfect for config space. Accessed at init time only, RO.
> >> 
> > You ignore the comment I answered before that proposal here is not based on RO/RW.
> > It is based on initialization time vs run time.
> >
> >> > > I think it's wrong definitiely for non network devices must
> >> > > sometimes for network too and generally we need a solution for
> >> > > config over DMA. This specific thing - whether it fits in CVQ is a
> >> > > separate discussion.
> >> > >
> >> > I explained it before, that 6 out of 19 devices has cvq which are complex
> >> enough doing things over cvq.
> >> > These are non-network devices already.
> >> >
> >> > If one of those remaining device becomes complex, it is likely it will need a
> >> cvq to suffice for the dma interface and it can just do with depth = 1.
> >> 
> >> Using generic caps and not net specific ones is a good idea.
> >> 
> > context here is cvq and net.
> >
> >> 
> >> 
> >> > >
> >> > > > > >
> >> > > > > > >
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > > The method proposed here is elegant and clearly promote
> >> > > > > > > > one way to do
> >> > > > > > > things for driver and device with predictability.
> >> > > > > > > >
> >> > > > > > >
> >> > > > > > > I don't see it as elegant at all. What is elegant is *a
> >> > > > > > > single
> >> > > > > > > tag* that describes each property of the device. And this
> >> > > > > > > single tag should be
> >> > > > > good for everything:
> >> > > > > > > driver, provisioning, migration. And config space offset serves as
> >> such.
> >> > > > > > The single tag is the set of structures.
> >> > > > >
> >> > > > > I have no idea how this will work. If migration format i started
> >> > > > > reviewing is anything to go by then there will be a huge
> >> > > > > elaborate structure nothing single or simple. By comparison
> >> > > > > there's already a proposal how provisioning can work by supplying
> >> config space.
> >> > > > > it is just a clean model to grasp.
> >> > > > >
> >> > > > The provisioning model is simple is to supply all the configuration.
> >> > > > To draw parallels to some sw side,
> >> > > >
> >> > > > There is per functionality socket option to set things, instead of
> >> > > > one giant
> >> > > structure.
> >> > > > There is per functionality ethtool option/cmd instead of Set
> >> > > > ALL/get ALL
> >> > > enforcement.
> >> > >
> >> > > I'm not sure how much of a parallel one can draw.
> >> > > Do not see a lot of similarity.
> >> > For lot of configuration they are similar that happens at slow pace.
> >> >
> >> > > Devices commonly use register map. Everyone understands this paradigm.
> >> > >
> >> > For initialization early device setup time, yes.
> >> >
> >> > >
> >> > > I am not altogether happy with the way you are making migration
> >> > > generate duplicate definitions for lots of things we already have definitions
> >> for.
> >> > > Having a 3rd one for provisioning? Gimme a break.
> >> >
> >> > For migration, we are not duplicating. Some structures are not well defined,
> >> it has some duplication.
> >> 
> >> And fyi it's already making people unhappy.
> >> 
> > Those exceptions are not the interesting one to take as example here.
> >
> >> > But large part seems be able to utilized pre-defined structs.
> >> > And here for flow filter also same structs will be used.
> >> 
> >> So if there's a 64 bit bitmap in config space, then provisioning command which
> >> already gets config space can just use its offset.
> >> Simpler, better.
> >> 
> > It is not simple to implement per device unique config space as we discussed already.
> > And no need another DMA interface either as cvq service that need already.
> 
> A general injection from me.
> 
> We're seeing more and more of those monster threads on the list, where
> everything is going in circles, sometimes spawning new versions, which
> create their own monster threads... I really dread looking at
> virtio-comment nowadays. This is drowning out other things on the list,
> which have a tendency to just get lost in the noise.
> 
> We should stop and think, figure out why that happens, and how we can
> get back to a productive environment.
> 
> >From what I've seen in this discussion here (and it seems to mirror
> other discussions I've browsed), it seems to boil down to narrow
> viewpoints and problematic ways of discussing the design.
> 
> Is PCI important? Of course. Is virtio-net important? Nobody disputes
> that. Are hardware implementations something we want to support well?
> Sure. But that does not mean other use cases are not important as
> well. Having a particular use case in mind is completely fine, treating
> other use cases as second-class citizens is not.
> 
> What has also stuck out to me in many of those discussions is something
> I'll call "argument by assertion", i.e. statements that something is
> this-and-that, without qualifiers like "I think that..." or "we've seen
> in other instances that...". From what I've seen, this tends to make
> people on the other side of the argument defensive instead of
> considering the arguments -- it certainly does not seem to foster a
> productive discussion, or at least that's my impression. I've also seen
> a tendency to reduce arguments to absolutes, which tends to bring out
> defensiveness as well.
> 
> Last but not least, I don't think rapid posting is helping -- it rather
> just seems to speed up ping-pong posting with each side becoming
> entrenched in their point of view even more. It's certainly not
> pleasurable to try to follow these threads. Can we try to calm down a
> bit, please?


Thanks for bringing this up Cornelia, I agree and think I've been
guilty of being too assertive and posting too fast recently, too.
I'll try to address that.

Thanks,
-- 
MST