[dpdk-dev] [PATCH v3 10/20] eal/dev: implement device iteration initialization

Gaëtan Rivet gaetan.rivet at 6wind.com
Tue Mar 27 22:20:40 CEST 2018


On Tue, Mar 27, 2018 at 02:26:33PM -0400, Neil Horman wrote:
> On Tue, Mar 27, 2018 at 02:40:00PM +0200, Gaëtan Rivet wrote:
> > On Tue, Mar 27, 2018 at 07:47:50AM -0400, Neil Horman wrote:
> > > On Tue, Mar 27, 2018 at 01:18:34AM +0200, Gaetan Rivet wrote:
> > > > Parse a device description.
> > > > Split this description in their relevant part for each layers.
> > > > No dynamic allocation is performed.
> > > > 
> > > > Cc: Neil Horman <nhorman at tuxdriver.com>
> > > > Cc: "Wiles, Keith" <keith.wiles at intel.com>
> > > > Signed-off-by: Gaetan Rivet <gaetan.rivet at 6wind.com>
> > > > ---
> > > > 
> > > > This version uses librte_kvargs.
> > > > 
> > > 
> > > Otherwise, this looks pretty good to me
> > 
> > Please look into the librte_kvargs compatibility patch as well (quite
> > short). I'm very unhappy about the logging hack.
> > There is always the solution of setting a function pointer on rte_log
> > with the proper loglevels and so on.
> > Ideally rte_log could be made independent (starting skimming EAL from
> > all the fat), but this is much less trivial.
> > 
> just posted about that.  I agree with Keith, I don't think you should need that
> patch.  RTE_LOG just calls rte_vlog which contains this code:
> 
> if (f == NULL) {
>                 f = default_log_stream;
>                 if (f == NULL) {
>                         /*
>                          * Grab the current value of stderr here, rather than
>                          * just initializing default_log_stream to stderr. This
>                          * ensures that we will always use the current value
>                          * of stderr, even if the application closes and
>                          * reopens it.
>                          */
>                         f = stderr;
>                 }
>         }
> }
> 
> Which I read as saying that the logging library should back off to stderr if its
> not initialized yet.  If you've encountered a problem that made you need that
> logging patch, it seems like you should be able to drop it, and we need to fix
> the logging library.  Can you elaborate on what you ran into here?
> 
> Neil

Neat. The issue is that rte_log.h is not symlink-ed until librte_eal is
processed. rte_log cannot be included.

I know Keith and Bruce discussed a few months back the possibility of
having an initial linking pass. Personally I am still against this kind of
solution, hiding design issues under the rug.

-- 
Gaëtan Rivet
6WIND


More information about the dev mailing list