> 18/12/2018 20:34, Dumitrescu, Cristian:
> > From: Ananyev, Konstantin
> > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > 18/12/2018 14:19, Dumitrescu, Cristian:
> > > > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > > > 18/12/2018 12:18, Dumitrescu, Cristian:
> > > > > > > > > I replied in v3 that it should stay in rte_meter.h.
> > > > > > > > > You can include rte_meter.h in ethdev.
> > > > > > > >
> > > > > > > > OK, thanks Thomas, makes sense to me as well.
> > > > > > > >
> > > > > > >
> > > > > > > Thomas,
> > > > > > >
> > > > > > > I agree with your input, but just want to make sure we are on the
> > > same
> > > > > > page:
> > > > > > >
> > > > > > > Besides including rte_meter.h in ethdev (which you are fine
> > > > > > > with), we
> > > > > > would also need to include rte_meter.h in mbuf.
> > > > > > >
> > > > > > > Are you OK with this as well?
> > > > > >
> > > > > > Why do we need rte_meter.h in mbuf?
> > > > > >
> > > > >
> > > > > You probably looked at V2 only, but in V3 we have functions to set/get
> > > the color within the mbuf->hash.sched field.
> > > > >
> > > > > For space compression reasons, the mbuf->hash.sched stores the color
> > > on 8-bit variable, while for the outside world the set/get functions
> > > > work with the 32-bit enum type. We do same thing in other places in
> > > > DPDK,
> > > such as rte_crypto_op, etc.
> > > >
> > > > So it's a different discussion.
> > > > We need to review this v3 and check how relevant this mbuf API is.
> > > >
> > > > If the API is accepted, yes the include should not be an issue.
> > >
> > > Personally, I don't think it is a good idea to add extra dependency for
> > > librte_mbuf.
> > > I'd prefer either to keep rte_color definition inside librte_mbuf,
> > > or move corresponding function definitions out of it.
> > > Konstantin
> >
> > Konstantin,
> >
> > As you see, the number of options is limited, and none of them is
> > perfect:
> >
> > 1/ color enum in EAL/common/include: still my favorite, as it does not
> > create any new library dependencies, and it is already used
> to store lots of similar generic items
> > 2/ color enum in rte_meter.h: results in creating new librte_mbuf
> > dependency to librte_meter
> > 3/ color enum in rte_mbuf.h: results in creating new librte_meter
> > dependency to librte_mbuf (yes, currently librte_meter does not
> depend on librte_mbuf)
> >
> > Personally, I can live with any of these options.
> >
> > Thomas,
> >
> > It would be good to have your input as well, Reshma just sent V4
> > implementing your proposal based on having the color enum
> defined in rte_meter.h, which gets included into rte_mbuf.h.
>
> We need a decision from Olivier, mbuf maintainer.
>
> > We need to make progress for RC1 deadline.
>
> I'm afraid 19.02 is not the right release to change the mbuf.
> This kind of decision usually takes several weeks.
> Cc the technical board to get more opinions.
I general, I don't see any problem with these changes for 19.02.
As I can see,, they only affect rte_mbuf sched filed format/interpretation.
All other fields meaning/layout remain the same.
Konstantin
