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

Neil Horman nhorman at tuxdriver.com
Wed Mar 28 01:53:46 CEST 2018


On Tue, Mar 27, 2018 at 08:48:01PM +0000, Richardson, Bruce wrote:
> 
> 
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet at 6wind.com]
> > Sent: Tuesday, March 27, 2018 9:35 PM
> > To: Richardson, Bruce <bruce.richardson at intel.com>
> > Cc: Neil Horman <nhorman at tuxdriver.com>; dev at dpdk.org; Wiles, Keith
> > <keith.wiles at intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v3 10/20] eal/dev: implement device
> > iteration initialization
> > 
> > On Tue, Mar 27, 2018 at 09:28:07PM +0100, Bruce Richardson wrote:
> > > On Tue, Mar 27, 2018 at 10:20:40PM +0200, Gaëtan Rivet wrote:
> > > > 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.
> > > >
> > > Sure it can - just pass -I/path/to/eal as a cflag.
> > >
> > > /Bruce
> > 
> > When you put it this way... :)
> > 
> > Sure, this is a solution, of which early symlink was a genericization.
> > I'm just not a fan of having co-dependent libraries.
> > 
> > But this will probably come to that.
> > 
> 
> The other alternative is what was done with rte_compat.h - create a new lib
> just with a single header file in it. Works ok too, and may seem less hacky
> to some folks.
> 
> /Bruce
> 
This seems like a good alternative to me.  I'm not entirely sure why the logging
functions are integrated to EAL anyway.  

Neil
 


More information about the dev mailing list