[dpdk-dev] tools brainstorming

[Wiles, Keith]

On 4/9/15, 4:23 PM, "Stephen Hemminger"  wrote:

>On Thu, 9 Apr 2015 21:10:19 +
>"Wiles, Keith"  wrote:
>
>> 
>> 
>> On 4/9/15, 2:38 PM, "Jay Rolette"  wrote:
>> 
>> >On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman 
>>wrote:
>> >
>> >> On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
>> >> > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
>> >> > stephen at networkplumber.org> wrote:
>> >> >
>> >> > > On Wed, 8 Apr 2015 16:29:54 -0600
>> >> > > Jay Rolette  wrote:
>> >> > >
>> >> > > > "C comments" includes //, right? It's been part of the C
>>standard
>> >> for a
>> >> > > long time now...
>> >> > >
>> >> > > Yes but.
>> >> > > I like to use checkpatch and checkpatch enforces kernel style
>>which
>> >> does
>> >> > > not allow // for
>> >> > > comments.
>> >> > >
>> >> >
>> >> > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
>> >> > requirement to follow all of its rules
>> >> >
>> >>
>> >> Doesn't that beg the question, why?  I understand the DPDK isn't the
>> >> kernel, but
>> >> we're not talking about clarity of code, not anything functional to
>>that
>> >> code.
>> >> It seems we would be better served by just taking something that
>>works
>> >>here
>> >> rather than re-inventing the wheel and digging into the minuate of
>>what
>> >> type of
>> >> comments should be allowed (unless there is a compelling reason to
>> >>change
>> >> it
>> >> that supercedes the avilable tools).  If not checkpath, then some
>>other
>> >> tool,
>> >> but It seems to me that coding style is one of those things where we
>>can
>> >> bend to
>> >> the tool rather than taking the time to make the tool do exactly
>>whats
>> >> desired,
>> >> at least until someone gets the time to modify it.
>> >>
>> >
>> >Fair question.
>> >
>> >It depends a bit on how much you want to encourage patch
>>contributions. Is
>> >it worth adding more pain for folks trying to contribute patches for
>> >things
>> >like this?
>> >
>> >Should we force someone to spend time redoing a patch because of which
>>way
>> >they do their parenthesis? What about number of spaces to indent code?
>>//
>> >vs /* */ comments? None of these matter functionally and they don't
>>affect
>> >maintenance generally.
>> >
>> >If someone is modifying existing code, then yeah, they should follow
>>the
>> >prevailing style (indention level, brace alignment, etc.) of the file
>>they
>> >are in. It helps readability, which makes maintenance easier. However,
>> >IMO,
>> >mixing // and /* */ for comments doesn't affect the readability of the
>> >source.
>> >
>> >I know if I submit a patch and the only feedback is that I should have
>> >used
>> >/* */ for comments, I'm extremely unlikely spend extra time to resubmit
>> >the
>> >patch for pedantry.
>> 
>> I looked at checkpatch.pl for few minutes and the code does check for
>>C99
>> comments and adding a command line option to allow C99 comments could
>> pretty simple. I found the code around line 3048 or search for C99, it
>>is
>> possible it could accepted back into Linux as long as the default option
>> was to not allow C99 comments.
>> 
>> Allowing C99 comments would be nice and the only problem I could see if
>> some compiler has a problem with them. I believe all of the compilers we
>> support allow C99 comments.
>> 
>> The only other reason to allow them is if we add some open source code
>>in
>> the future to DPDK which has C99 comments and if would be a pain to have
>> to convert that code every time the open source group released a new
>> version. It does open that path IMO.
>> 
>> Regards,
>> ++Keith
>> >
>> 
>
>But forking a tool means maintaining that tool.

If the tool is pushed back into the main stream, then no you do not have
to maintain it, right?
That was my point and the change a simple one plus I would expect it
should not give anyone a problem unless Linux really wants to stay pre
C99, which is not the case today, right?
>

[dpdk-dev] tools brainstorming

[Wiles, Keith]

On 4/9/15, 2:38 PM, "Jay Rolette"  wrote:

>On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman  wrote:
>
>> On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
>> > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
>> > stephen at networkplumber.org> wrote:
>> >
>> > > On Wed, 8 Apr 2015 16:29:54 -0600
>> > > Jay Rolette  wrote:
>> > >
>> > > > "C comments" includes //, right? It's been part of the C standard
>> for a
>> > > long time now...
>> > >
>> > > Yes but.
>> > > I like to use checkpatch and checkpatch enforces kernel style which
>> does
>> > > not allow // for
>> > > comments.
>> > >
>> >
>> > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
>> > requirement to follow all of its rules
>> >
>>
>> Doesn't that beg the question, why?  I understand the DPDK isn't the
>> kernel, but
>> we're not talking about clarity of code, not anything functional to that
>> code.
>> It seems we would be better served by just taking something that works
>>here
>> rather than re-inventing the wheel and digging into the minuate of what
>> type of
>> comments should be allowed (unless there is a compelling reason to
>>change
>> it
>> that supercedes the avilable tools).  If not checkpath, then some other
>> tool,
>> but It seems to me that coding style is one of those things where we can
>> bend to
>> the tool rather than taking the time to make the tool do exactly whats
>> desired,
>> at least until someone gets the time to modify it.
>>
>
>Fair question.
>
>It depends a bit on how much you want to encourage patch contributions. Is
>it worth adding more pain for folks trying to contribute patches for
>things
>like this?
>
>Should we force someone to spend time redoing a patch because of which way
>they do their parenthesis? What about number of spaces to indent code? //
>vs /* */ comments? None of these matter functionally and they don't affect
>maintenance generally.
>
>If someone is modifying existing code, then yeah, they should follow the
>prevailing style (indention level, brace alignment, etc.) of the file they
>are in. It helps readability, which makes maintenance easier. However,
>IMO,
>mixing // and /* */ for comments doesn't affect the readability of the
>source.
>
>I know if I submit a patch and the only feedback is that I should have
>used
>/* */ for comments, I'm extremely unlikely spend extra time to resubmit
>the
>patch for pedantry.

I looked at checkpatch.pl for few minutes and the code does check for C99
comments and adding a command line option to allow C99 comments could
pretty simple. I found the code around line 3048 or search for C99, it is
possible it could accepted back into Linux as long as the default option
was to not allow C99 comments.

Allowing C99 comments would be nice and the only problem I could see if
some compiler has a problem with them. I believe all of the compilers we
support allow C99 comments.

The only other reason to allow them is if we add some open source code in
the future to DPDK which has C99 comments and if would be a pain to have
to convert that code every time the open source group released a new
version. It does open that path IMO.

Regards,
++Keith
>

[dpdk-dev] tools brainstorming

[Neil Horman]

On Thu, Apr 09, 2015 at 09:10:19PM +, Wiles, Keith wrote:
> 
> 
> On 4/9/15, 2:38 PM, "Jay Rolette"  wrote:
> 
> >On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman  wrote:
> >
> >> On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
> >> > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
> >> > stephen at networkplumber.org> wrote:
> >> >
> >> > > On Wed, 8 Apr 2015 16:29:54 -0600
> >> > > Jay Rolette  wrote:
> >> > >
> >> > > > "C comments" includes //, right? It's been part of the C standard
> >> for a
> >> > > long time now...
> >> > >
> >> > > Yes but.
> >> > > I like to use checkpatch and checkpatch enforces kernel style which
> >> does
> >> > > not allow // for
> >> > > comments.
> >> > >
> >> >
> >> > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
> >> > requirement to follow all of its rules
> >> >
> >>
> >> Doesn't that beg the question, why?  I understand the DPDK isn't the
> >> kernel, but
> >> we're not talking about clarity of code, not anything functional to that
> >> code.
> >> It seems we would be better served by just taking something that works
> >>here
> >> rather than re-inventing the wheel and digging into the minuate of what
> >> type of
> >> comments should be allowed (unless there is a compelling reason to
> >>change
> >> it
> >> that supercedes the avilable tools).  If not checkpath, then some other
> >> tool,
> >> but It seems to me that coding style is one of those things where we can
> >> bend to
> >> the tool rather than taking the time to make the tool do exactly whats
> >> desired,
> >> at least until someone gets the time to modify it.
> >>
> >
> >Fair question.
> >
> >It depends a bit on how much you want to encourage patch contributions. Is
> >it worth adding more pain for folks trying to contribute patches for
> >things
> >like this?
> >
> >Should we force someone to spend time redoing a patch because of which way
> >they do their parenthesis? What about number of spaces to indent code? //
> >vs /* */ comments? None of these matter functionally and they don't affect
> >maintenance generally.
> >
> >If someone is modifying existing code, then yeah, they should follow the
> >prevailing style (indention level, brace alignment, etc.) of the file they
> >are in. It helps readability, which makes maintenance easier. However,
> >IMO,
> >mixing // and /* */ for comments doesn't affect the readability of the
> >source.
> >
> >I know if I submit a patch and the only feedback is that I should have
> >used
> >/* */ for comments, I'm extremely unlikely spend extra time to resubmit
> >the
> >patch for pedantry.
> 
> I looked at checkpatch.pl for few minutes and the code does check for C99
> comments and adding a command line option to allow C99 comments could
> pretty simple. I found the code around line 3048 or search for C99, it is
> possible it could accepted back into Linux as long as the default option
> was to not allow C99 comments.
> 
> Allowing C99 comments would be nice and the only problem I could see if
> some compiler has a problem with them. I believe all of the compilers we
> support allow C99 comments.
> 
> The only other reason to allow them is if we add some open source code in
> the future to DPDK which has C99 comments and if would be a pain to have
> to convert that code every time the open source group released a new
> version. It does open that path IMO.
> 

So, this again seems to be bad philosophy in my mind.  If we are, to use your
exmple, accept code into the DPDK in the future with comments that violate our
selected style, it is then, by definition, in violation of the style guidelines.
If we accept it anyway, or if we allow both styles (by documenting it/codifying
it a tool to check for/etc) then we dilute the style guide.  Maybe in some
cases, such as this, thats ok, but its something to be cogniscent of.
Especially if making the choice to allow both put us in a position of having to
maintain a tool to do the checking, then I think we need to fall on the side of
going with what the tool (checkpatch or something else) does, unless we have a
maintainer stepping up.

The bottom line is that style guides enforce style, and tooling makes
contributors condusive to following the style.  If we have someone that is
willing to maintain such a tool, then we have a lot of leway in what the style
is, but if we don't then we really need to follow the style that an existing
tool provides, because without tooling, contributors aren't likely to bother
with strict adherence to the style.

Neil

[dpdk-dev] tools brainstorming

[Neil Horman]

On Thu, Apr 09, 2015 at 09:29:14PM +, Wiles, Keith wrote:
> 
> 
> On 4/9/15, 4:23 PM, "Stephen Hemminger"  wrote:
> 
> >On Thu, 9 Apr 2015 21:10:19 +
> >"Wiles, Keith"  wrote:
> >
> >> 
> >> 
> >> On 4/9/15, 2:38 PM, "Jay Rolette"  wrote:
> >> 
> >> >On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman 
> >>wrote:
> >> >
> >> >> On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
> >> >> > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
> >> >> > stephen at networkplumber.org> wrote:
> >> >> >
> >> >> > > On Wed, 8 Apr 2015 16:29:54 -0600
> >> >> > > Jay Rolette  wrote:
> >> >> > >
> >> >> > > > "C comments" includes //, right? It's been part of the C
> >>standard
> >> >> for a
> >> >> > > long time now...
> >> >> > >
> >> >> > > Yes but.
> >> >> > > I like to use checkpatch and checkpatch enforces kernel style
> >>which
> >> >> does
> >> >> > > not allow // for
> >> >> > > comments.
> >> >> > >
> >> >> >
> >> >> > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
> >> >> > requirement to follow all of its rules
> >> >> >
> >> >>
> >> >> Doesn't that beg the question, why?  I understand the DPDK isn't the
> >> >> kernel, but
> >> >> we're not talking about clarity of code, not anything functional to
> >>that
> >> >> code.
> >> >> It seems we would be better served by just taking something that
> >>works
> >> >>here
> >> >> rather than re-inventing the wheel and digging into the minuate of
> >>what
> >> >> type of
> >> >> comments should be allowed (unless there is a compelling reason to
> >> >>change
> >> >> it
> >> >> that supercedes the avilable tools).  If not checkpath, then some
> >>other
> >> >> tool,
> >> >> but It seems to me that coding style is one of those things where we
> >>can
> >> >> bend to
> >> >> the tool rather than taking the time to make the tool do exactly
> >>whats
> >> >> desired,
> >> >> at least until someone gets the time to modify it.
> >> >>
> >> >
> >> >Fair question.
> >> >
> >> >It depends a bit on how much you want to encourage patch
> >>contributions. Is
> >> >it worth adding more pain for folks trying to contribute patches for
> >> >things
> >> >like this?
> >> >
> >> >Should we force someone to spend time redoing a patch because of which
> >>way
> >> >they do their parenthesis? What about number of spaces to indent code?
> >>//
> >> >vs /* */ comments? None of these matter functionally and they don't
> >>affect
> >> >maintenance generally.
> >> >
> >> >If someone is modifying existing code, then yeah, they should follow
> >>the
> >> >prevailing style (indention level, brace alignment, etc.) of the file
> >>they
> >> >are in. It helps readability, which makes maintenance easier. However,
> >> >IMO,
> >> >mixing // and /* */ for comments doesn't affect the readability of the
> >> >source.
> >> >
> >> >I know if I submit a patch and the only feedback is that I should have
> >> >used
> >> >/* */ for comments, I'm extremely unlikely spend extra time to resubmit
> >> >the
> >> >patch for pedantry.
> >> 
> >> I looked at checkpatch.pl for few minutes and the code does check for
> >>C99
> >> comments and adding a command line option to allow C99 comments could
> >> pretty simple. I found the code around line 3048 or search for C99, it
> >>is
> >> possible it could accepted back into Linux as long as the default option
> >> was to not allow C99 comments.
> >> 
> >> Allowing C99 comments would be nice and the only problem I could see if
> >> some compiler has a problem with them. I believe all of the compilers we
> >> support allow C99 comments.
> >> 
> >> The only other reason to allow them is if we add some open source code
> >>in
> >> the future to DPDK which has C99 comments and if would be a pain to have
> >> to convert that code every time the open source group released a new
> >> version. It does open that path IMO.
> >> 
> >> Regards,
> >> ++Keith
> >> >
> >> 
> >
> >But forking a tool means maintaining that tool.
> 
> If the tool is pushed back into the main stream, then no you do not have
> to maintain it, right?
> That was my point and the change a simple one plus I would expect it
> should not give anyone a problem unless Linux really wants to stay pre
> C99, which is not the case today, right?
This is true, but the typical attitude in the linux kernel is that a change must
have a use case and a user to be considered.  I don't want to dissuade you from
trying to get it accepted, but my concern would be that, since the kernel folks
don't allow the '//' comments, they won't be interested in maintaining the
change.

Neil

> >
> 
> 

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Avi Kivity]

On 04/09/2015 02:19 PM, Neil Horman wrote:
> On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
>>
>> On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
>>> On 08/04/2015 19:26, Stephen Hemminger wrote:
 On Wed,  8 Apr 2015 16:07:21 +0100
 Sergio Gonzalez Monroy  wrote:

> Currently, the target/rules to build combined libraries is different
> than the one to build individual libraries.
>
> By removing the combined library option as a build configuration option
> we simplify the build pocess by having a single point for
> linking/archiving
> libraries in DPDK.
>
> This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
> removes the makefiles associated with building a combined library.
>
> The CONFIG_RTE_LIBNAME config option is kept as it will be use to
> always generate a linker script that acts as a single combined library.
>
> Signed-off-by: Sergio Gonzalez Monroy
> 
 No. We use combined library and it greatly simplfies the application
 linking process.

>>> After all the opposition this patch had in v2, I did explain the current
>>> issues
>>> (see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this was
>>> the agreed solution.
>>>
>>> As I mention in the cover letter (also see patch 2/5), building DPDK
>>> (after applying this patch series) will always generate a very simple
>>> linker script that behaves as a combined library.
>>> I encourage you to apply this patch series and try to build your app
>>> (which links against combined lib).
>>> Your app should build without problem unless I messed up somewhere and it
>>> needs fixing.
>> Is it possible to generate a pkgconfig file (dpdk.pc) that contains all of
>> the setting needed to compile and link with dpdk?  That will greatly
>> simplify usage.
>>
>> A linker script is just too esoteric.
>>
> Why esoteric?  We're not talking about a linker script in the sense of a 
> binary
> layout file, we're talking about a prewritten/generated libdpdk_core.so that
> contains linker directives to include the appropriate libraries.  You link it
> just like you do any other library, but it lets you ignore how they are broken
> up.

You mean DT_NEEDED?  That's great, but it shouldn't be called a linker 
script.

> We could certainly do a pkg-config file, but I don't think thats any more
> adventageous than this solution.

It solves more problems -- cflags etc.  Of course having the right 
DT_NEEDED is a good thing regardless.

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Neil Horman]

On Thu, Apr 09, 2015 at 08:00:26PM +0300, Avi Kivity wrote:
> On 04/09/2015 02:19 PM, Neil Horman wrote:
> >On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
> >>
> >>On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
> >>>On 08/04/2015 19:26, Stephen Hemminger wrote:
> On Wed,  8 Apr 2015 16:07:21 +0100
> Sergio Gonzalez Monroy  wrote:
> 
> >Currently, the target/rules to build combined libraries is different
> >than the one to build individual libraries.
> >
> >By removing the combined library option as a build configuration option
> >we simplify the build pocess by having a single point for
> >linking/archiving
> >libraries in DPDK.
> >
> >This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
> >removes the makefiles associated with building a combined library.
> >
> >The CONFIG_RTE_LIBNAME config option is kept as it will be use to
> >always generate a linker script that acts as a single combined library.
> >
> >Signed-off-by: Sergio Gonzalez Monroy
> >
> No. We use combined library and it greatly simplfies the application
> linking process.
> 
> >>>After all the opposition this patch had in v2, I did explain the current
> >>>issues
> >>>(see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this was
> >>>the agreed solution.
> >>>
> >>>As I mention in the cover letter (also see patch 2/5), building DPDK
> >>>(after applying this patch series) will always generate a very simple
> >>>linker script that behaves as a combined library.
> >>>I encourage you to apply this patch series and try to build your app
> >>>(which links against combined lib).
> >>>Your app should build without problem unless I messed up somewhere and it
> >>>needs fixing.
> >>Is it possible to generate a pkgconfig file (dpdk.pc) that contains all of
> >>the setting needed to compile and link with dpdk?  That will greatly
> >>simplify usage.
> >>
> >>A linker script is just too esoteric.
> >>
> >Why esoteric?  We're not talking about a linker script in the sense of a 
> >binary
> >layout file, we're talking about a prewritten/generated libdpdk_core.so that
> >contains linker directives to include the appropriate libraries.  You link it
> >just like you do any other library, but it lets you ignore how they are 
> >broken
> >up.
> 
> You mean DT_NEEDED?  That's great, but it shouldn't be called a linker
> script.
> 
no, I don't mean DT_NEEDED, I mean a linker script, because thats what what
sergio wrote is.

> >We could certainly do a pkg-config file, but I don't think thats any more
> >adventageous than this solution.
> 
> It solves more problems -- cflags etc.  Of course having the right DT_NEEDED
> is a good thing regardless.
> 
Thats a good point, pkgconfig doesn't provide that additionally.  Perhaps having
both is the best solution.  As for the DT_NEEDED issues, the earlier threads
ennumerated all the problems that were being found with the way the libraries
were organized (circular dependencies).

Neil

[dpdk-dev] tools brainstorming

[Neil Horman]

On Thu, Apr 09, 2015 at 02:38:32PM -0500, Jay Rolette wrote:
> On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman  wrote:
> 
> > On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
> > > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
> > > stephen at networkplumber.org> wrote:
> > >
> > > > On Wed, 8 Apr 2015 16:29:54 -0600
> > > > Jay Rolette  wrote:
> > > >
> > > > > "C comments" includes //, right? It's been part of the C standard
> > for a
> > > > long time now...
> > > >
> > > > Yes but.
> > > > I like to use checkpatch and checkpatch enforces kernel style which
> > does
> > > > not allow // for
> > > > comments.
> > > >
> > >
> > > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
> > > requirement to follow all of its rules
> > >
> >
> > Doesn't that beg the question, why?  I understand the DPDK isn't the
> > kernel, but
> > we're not talking about clarity of code, not anything functional to that
> > code.
> > It seems we would be better served by just taking something that works here
> > rather than re-inventing the wheel and digging into the minuate of what
> > type of
> > comments should be allowed (unless there is a compelling reason to change
> > it
> > that supercedes the avilable tools).  If not checkpath, then some other
> > tool,
> > but It seems to me that coding style is one of those things where we can
> > bend to
> > the tool rather than taking the time to make the tool do exactly whats
> > desired,
> > at least until someone gets the time to modify it.
> >
> 
> Fair question.
> 
> It depends a bit on how much you want to encourage patch contributions. Is
> it worth adding more pain for folks trying to contribute patches for things
> like this?
> 
> Should we force someone to spend time redoing a patch because of which way
> they do their parenthesis? What about number of spaces to indent code? //
> vs /* */ comments? None of these matter functionally and they don't affect
> maintenance generally.
> 
> If someone is modifying existing code, then yeah, they should follow the
> prevailing style (indention level, brace alignment, etc.) of the file they
> are in. It helps readability, which makes maintenance easier. However, IMO,
> mixing // and /* */ for comments doesn't affect the readability of the
> source.
> 
I take your meaning (that we shouldn't be overly restrictive on aspects of the
code that don't affect functionality, but I think this line of thinking quickly
spirals out into the question of weather to have coding styles at all.  For any
aspect of code that you codify in a style guide, you are almost by definition
being restrictive:

// comments

vs.

/*
 * comments
 */

or

void func(int args) {

}

vs.

void
func (int args)
{

}

Insert your own pet coding style variants as you see fit.  If we want to enforce
coding styles, we have to pick something and enforce it.  To suggest that we
allow both (or some subset of the entire set of some coding style aspect, as I
think you are trying to propose), while fine to do, somewhat calls into
question the need/desire for style guidlines at all. As you note below, you're
unlikely to revise a patch if the only comment is "use different commenting
style".  The exact same might be a response to your function declaration style,
or any other aspect.  If you say both/all are allowed, you quickly get to the
point of not really having a style (which may be acceptible here).

> I know if I submit a patch and the only feedback is that I should have used
> /* */ for comments, I'm extremely unlikely spend extra time to resubmit the
> patch for pedantry.

Hence my desire for the tool.  Ideally, style is best enforced when its a
non-issue during the review process (i.e. a tool is able to tell you where
your style problems are, and the issue never comes up during review).  We can as
you previously suggested fork a tool and modify it to conform to some other
style guidelines of our own design.  But honestly, I don't want to invest alot
of time in what the guidelines are.  i.e. I value consistency in style, not a
specific style.  As such, borrowing checkpatch (or some other tool), and
adopting its style enforcement, seems like the best, most efficient path forward
in my mind.

Neil

[dpdk-dev] [PATCH] Add toeplitz hash algorithm

[Vladimir Medvedkin]

Hi Gleb,


2015-04-09 9:37 GMT+03:00 Gleb Natapov :

> On Wed, Apr 08, 2015 at 03:06:13PM -0400, Vladimir Medvedkin wrote:
> > Software implementation of the Toeplitz hash function used by RSS.
> > Can be used either for packet distribution on single queue NIC
> > or for simulating of RSS computation on specific NIC (for example
> > after GRE header decapsulating).
> >
> > Signed-off-by: Vladimir Medvedkin 
> > ---
> >  lib/librte_hash/Makefile|   1 +
> >  lib/librte_hash/rte_thash.h | 179
> 
> >  2 files changed, 180 insertions(+)
> >  create mode 100644 lib/librte_hash/rte_thash.h
> >
> > diff --git a/lib/librte_hash/Makefile b/lib/librte_hash/Makefile
> > index 3696cb1..083a9e5 100644
> > --- a/lib/librte_hash/Makefile
> > +++ b/lib/librte_hash/Makefile
> > @@ -50,6 +50,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include := rte_hash.h
> >  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_hash_crc.h
> >  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_jhash.h
> >  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_fbk_hash.h
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_thash.h
> >
> >  # this lib needs eal
> >  DEPDIRS-$(CONFIG_RTE_LIBRTE_HASH) += lib/librte_eal lib/librte_malloc
> > diff --git a/lib/librte_hash/rte_thash.h b/lib/librte_hash/rte_thash.h
> > new file mode 100644
> > index 000..1acfa3a
> > --- /dev/null
> > +++ b/lib/librte_hash/rte_thash.h
> > @@ -0,0 +1,179 @@
> > +/*-
> > + *   BSD LICENSE
> > + *
> > + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + * * Redistributions of source code must retain the above copyright
> > + *   notice, this list of conditions and the following disclaimer.
> > + * * Redistributions in binary form must reproduce the above
> copyright
> > + *   notice, this list of conditions and the following disclaimer in
> > + *   the documentation and/or other materials provided with the
> > + *   distribution.
> > + * * Neither the name of Intel Corporation nor the names of its
> > + *   contributors may be used to endorse or promote products derived
> > + *   from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> > + */
> > +
> > +#ifndef _RTE_THASH_H
> > +#define _RTE_THASH_H
> > +
> > +/**
> > + * @file
> > + *
> > + * toeplitz hash functions.
> > + */
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * Software implementation of the Toeplitz hash function used by RSS.
> > + * Can be used either for packet distribution on single queue NIC
> > + * or for simulating of RSS computation on specific NIC (for example
> > + * after GRE header decapsulating)
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +enum rte_thash_flag {
> > + RTE_THASH_L3 = 0,   //calculate hash tacking into account only
> l3 header
> > + RTE_THASH_L4//calculate hash tacking into account l4 +
> l4 headers
> > +};
> > +
> > +/**
> > + * Prepare special converted key to use with rte_softrss_be()
> > + * @param orig
> > + *   pointer to original RSS key
> > + * @param targ
> > + *   pointer to target RSS key
> > + */
> > +
> > +static inline void
> > +rte_convert_rss_key(uint32_t *orig, uint32_t *targ)
> > +{
> > + int i;
> > + for (i = 0; i < 10; i++) {
> > + targ[i] = rte_be_to_cpu_32(orig[i]);
> > + }
> > +}
> > +
> > +/**
> > + * Generic implementation. Can be used with original rss_key
> > + * All ip's and ports have to be CPU byte order.
> > + * @param sip
> > + *   Source ip address.
> > + * @param dip
> > + *   Destination ip address.
> ipv4, what about ipv6? Why not define rss function that works on byte
> buffer and let caller build it according to whatever fields it want to
> hash?
>
Good idea, I will think about it.


>
> > + * @param sp
> > + *   Source TCP|UDP port.
> > + * @param dp
> > + *   

[dpdk-dev] [PATCH] Add toeplitz hash algorithm

[Vladimir Medvedkin]

Hi Stephen,



2015-04-09 1:24 GMT+03:00 Stephen Hemminger :

> On Wed,  8 Apr 2015 15:06:13 -0400
> Vladimir Medvedkin  wrote:
>
> > Software implementation of the Toeplitz hash function used by RSS.
> > Can be used either for packet distribution on single queue NIC
> > or for simulating of RSS computation on specific NIC (for example
> > after GRE header decapsulating).
> >
> > Signed-off-by: Vladimir Medvedkin 
>
> > +enum rte_thash_flag {
> > + RTE_THASH_L3 = 0,   //calculate hash tacking into account only
> l3 header
> > + RTE_THASH_L4//calculate hash tacking into account l4 +
> l4 headers
> > +};
> > +
> > +/**
> > + * Prepare special converted key to use with rte_softrss_be()
> > + * @param orig
> > + *   pointer to original RSS key
> > + * @param targ
> > + *   pointer to target RSS key
> > + */
> > +
> > +static inline void
> > +rte_convert_rss_key(uint32_t *orig, uint32_t *targ)
> orig should be const
>
> > +{
> > + int i;
> > + for (i = 0; i < 10; i++) {
> > + targ[i] = rte_be_to_cpu_32(orig[i]);
> > + }
> > +}
>
> > +static inline uint32_t
> > +rte_softrss(uint32_t sip, uint32_t dip, uint16_t sp, uint16_t dp, enum
> rte_thash_flag flag, uint32_t *rss_key)
>
> rss_key should be const
>
> > +{
> > + uint32_t ret = 0;
> > + int i;
> > + for (i = 0; i < 32; i++) {
> blank line after declaration please
>
> > + if (sip & (1 << (31 - i))) {
> > + ret ^= (rte_cpu_to_be_32(*rss_key) <<
> i)|(uint32_t)((uint64_t)(rte_cpu_to_be_32(*(rss_key + 1))) >> (32 - i));
>
> Long expression > 80 characters.
> Repeated multiple times (should be inline)
> Extra parens ()
>
Thanks for remarks, I'll fix it.

> Extension to 64 bits is only to avoid compiler warning?
>
No, in case when i = 0 we shift uint32_t left by 32 bits, which leads to
undefined behaviour. In fact, shift counter just masked to 5 bits, so count
range is limited to 0 to 31.

>
>
> > + }
> > + }
> > + rss_key++;
> > + for (i = 0; i < 32; i++) {
> > + if (dip & (1 << (31 - i))) {
> > + ret ^= (rte_cpu_to_be_32(*rss_key) <<
> i)|(uint32_t)((uint64_t)(rte_cpu_to_be_32(*(rss_key + 1))) >> (32 - i));
> > + }
> > + }
> > +if (flag == RTE_THASH_L4) {
> > + rss_key++;
> > + for (i = 0; i < 32; i++) {
> > + if (((sp<<16)|dp) & (1 << (31 - i))) {
> > + ret ^= (rte_cpu_to_be_32(*rss_key) <<
> i)|(uint32_t)((uint64_t)(rte_cpu_to_be_32(*(rss_key + 1))) >> (32 - i));
> > + }
> > + }
> > + }
> > + return ret;
> > +}
> > +
> > +/**
> > + * Optimized implementation.
> > + * If you want the calculated hash value matches NIC RSS value
> > + * you have to use special converted key.
> > + * All ip's and ports have to be CPU byte order.
> > + * @param sip
> > + *   Source ip address.
> > + * @param dip
> > + *   Destination ip address.
> > + * @param sp
> > + *   Source TCP|UDP port.
> > + * @param dp
> > + *   Destination TCP|UDP port.
> > + * @param flag
> > + *   RTE_THASH_L3:   calculate hash tacking into account only sip and
> dip
> > + *   RTE_THASH_L4:   calculate hash tacking into account sip, dip, sp
> and dp
> > + * @param *rss_key
> > + *   Pointer to 40-byte RSS hash key.
> > + * @return
> > + *   Calculated hash value.
> > + */
> > +
> > +static inline uint32_t
> > +rte_softrss_be(uint32_t sip, uint32_t dip, uint16_t sp, uint16_t dp,
> enum rte_thash_flag flag, uint32_t *rss_key)
> > +{
>
> Same problems as previous code.
> Also lots of copy paste (see Do Not Repeat Yourself principle).
>

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Thomas Monjalon]

2015-04-09 07:19, Neil Horman:
> On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
> > On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
> > >On 08/04/2015 19:26, Stephen Hemminger wrote:
> > >>On Wed,  8 Apr 2015 16:07:21 +0100
> > >>Sergio Gonzalez Monroy  wrote:
> > >>
> > >>>Currently, the target/rules to build combined libraries is different
> > >>>than the one to build individual libraries.
> > >>>
> > >>>By removing the combined library option as a build configuration option
> > >>>we simplify the build pocess by having a single point for
> > >>>linking/archiving
> > >>>libraries in DPDK.
> > >>>
> > >>>This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
> > >>>removes the makefiles associated with building a combined library.
> > >>>
> > >>>The CONFIG_RTE_LIBNAME config option is kept as it will be use to
> > >>>always generate a linker script that acts as a single combined library.
> > >>>
> > >>>Signed-off-by: Sergio Gonzalez Monroy
> > >>>
> > >>No. We use combined library and it greatly simplfies the application
> > >>linking process.
> > >>
> > >After all the opposition this patch had in v2, I did explain the current
> > >issues
> > >(see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this was
> > >the agreed solution.
> > >
> > >As I mention in the cover letter (also see patch 2/5), building DPDK
> > >(after applying this patch series) will always generate a very simple
> > >linker script that behaves as a combined library.
> > >I encourage you to apply this patch series and try to build your app
> > >(which links against combined lib).
> > >Your app should build without problem unless I messed up somewhere and it
> > >needs fixing.
> > 
> > Is it possible to generate a pkgconfig file (dpdk.pc) that contains all of
> > the setting needed to compile and link with dpdk?  That will greatly
> > simplify usage.
> > 
> > A linker script is just too esoteric.
> > 
> Why esoteric?  We're not talking about a linker script in the sense of a 
> binary
> layout file, we're talking about a prewritten/generated libdpdk_core.so that
> contains linker directives to include the appropriate libraries.  You link it
> just like you do any other library, but it lets you ignore how they are broken
> up.
> 
> We could certainly do a pkg-config file, but I don't think thats any more
> adventageous than this solution.

As already commented (http://dpdk.org/ml/archives/dev/2015-March/015367.html),
pkgconfig could be something useful in any case (single or multi-libraries).
Having a linker script to replace the single ("combined") library may be
convenient in some cases but do not replace pkgconfig.

[dpdk-dev] Polling too often at lower packet rates?

[Aaron Campbell]

Hi Stephen,

Thanks, that was an informative talk.  In this case, are you referring to your 
comments about the thermal budget?

That?s definitely interesting, but there must be more to it than that.  Again, 
if I loop over all 6 ports (i.e., continue to keep the CPU busy), it works 
around the problem.

I agree that adaptive polling makes sense and will look into it.  But will 
still take any further ideas on what is going on here.

-Aaron

> On Apr 8, 2015, at 3:00 PM, Stephen Hemminger  
> wrote:
> 
> We use adaptive polling loop similar to l3fwd-power example.
> See:
>   
> 
> http://video.fosdem.org/2015/devroom-network_management_and_sdn/ 
> 
> 
> On Wed, Apr 8, 2015 at 9:35 AM, Aaron Campbell  > wrote:
> Hi,
> 
> I have a machine with 6 DPDK ports (4 igb, 2 ixgbe), with 1.23Mpps traffic 
> offered to only one of the 10G ports (the other 5 are unused).  I also have a 
> program with a pretty standard looking DPDK receive loop, where it calls 
> rte_eth_rx_burst() for each configured port.  If I configure the loop to read 
> from all 6 ports, it can read the 1.23Mpps rate with no drops.  If I 
> configure the loop to poll only 1 port (the ixgbe receiving the traffic), I 
> lose about 1/3rd of the packets (i.e., the NIC drops ~400Kpps).
> 
> Another data point is that if I configure the loop to read from 3 out of the 
> 6 ports, the drop rate is reduced to less than half (i.e., the NIC is only 
> dropping ~190Kpps now).  So it seems that in this test, throughput improves 
> by adding NICs, not removing them, which is counter-intuitive.  Again, I get 
> no drops when polling all 6 ports.  Note, the burst size is 32.
> 
> I did find a reference to a similar issue in a recent paper 
> (http://www.net.in.tum.de/fileadmin/bibtex/publications/papers/ICN2015.pdf 
> ), 
> Section III, which states:
> 
> "The DPDK L2FWD application initially only managed to forward 13.8 Mpps in 
> the single direction test at the maximum CPU frequency, a similar result can 
> be found in [11]. Reducing the CPU frequency increased the throughput to the 
> expected value of 14.88 Mpps. Our investigation of this anomaly revealed that 
> the lack of any processing combined with the fast CPU caused DPDK to poll the 
> NIC too often. DPDK does not use interrupts, it utilizes a busy wait loop 
> that polls the NIC until at least one packet is returned. This resulted in a 
> high poll rate which affected the throughput. We limited the poll rate to 
> 500,000 poll operations per second (i.e., a batch size of about 30 packets) 
> and achieved line rate in the unidirectional test with all frequencies. This 
> effect was only observed with the X520 NIC, tests with X540 NICs did not show 
> this anomaly.?
> 
> Another reference, from this mailing list last year 
> (http://wiki.dpdk.org/ml/archives/dev/2014-January/001169.html 
> ):
> 
> "I suggest you to check average burst sizes on receive queues. Looks like I 
> stumbled upon a similar issue several times. If you are calling 
> rte_eth_rx_burst too frequently, NIC begins losing packets no matter how many 
> CPU horse power you have (more you have, more it loses, actually). In my case 
> this situation occured when average burst size is less than 20 packets or so. 
> I'm not sure what's the reason for this behavior, but I observed it on 
> several applications on Intel 82599 10Gb cards.?
> 
> So I?m wondering if anyone can explain at a lower level what happens when you 
> poll ?too often?, and if there are any practical workarounds.  We?re using 
> this same program and DPDK version to process 10G line-rate in other 
> scenarios, so I?m confident that the overall packet capture architecture is 
> sound.
> 
> -Aaron
> 

[dpdk-dev] tools brainstorming

[Neil Horman]

On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
> On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
> stephen at networkplumber.org> wrote:
> 
> > On Wed, 8 Apr 2015 16:29:54 -0600
> > Jay Rolette  wrote:
> >
> > > "C comments" includes //, right? It's been part of the C standard for a
> > long time now...
> >
> > Yes but.
> > I like to use checkpatch and checkpatch enforces kernel style which does
> > not allow // for
> > comments.
> >
> 
> Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
> requirement to follow all of its rules
> 

Doesn't that beg the question, why?  I understand the DPDK isn't the kernel, but
we're not talking about clarity of code, not anything functional to that code.
It seems we would be better served by just taking something that works here
rather than re-inventing the wheel and digging into the minuate of what type of
comments should be allowed (unless there is a compelling reason to change it
that supercedes the avilable tools).  If not checkpath, then some other tool,
but It seems to me that coding style is one of those things where we can bend to
the tool rather than taking the time to make the tool do exactly whats desired,
at least until someone gets the time to modify it.

Neil

[dpdk-dev] [PATCH v3 1/5] mbuf: fix clone support when application uses private mbuf data

[Olivier MATZ]

Hi Konstantin,

On 04/08/2015 03:45 PM, Ananyev, Konstantin wrote:
> Hi Olivier,
>
>> -Original Message-
>> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
>> Sent: Wednesday, April 08, 2015 10:44 AM
>> To: Ananyev, Konstantin; dev at dpdk.org
>> Cc: zoltan.kiss at linaro.org; Richardson, Bruce
>> Subject: Re: [PATCH v3 1/5] mbuf: fix clone support when application uses 
>> private mbuf data
>>
>> Hi Konstantin,
>>
>> On 04/07/2015 07:17 PM, Ananyev, Konstantin wrote:
 Just to be sure we're on the same line:

 - before the patch series

  - private area was working before that patch series if clones were not
used. To use a private are, the user had to provide another
function derived from pktmbuf_init() to change m->buf_addr and
m->buf_len.
  - using both private area + clones was broken

 - after the patch series

  - private area is working with or without clone. But yo use it,
the user still has to provide another function to change
m->buf_addr, m->buf_len *and m->priv_size*.

 The series just fixes the fact that "clones + priv" was not working.
 It does not address the problem that providing a new pktmbuf_init()
 function is required to use privata area. To fix this, I think it
 could require a API evolution that should be part of another series.
>>>
>>> I don't think we need new pktmbuf_init().
>>> We just need to update it, so both pktmbuf_init() and detach() setup
>>> buf_addr, buf_len (and priv_size) to exactly the same values.
>>> If they don't do that, it means that you can't use attach/detach with
>>> mempools created with pktmbuf_init() any more.
>>>
>>> BTW, another thing that I just realised:
>>> examples/ipv4_multicast and examples/ip_fragmentation/ -
>>> both create a pool of mbufs with elem_size < 2K and don't populate 
>>> mempool's private area -
>>> so mbp_priv->mbuf_data_room_size == 0, for them.
>>>
>>> So that code in detach():
>>>
>>>+mbp_priv = rte_mempool_get_priv(mp);
>>>+m->priv_size = mp->elt_size - RTE_PKTMBUF_HEADROOM -
>>>+mbp_priv->mbuf_data_room_size -
>>>+sizeof(struct rte_mbuf);
>>>
>>>
>>> Would break both these samples.
>>> I suppose we need to handle situation when mp->elt_size < 
>>> RTE_PKTMBUF_HEADROOM + sizeof(struct rte_mbuf),
>>> (and probably also when mbuf_data_room_size == 0) correctly.
>>
>> Indeed. I think a mbuf pool (even with buf_len == 0 like in
>> ip_fragmentation example) should have a pool with a private area and
>> should call rte_pktmbuf_pool_init() to populate it. So
>> rte_pktmbuf_pool_init() has to be fixed first to use elt_size
>> and support the buf_len < RTE_PKTMBUF_HEADROOM, then we can
>> update frag/multicast examples.
>>
>> Unfortunately, we don't know the size of the mbuf private area
>> in rte_pktmbuf_pool_init() if the opaque arg (data_room_size)
>> is 0, which is the default. I think it should be replaced by a structure
>> containing data_room_size and mbuf_priv_size, but it would break
>> applications that are setting data_room_size.
>
> Yes, same thoughts here.
>
>> I don't see any good
>> solution to do that while keeping a backward compatibility for
>> rte_pktmbuf_pool_init(), but as the current API is not ideal,
>> I think it's worth changing it and add something in the release
>> note.
>
> If no one else has a better alternative than that, then I suppose it is good 
> enough.
>
>>
>> We may also want to introduce a new helper as discussed previously:
>>
>> struct rte_mempool *
>> rte_pktmbuf_pool_create(const char *name, unsigned n, unsigned elt_size,
>>  unsigned cache_size, size_t mbuf_priv_size,
>>  rte_mempool_obj_ctor_t *obj_init, void *obj_init_arg,
>>  int socket_id, unsigned flags)
>>
>> Any comment?
>
> Looks good to me.
> Should we also introduce rte_pktmbuf_pool_xmem_create()?

Hmmm as it's only used in one place, I'm not sure it's really
required. As the previous way to create mbuf pool using
rte_mempool_create() will still work, I think we could consider
it's a special case. If it becomes necessary, there's no problem
to add it later.

Olivier



> Konstantin
>
>>
>>
>>>
>>> Konstantin
>

[dpdk-dev] [snabb-devel] Re: memory barriers in virtq.lua?

[Xie, Huawei]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Luke Gorrie
> Sent: Thursday, April 09, 2015 11:13 AM
> To: snabb-devel at googlegroups.com
> Cc: dev at dpdk.org; VirtualOpenSystems Technical Team; Michael S. Tsirkin;
> virtualization at lists.linux-foundation.org
> Subject: Re: [dpdk-dev] [snabb-devel] Re: memory barriers in virtq.lua?
> 
> Howdy,
> 
> On 8 April 2015 at 17:15, Xie, Huawei  wrote:
> 
> > luke:
> > 1. host read the flag. 2 guest toggles the flag 3.guest checks the used.
> > 4. host update used.
> > Is this your case?
> >
> 
> Yep, that is exactly the case I mean.
Thanks. Will provide fix after evaluation.
> 
> Cheers,
> -Luke

[dpdk-dev] [PATCH] enic: migrating to new fdir api

[Sujith Sankar]

This patch helps enic migrate to the new flow-director API.

It takes care of the following.
1.  The change in fdir_filter structure and stats structure
2.  DPDK interface functions in enic_ethdev.c
3.  ENIC driver functions that deal with the VIC adapter


Signed-off-by: Sujith Sankar 
---
 lib/librte_pmd_enic/enic.h| 12 +++---
 lib/librte_pmd_enic/enic_clsf.c   | 43 --
 lib/librte_pmd_enic/enic_ethdev.c | 77 ++-
 3 files changed, 89 insertions(+), 43 deletions(-)

diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
index a50bff1..acdd0f6 100644
--- a/lib/librte_pmd_enic/enic.h
+++ b/lib/librte_pmd_enic/enic.h
@@ -80,13 +80,13 @@
 #define ENICPMD_FDIR_MAX   64

 struct enic_fdir_node {
-   struct rte_fdir_filter filter;
+   struct rte_eth_fdir_filter filter;
u16 fltr_id;
u16 rq_index;
 };

 struct enic_fdir {
-   struct rte_eth_fdir stats;
+   struct rte_eth_fdir_stats stats;
struct rte_hash *hash;
struct enic_fdir_node *nodes[ENICPMD_FDIR_MAX];
 };
@@ -110,7 +110,7 @@ struct enic {
pthread_t err_intr_thread;
int promisc;
int allmulti;
-   uint8_t ig_vlan_strip_en;
+   u8 ig_vlan_strip_en;
int link_status;
u8 hw_ip_checksum;

@@ -154,10 +154,12 @@ static inline struct enic *pmd_priv(struct rte_eth_dev 
*eth_dev)
return (struct enic *)eth_dev->data->dev_private;
 }

+extern void enic_fdir_stats_get(struct enic *enic,
+   struct rte_eth_fdir_stats *stats);
 extern int enic_fdir_add_fltr(struct enic *enic,
-   struct rte_fdir_filter *params, u16 queue, u8 drop);
+   struct rte_eth_fdir_filter *params);
 extern int enic_fdir_del_fltr(struct enic *enic,
-   struct rte_fdir_filter *params);
+   struct rte_eth_fdir_filter *params);
 extern void enic_free_wq(void *txq);
 extern int enic_alloc_intr_resources(struct enic *enic);
 extern int enic_setup_finish(struct enic *enic);
diff --git a/lib/librte_pmd_enic/enic_clsf.c b/lib/librte_pmd_enic/enic_clsf.c
index b61d625..f4cba75 100644
--- a/lib/librte_pmd_enic/enic_clsf.c
+++ b/lib/librte_pmd_enic/enic_clsf.c
@@ -65,7 +65,12 @@
 #define ENICPMD_CLSF_HASH_ENTRIES   ENICPMD_FDIR_MAX
 #define ENICPMD_CLSF_BUCKET_ENTRIES 4

-int enic_fdir_del_fltr(struct enic *enic, struct rte_fdir_filter *params)
+void enic_fdir_stats_get(struct enic *enic, struct rte_eth_fdir_stats *stats)
+{
+   *stats = enic->fdir.stats;
+}
+
+int enic_fdir_del_fltr(struct enic *enic, struct rte_eth_fdir_filter *params)
 {
int32_t pos;
struct enic_fdir_node *key;
@@ -92,23 +97,33 @@ int enic_fdir_del_fltr(struct enic *enic, struct 
rte_fdir_filter *params)
return 0;
 }

-int enic_fdir_add_fltr(struct enic *enic, struct rte_fdir_filter *params,
-   u16 queue, u8 drop)
+int enic_fdir_add_fltr(struct enic *enic, struct rte_eth_fdir_filter *params)
 {
struct enic_fdir_node *key;
struct filter fltr = {.type = 0};
int32_t pos;
u8 do_free = 0;
u16 old_fltr_id = 0;
+   u32 flowtype_supported;
+   u16 flex_bytes;
+   u16 queue;
+
+   flowtype_supported = (
+   (RTE_ETH_FLOW_NONFRAG_IPV4_TCP == params->input.flow_type) ||
+   (RTE_ETH_FLOW_NONFRAG_IPV4_UDP == params->input.flow_type));
+
+   flex_bytes = ((params->input.flow_ext.flexbytes[1] << 8 & 0xFF00) |
+   (params->input.flow_ext.flexbytes[0] & 0xFF));

-   if (!enic->fdir.hash || params->vlan_id || !params->l4type ||
-   (RTE_FDIR_IPTYPE_IPV6 == params->iptype) ||
-   (RTE_FDIR_L4TYPE_SCTP == params->l4type) ||
-   params->flex_bytes || drop) {
+   if (!enic->fdir.hash ||
+   (params->input.flow_ext.vlan_tci & 0xFFF) ||
+   !flowtype_supported || flex_bytes ||
+   params->action.behavior /* drop */) {
enic->fdir.stats.f_add++;
return -ENOTSUP;
}

+   queue = params->action.rx_queue;
/* See if the key is already there in the table */
pos = rte_hash_del_key(enic->fdir.hash, params);
switch (pos) {
@@ -168,12 +183,16 @@ int enic_fdir_add_fltr(struct enic *enic, struct 
rte_fdir_filter *params,
key->rq_index = queue;

fltr.type = FILTER_IPV4_5TUPLE;
-   fltr.u.ipv4.src_addr = rte_be_to_cpu_32(params->ip_src.ipv4_addr);
-   fltr.u.ipv4.dst_addr = rte_be_to_cpu_32(params->ip_dst.ipv4_addr);
-   fltr.u.ipv4.src_port = rte_be_to_cpu_16(params->port_src);
-   fltr.u.ipv4.dst_port = rte_be_to_cpu_16(params->port_dst);
+   fltr.u.ipv4.src_addr = rte_be_to_cpu_32(
+   params->input.flow.ip4_flow.src_ip);
+   fltr.u.ipv4.dst_addr = rte_be_to_cpu_32(
+   params->input.flow.ip4_flow.dst_ip);
+   fltr.u.ipv4.src_port = rte_be_to_cpu_16(
+   params->input.flow.udp4_flow.src_port);
+   

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Gonzalez Monroy, Sergio]

On 09/04/2015 14:33, Thomas Monjalon wrote:
> 2015-04-09 07:19, Neil Horman:
>> On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
>>> On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
 On 08/04/2015 19:26, Stephen Hemminger wrote:
> On Wed,  8 Apr 2015 16:07:21 +0100
> Sergio Gonzalez Monroy  wrote:
>
>> Currently, the target/rules to build combined libraries is different
>> than the one to build individual libraries.
>>
>> By removing the combined library option as a build configuration option
>> we simplify the build pocess by having a single point for
>> linking/archiving
>> libraries in DPDK.
>>
>> This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
>> removes the makefiles associated with building a combined library.
>>
>> The CONFIG_RTE_LIBNAME config option is kept as it will be use to
>> always generate a linker script that acts as a single combined library.
>>
>> Signed-off-by: Sergio Gonzalez Monroy
>> 
> No. We use combined library and it greatly simplfies the application
> linking process.
>
 After all the opposition this patch had in v2, I did explain the current
 issues
 (see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this was
 the agreed solution.

 As I mention in the cover letter (also see patch 2/5), building DPDK
 (after applying this patch series) will always generate a very simple
 linker script that behaves as a combined library.
 I encourage you to apply this patch series and try to build your app
 (which links against combined lib).
 Your app should build without problem unless I messed up somewhere and it
 needs fixing.
>>> Is it possible to generate a pkgconfig file (dpdk.pc) that contains all of
>>> the setting needed to compile and link with dpdk?  That will greatly
>>> simplify usage.
>>>
>>> A linker script is just too esoteric.
>>>
>> Why esoteric?  We're not talking about a linker script in the sense of a 
>> binary
>> layout file, we're talking about a prewritten/generated libdpdk_core.so that
>> contains linker directives to include the appropriate libraries.  You link it
>> just like you do any other library, but it lets you ignore how they are 
>> broken
>> up.
>>
>> We could certainly do a pkg-config file, but I don't think thats any more
>> adventageous than this solution.
> As already commented (http://dpdk.org/ml/archives/dev/2015-March/015367.html),
I misunderstood the pkgconfig reference in your previous comment.
It seems even more trivial to generate the 'combined' linker script lib 
having a pkg-config file.
We could simplify much of the rte.app.mk by using a pkg-config file.

Sergio
> pkgconfig could be something useful in any case (single or multi-libraries).
> Having a linker script to replace the single ("combined") library may be
> convenient in some cases but do not replace pkgconfig.
>

[dpdk-dev] tools brainstorming

[Jay Rolette]

On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman  wrote:

> On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
> > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
> > stephen at networkplumber.org> wrote:
> >
> > > On Wed, 8 Apr 2015 16:29:54 -0600
> > > Jay Rolette  wrote:
> > >
> > > > "C comments" includes //, right? It's been part of the C standard
> for a
> > > long time now...
> > >
> > > Yes but.
> > > I like to use checkpatch and checkpatch enforces kernel style which
> does
> > > not allow // for
> > > comments.
> > >
> >
> > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
> > requirement to follow all of its rules
> >
>
> Doesn't that beg the question, why?  I understand the DPDK isn't the
> kernel, but
> we're not talking about clarity of code, not anything functional to that
> code.
> It seems we would be better served by just taking something that works here
> rather than re-inventing the wheel and digging into the minuate of what
> type of
> comments should be allowed (unless there is a compelling reason to change
> it
> that supercedes the avilable tools).  If not checkpath, then some other
> tool,
> but It seems to me that coding style is one of those things where we can
> bend to
> the tool rather than taking the time to make the tool do exactly whats
> desired,
> at least until someone gets the time to modify it.
>

Fair question.

It depends a bit on how much you want to encourage patch contributions. Is
it worth adding more pain for folks trying to contribute patches for things
like this?

Should we force someone to spend time redoing a patch because of which way
they do their parenthesis? What about number of spaces to indent code? //
vs /* */ comments? None of these matter functionally and they don't affect
maintenance generally.

If someone is modifying existing code, then yeah, they should follow the
prevailing style (indention level, brace alignment, etc.) of the file they
are in. It helps readability, which makes maintenance easier. However, IMO,
mixing // and /* */ for comments doesn't affect the readability of the
source.

I know if I submit a patch and the only feedback is that I should have used
/* */ for comments, I'm extremely unlikely spend extra time to resubmit the
patch for pedantry.

[dpdk-dev] Running DPDK Binaries on a different Target

[Venkat Thummala]

I have the following ACL rule configured in the ACL Table.

Priority - 20
SIP - 0
SIP MASK - 0
DIP - 0
DIP MASK - 0
PROTO - 17 [UDP]
PROTO MASK - 0xFF
SPORT - 0
SPORT RANGE - 0x
DPORT - 0
DPORT RANGE - 0x

And pumping UDP traffic.

On Machine 1, it always HITS the rule as expected.

But On Machine 2, the same traffic never HITS the rule.

I have tried this with both SCALAR and SSE classification algorithms, but
the result is same.

Could some one, please help me on this?

DPDK Version:
===
2.0

Machine 1 CPU INFO:

vendor_id: GenuineIntel
cpu family: 6
model: 69
model name: Intel(R) Core(TM) i5-4200U CPU @ 1.60GHz
stepping: 1
microcode: 0x16
cpu MHz: 759.000
cache size: 3072 KB
physical id: 0
siblings: 4
core id: 1
cpu cores: 2
apicid: 3
initial apicid: 3
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes
flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe popcnt
tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm ida arat epb
xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid fsgsbase
tsc_adjust bmi1 avx2 smep bmi2 erms invpcid
bogomips: 4589.70
clflush size: 64
cache_alignment: 64
address sizes: 39 bits physical, 48 bits virtual
power management:

MACHINE 1 OS:
=
Linux vthummala-HP-Pavilion-15-Notebook-PC 3.13.0-48-generic #80-Ubuntu SMP
Thu Mar 12 11:16:15 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

MACHINE 2 CPU INFO:
==
vendor_id: GenuineIntel
cpu family: 6
model: 62
model name: Intel(R) Xeon(R) CPU E5-2650 v2 @ 2.60GHz
stepping: 4
microcode: 0x416
cpu MHz: 2599.890
cache size: 20480 KB
physical id: 1
siblings: 16
core id: 7
cpu cores: 8
apicid: 47
initial apicid: 47
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes
flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic
popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm ida arat
xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid fsgsbase smep
erms
bogomips: 5201.35
clflush size: 64
cache_alignment: 64
address sizes: 46 bits physical, 48 bits virtual
power management:

MACHINE 2 OS:
=
Linux vthummala-PowerEdge-R720 3.13.0-24-generic #46-Ubuntu SMP Thu Apr 10
19:11:08 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux


On 8 April 2015 at 17:17, Neil Horman  wrote:

> On Wed, Apr 08, 2015 at 02:03:05PM +0530, Venkat Thummala wrote:
> > Hi Neil,
> >
> > Thanks for the quick response.
> >
> > The issue got fixed by setting CONFIG_RTE_MACHINE to "default".
> >
> Default is the lowest common demoninator system that dpdk supports (core2
> duo).
> So if you're system is superior to that processor, you're ok.
>
> > Though, this fixed the CRASH issue, I am observing inconsistent behavior
> > with ACL tables.
> >
> > With the same traffic and same ACL rule, on ONE machine the ACL rule is
> > being HIT, where as on the OTHER machine the rule never HITS.
> >
> > Is this because of the "default" machine option?
> >
> Possibly, the machine type changes the method of lookup for ACL rules,
> though it
> should remain consistent in terms of which rules are hit.
> > Regards
> > Venkat
> >
> >
> > On 7 April 2015 at 20:05, Neil Horman  wrote:
> >
> > > On Tue, Apr 07, 2015 at 05:30:15PM +0530, Venkat Thummala wrote:
> > > > Attaching the CPU Info.
> > > >
> > > > On 7 April 2015 at 17:28, Venkat Thummala <
> > > venkat.thummala.1978 at gmail.com>
> > > > wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > I have built a DPDK application [based on version 2.0] and run on
> the
> > > > > native machine successfully.
> > > > >
> > > > > I have tried running the binary on a different machine, but it
> > > resulted in
> > > > > a CRASH with the following back trace.
> > > > >
> > > > > Please find the CPU info of the machines from the attachment.
> > > > >
> > > > > cpuinfo-1 - Native Machine
> > > > > cpuinfo-2 - Non Native Machine
> > > > >
> > > > > Could someone please help me in understanding the issue here and
> > > making it
> > > > > work?
> > > > >
> > > > > Regards
> > > > > Venkat
> > > > >
> > > > > Program terminated with signal SIGILL, Illegal instruction.
> > > > > #0  0x004209f2 in rte_cpu_get_flag_enabled
> (feature=2147483656,
> > > > > feature 

[dpdk-dev] [PATCH 4/4] rte_ethdev: remove unnecessary paren on return

[Stephen Hemminger]

The Linux style is not to put extra useless paren's around
the expression passed to return statement.

Signed-off-by: Stephen Hemminger 
---
 lib/librte_ether/rte_ethdev.c | 298 +-
 1 file changed, 149 insertions(+), 149 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 416e778..b856100 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -81,7 +81,7 @@
 #define PROC_PRIMARY_OR_ERR_RET(retval) do { \
if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
-   return (retval); \
+   return (retval);\
} \
 } while (0)
 #define PROC_PRIMARY_OR_RET() do { \
@@ -95,7 +95,7 @@
 #define FUNC_PTR_OR_ERR_RET(func, retval) do { \
if ((func) == NULL) { \
PMD_DEBUG_TRACE("Function not supported\n"); \
-   return (retval); \
+   return (retval); \
} \
 } while (0)
 #define FUNC_PTR_OR_RET(func) do { \
@@ -325,7 +325,7 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
/* Invoke PMD device initialization function */
diag = (*eth_drv->eth_dev_init)(eth_dev);
if (diag == 0)
-   return (0);
+   return 0;

PMD_DEBUG_TRACE("driver %s: eth_dev_init(vendor_id=0x%u device_id=0x%x)"
" failed\n", pci_drv->name,
@@ -422,7 +422,7 @@ rte_eth_dev_socket_id(uint8_t port_id)
 uint8_t
 rte_eth_dev_count(void)
 {
-   return (nb_ports);
+   return nb_ports;
 }

 /* So far, DPDK hotplug function only supports linux */
@@ -763,7 +763,7 @@ rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, 
uint16_t nb_queues)

}
dev->data->nb_rx_queues = nb_queues;
-   return (0);
+   return 0;
 }

 int
@@ -907,7 +907,7 @@ rte_eth_dev_tx_queue_config(struct rte_eth_dev *dev, 
uint16_t nb_queues)

}
dev->data->nb_tx_queues = nb_queues;
-   return (0);
+   return 0;
 }

 static int
@@ -954,7 +954,7 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t 
nb_rx_q, uint16_t nb_tx_q,
port_id,
dev_conf->rxmode.mq_mode,
dev_conf->txmode.mq_mode);
-   return (-EINVAL);
+   return -EINVAL;
}

switch (dev_conf->rxmode.mq_mode) {
@@ -965,7 +965,7 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t 
nb_rx_q, uint16_t nb_tx_q,
" SRIOV active, "
"unsupported VMDQ mq_mode rx %u\n",
port_id, dev_conf->rxmode.mq_mode);
-   return (-EINVAL);
+   return -EINVAL;
case ETH_MQ_RX_RSS:
PMD_DEBUG_TRACE("ethdev port_id=%" PRIu8
" SRIOV active, "
@@ -1001,7 +1001,7 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t 
nb_rx_q, uint16_t nb_tx_q,
" SRIOV active, "
"unsupported VMDQ mq_mode tx %u\n",
port_id, dev_conf->txmode.mq_mode);
-   return (-EINVAL);
+   return -EINVAL;
default: /* ETH_MQ_TX_VMDQ_ONLY or ETH_MQ_TX_NONE */
/* if nothing mq mode configure, use default scheme */
dev->data->dev_conf.txmode.mq_mode = 
ETH_MQ_TX_VMDQ_ONLY;
@@ -1014,7 +1014,7 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t 
nb_rx_q, uint16_t nb_tx_q,
PMD_DEBUG_TRACE("ethdev port_id=%d SRIOV active, "
"queue number must less equal to %d\n",
port_id, 
RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool);
-   return (-EINVAL);
+   return -EINVAL;
}
} else {
/* For vmdb+dcb mode check our configuration before we go 
further */
@@ -1025,7 +1025,7 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t 
nb_rx_q, uint16_t nb_tx_q,
PMD_DEBUG_TRACE("ethdev port_id=%d VMDQ+DCB, 
nb_rx_q "
"!= %d\n",
port_id, 
ETH_VMDQ_DCB_NUM_QUEUES);
-   return (-EINVAL);
+   return -EINVAL;
}
conf = &(dev_conf->rx_adv_conf.vmdq_dcb_conf);
if (!(conf->nb_queue_pools == ETH_16_POOLS ||
@@ -1033,7 +1033,7 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t 
nb_rx_q, uint16_t 

[dpdk-dev] [PATCH 3/4] rte_ethdev: make tables const

[Stephen Hemminger]

The statistics tables and null mac address should be immutable.
Fix up get_addr routines to accept const args.

Signed-off-by: Stephen Hemminger 
---
 lib/librte_ether/rte_ethdev.c | 44 +--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 56e22ea..416e778 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -119,34 +119,34 @@ struct rte_eth_xstats_name_off {
unsigned offset;
 };

-static struct rte_eth_xstats_name_off rte_stats_strings[] = {
-{"rx_packets", offsetof(struct rte_eth_stats, ipackets)},
-{"tx_packets", offsetof(struct rte_eth_stats, opackets)},
-{"rx_bytes", offsetof(struct rte_eth_stats, ibytes)},
-{"tx_bytes", offsetof(struct rte_eth_stats, obytes)},
-{"tx_errors", offsetof(struct rte_eth_stats, oerrors)},
-{"rx_missed_errors", offsetof(struct rte_eth_stats, imissed)},
-{"rx_crc_errors", offsetof(struct rte_eth_stats, ibadcrc)},
-{"rx_bad_length_errors", offsetof(struct rte_eth_stats, ibadlen)},
-{"rx_errors", offsetof(struct rte_eth_stats, ierrors)},
-{"alloc_rx_buff_failed", offsetof(struct rte_eth_stats, rx_nombuf)},
-{"fdir_match", offsetof(struct rte_eth_stats, fdirmatch)},
-{"fdir_miss", offsetof(struct rte_eth_stats, fdirmiss)},
-{"tx_flow_control_xon", offsetof(struct rte_eth_stats, tx_pause_xon)},
-{"rx_flow_control_xon", offsetof(struct rte_eth_stats, rx_pause_xon)},
-{"tx_flow_control_xoff", offsetof(struct rte_eth_stats, 
tx_pause_xoff)},
-{"rx_flow_control_xoff", offsetof(struct rte_eth_stats, 
rx_pause_xoff)},
+static const struct rte_eth_xstats_name_off rte_stats_strings[] = {
+   {"rx_packets", offsetof(struct rte_eth_stats, ipackets)},
+   {"tx_packets", offsetof(struct rte_eth_stats, opackets)},
+   {"rx_bytes", offsetof(struct rte_eth_stats, ibytes)},
+   {"tx_bytes", offsetof(struct rte_eth_stats, obytes)},
+   {"tx_errors", offsetof(struct rte_eth_stats, oerrors)},
+   {"rx_missed_errors", offsetof(struct rte_eth_stats, imissed)},
+   {"rx_crc_errors", offsetof(struct rte_eth_stats, ibadcrc)},
+   {"rx_bad_length_errors", offsetof(struct rte_eth_stats, ibadlen)},
+   {"rx_errors", offsetof(struct rte_eth_stats, ierrors)},
+   {"alloc_rx_buff_failed", offsetof(struct rte_eth_stats, rx_nombuf)},
+   {"fdir_match", offsetof(struct rte_eth_stats, fdirmatch)},
+   {"fdir_miss", offsetof(struct rte_eth_stats, fdirmiss)},
+   {"tx_flow_control_xon", offsetof(struct rte_eth_stats, tx_pause_xon)},
+   {"rx_flow_control_xon", offsetof(struct rte_eth_stats, rx_pause_xon)},
+   {"tx_flow_control_xoff", offsetof(struct rte_eth_stats, tx_pause_xoff)},
+   {"rx_flow_control_xoff", offsetof(struct rte_eth_stats, rx_pause_xoff)},
 };
 #define RTE_NB_STATS (sizeof(rte_stats_strings) / sizeof(rte_stats_strings[0]))

-static struct rte_eth_xstats_name_off rte_rxq_stats_strings[] = {
+static const struct rte_eth_xstats_name_off rte_rxq_stats_strings[] = {
{"rx_packets", offsetof(struct rte_eth_stats, q_ipackets)},
{"rx_bytes", offsetof(struct rte_eth_stats, q_ibytes)},
 };
 #define RTE_NB_RXQ_STATS (sizeof(rte_rxq_stats_strings) /  \
sizeof(rte_rxq_stats_strings[0]))

-static struct rte_eth_xstats_name_off rte_txq_stats_strings[] = {
+static const struct rte_eth_xstats_name_off rte_txq_stats_strings[] = {
{"tx_packets", offsetof(struct rte_eth_stats, q_opackets)},
{"tx_bytes", offsetof(struct rte_eth_stats, q_obytes)},
{"tx_errors", offsetof(struct rte_eth_stats, q_errors)},
@@ -2651,7 +2651,7 @@ rte_eth_led_off(uint8_t port_id)
  * an empty spot.
  */
 static int
-get_mac_addr_index(uint8_t port_id, struct ether_addr *addr)
+get_mac_addr_index(uint8_t port_id, const struct ether_addr *addr)
 {
struct rte_eth_dev_info dev_info;
struct rte_eth_dev *dev = _eth_devices[port_id];
@@ -2666,7 +2666,7 @@ get_mac_addr_index(uint8_t port_id, struct ether_addr 
*addr)
return -1;
 }

-static struct ether_addr null_mac_addr = {{0, 0, 0, 0, 0, 0}};
+static const struct ether_addr null_mac_addr;

 int
 rte_eth_dev_mac_addr_add(uint8_t port_id, struct ether_addr *addr,
@@ -2792,7 +2792,7 @@ rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
  * an empty spot.
  */
 static int
-get_hash_mac_addr_index(uint8_t port_id, struct ether_addr *addr)
+get_hash_mac_addr_index(uint8_t port_id, const struct ether_addr *addr)
 {
struct rte_eth_dev_info dev_info;
struct rte_eth_dev *dev = _eth_devices[port_id];
-- 
2.1.4

[dpdk-dev] [PATCH 2/4] rte_ethdev: whitespace cleanup

[Stephen Hemminger]

Fix space after keywords, and other missing whitespace.

Signed-off-by: Stephen Hemminger 
---
 lib/librte_ether/rte_ethdev.c | 191 ++
 lib/librte_ether/rte_ethdev.h |  57 ++---
 2 files changed, 128 insertions(+), 120 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 3120c3a..56e22ea 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -83,13 +83,13 @@
PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
return (retval); \
} \
-} while(0)
+} while (0)
 #define PROC_PRIMARY_OR_RET() do { \
if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
return; \
} \
-} while(0)
+} while (0)

 /* Macros to check for invlaid function pointers in dev_ops structure */
 #define FUNC_PTR_OR_ERR_RET(func, retval) do { \
@@ -97,13 +97,13 @@
PMD_DEBUG_TRACE("Function not supported\n"); \
return (retval); \
} \
-} while(0)
+} while (0)
 #define FUNC_PTR_OR_RET(func) do { \
if ((func) == NULL) { \
PMD_DEBUG_TRACE("Function not supported\n"); \
return; \
} \
-} while(0)
+} while (0)

 static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
 struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
@@ -185,7 +185,7 @@ rte_eth_dev_data_alloc(void)
const unsigned flags = 0;
const struct rte_memzone *mz;

-   if (rte_eal_process_type() == RTE_PROC_PRIMARY){
+   if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
mz = rte_memzone_reserve(MZ_RTE_ETH_DEV_DATA,
RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data),
rte_socket_id(), flags);
@@ -257,7 +257,7 @@ rte_eth_dev_allocate(const char *name, enum 
rte_eth_dev_type type)

 static int
 rte_eth_dev_create_unique_device_name(char *name, size_t size,
-   struct rte_pci_device *pci_dev)
+ struct rte_pci_device *pci_dev)
 {
int ret;

@@ -265,8 +265,8 @@ rte_eth_dev_create_unique_device_name(char *name, size_t 
size,
return -EINVAL;

ret = snprintf(name, size, "%d:%d.%d",
-   pci_dev->addr.bus, pci_dev->addr.devid,
-   pci_dev->addr.function);
+  pci_dev->addr.bus, pci_dev->addr.devid,
+  pci_dev->addr.function);
if (ret < 0)
return ret;
return 0;
@@ -303,7 +303,7 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
if (eth_dev == NULL)
return -ENOMEM;

-   if (rte_eal_process_type() == RTE_PROC_PRIMARY){
+   if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
eth_dev->data->dev_private = rte_zmalloc("ethdev private 
structure",
  eth_drv->dev_private_size,
  RTE_CACHE_LINE_SIZE);
@@ -699,9 +699,9 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
ret = rte_eth_dev_detach_pdev(port_id, );
if (ret == 0)
snprintf(name, RTE_ETH_NAME_MAX_LEN,
-   "%04x:%02x:%02x.%d",
-   addr.domain, addr.bus,
-   addr.devid, addr.function);
+"%04x:%02x:%02x.%d",
+addr.domain, addr.bus,
+addr.devid, addr.function);

return ret;
} else
@@ -710,7 +710,7 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 #else /* RTE_LIBRTE_EAL_HOTPLUG */
 int
 rte_eth_dev_attach(const char *devargs __rte_unused,
-   uint8_t *port_id __rte_unused)
+  uint8_t *port_id __rte_unused)
 {
RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n");
return -1;
@@ -719,7 +719,7 @@ rte_eth_dev_attach(const char *devargs __rte_unused,
 /* detach the device, then store the name of the device */
 int
 rte_eth_dev_detach(uint8_t port_id __rte_unused,
-   char *name __rte_unused)
+  char *name __rte_unused)
 {
RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n");
return -1;
@@ -754,6 +754,7 @@ rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, 
uint16_t nb_queues)
return -(ENOMEM);
if (nb_queues > old_nb_queues) {
uint16_t new_qs = nb_queues - old_nb_queues;
+
memset(rxq + old_nb_queues, 0,
sizeof(rxq[0]) * new_qs);
}
@@ -897,6 +898,7 @@ rte_eth_dev_tx_queue_config(struct rte_eth_dev *dev, 
uint16_t nb_queues)
return -ENOMEM;
if (nb_queues > old_nb_queues) {
 

[dpdk-dev] [PATCH 1/4] rte_ethdev: remove extra inline

[Stephen Hemminger]

There is no reason to inline functions that are not in the critical
path.

Signed-off-by: Stephen Hemminger 
---
 lib/librte_ether/rte_ethdev.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e20cca5..3120c3a 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -179,7 +179,7 @@ enum {
DEV_ATTACHED
 };

-static inline void
+static void
 rte_eth_dev_data_alloc(void)
 {
const unsigned flags = 0;
@@ -255,7 +255,7 @@ rte_eth_dev_allocate(const char *name, enum 
rte_eth_dev_type type)
return eth_dev;
 }

-static inline int
+static int
 rte_eth_dev_create_unique_device_name(char *name, size_t size,
struct rte_pci_device *pci_dev)
 {
@@ -2415,7 +2415,7 @@ rte_eth_dev_priority_flow_ctrl_set(uint8_t port_id, 
struct rte_eth_pfc_conf *pfc
return (-ENOTSUP);
 }

-static inline int
+static int
 rte_eth_check_reta_mask(struct rte_eth_rss_reta_entry64 *reta_conf,
uint16_t reta_size)
 {
@@ -2439,7 +2439,7 @@ rte_eth_check_reta_mask(struct rte_eth_rss_reta_entry64 
*reta_conf,
return -EINVAL;
 }

-static inline int
+static int
 rte_eth_check_reta_entry(struct rte_eth_rss_reta_entry64 *reta_conf,
 uint16_t reta_size,
 uint8_t max_rxq)
@@ -2648,7 +2648,7 @@ rte_eth_led_off(uint8_t port_id)
  * Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to find
  * an empty spot.
  */
-static inline int
+static int
 get_mac_addr_index(uint8_t port_id, struct ether_addr *addr)
 {
struct rte_eth_dev_info dev_info;
@@ -2789,7 +2789,7 @@ rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
  * Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to find
  * an empty spot.
  */
-static inline int
+static int
 get_hash_mac_addr_index(uint8_t port_id, struct ether_addr *addr)
 {
struct rte_eth_dev_info dev_info;
-- 
2.1.4

[dpdk-dev] [PATCH 0/4] rte_ethdev: cleanups

[Stephen Hemminger]

A bunch of small (almost trivial) patches to fix style and other
issues in the base Ethernet driver interface code.

Stephen Hemminger (4):
  rte_ethdev: remove extra inline
  rte_ethdev: whitespace cleanup
  rte_ethdev: make tables const
  rte_ethdev: remove unnecessary paren on return

 lib/librte_ether/rte_ethdev.c | 545 +-
 lib/librte_ether/rte_ethdev.h |  57 ++---
 2 files changed, 305 insertions(+), 297 deletions(-)

-- 
2.1.4

[dpdk-dev] Polling too often at lower packet rates?

[Stephen Hemminger]

On Thu, 9 Apr 2015 15:26:23 -0300
Aaron Campbell  wrote:

> Hi Stephen,
> 
> Thanks, that was an informative talk.  In this case, are you referring to 
> your comments about the thermal budget?
> 
> That?s definitely interesting, but there must be more to it than that.  
> Again, if I loop over all 6 ports (i.e., continue to keep the CPU busy), it 
> works around the problem.
> 
> I agree that adaptive polling makes sense and will look into it.  But will 
> still take any further ideas on what is going on here.
> 
> -Aaron

Your excess polling consumes PCI bandwidth which is a fixed resource.

[dpdk-dev] tools brainstorming

[Stephen Hemminger]

On Thu, 9 Apr 2015 21:10:19 +
"Wiles, Keith"  wrote:

> 
> 
> On 4/9/15, 2:38 PM, "Jay Rolette"  wrote:
> 
> >On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman  wrote:
> >
> >> On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote:
> >> > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
> >> > stephen at networkplumber.org> wrote:
> >> >
> >> > > On Wed, 8 Apr 2015 16:29:54 -0600
> >> > > Jay Rolette  wrote:
> >> > >
> >> > > > "C comments" includes //, right? It's been part of the C standard
> >> for a
> >> > > long time now...
> >> > >
> >> > > Yes but.
> >> > > I like to use checkpatch and checkpatch enforces kernel style which
> >> does
> >> > > not allow // for
> >> > > comments.
> >> > >
> >> >
> >> > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
> >> > requirement to follow all of its rules
> >> >
> >>
> >> Doesn't that beg the question, why?  I understand the DPDK isn't the
> >> kernel, but
> >> we're not talking about clarity of code, not anything functional to that
> >> code.
> >> It seems we would be better served by just taking something that works
> >>here
> >> rather than re-inventing the wheel and digging into the minuate of what
> >> type of
> >> comments should be allowed (unless there is a compelling reason to
> >>change
> >> it
> >> that supercedes the avilable tools).  If not checkpath, then some other
> >> tool,
> >> but It seems to me that coding style is one of those things where we can
> >> bend to
> >> the tool rather than taking the time to make the tool do exactly whats
> >> desired,
> >> at least until someone gets the time to modify it.
> >>
> >
> >Fair question.
> >
> >It depends a bit on how much you want to encourage patch contributions. Is
> >it worth adding more pain for folks trying to contribute patches for
> >things
> >like this?
> >
> >Should we force someone to spend time redoing a patch because of which way
> >they do their parenthesis? What about number of spaces to indent code? //
> >vs /* */ comments? None of these matter functionally and they don't affect
> >maintenance generally.
> >
> >If someone is modifying existing code, then yeah, they should follow the
> >prevailing style (indention level, brace alignment, etc.) of the file they
> >are in. It helps readability, which makes maintenance easier. However,
> >IMO,
> >mixing // and /* */ for comments doesn't affect the readability of the
> >source.
> >
> >I know if I submit a patch and the only feedback is that I should have
> >used
> >/* */ for comments, I'm extremely unlikely spend extra time to resubmit
> >the
> >patch for pedantry.
> 
> I looked at checkpatch.pl for few minutes and the code does check for C99
> comments and adding a command line option to allow C99 comments could
> pretty simple. I found the code around line 3048 or search for C99, it is
> possible it could accepted back into Linux as long as the default option
> was to not allow C99 comments.
> 
> Allowing C99 comments would be nice and the only problem I could see if
> some compiler has a problem with them. I believe all of the compilers we
> support allow C99 comments.
> 
> The only other reason to allow them is if we add some open source code in
> the future to DPDK which has C99 comments and if would be a pain to have
> to convert that code every time the open source group released a new
> version. It does open that path IMO.
> 
> Regards,
> ++Keith
> >
> 

But forking a tool means maintaining that tool.

[dpdk-dev] [PATCH v4 0/7] Move EAL common function

[Ravi Kerur]

Thomas, Any additional tests i need to do on linux/bsd for v5 series please
let me know.

Thanks,
Ravi

On Tue, Mar 31, 2015 at 11:39 AM, Ravi Kerur  wrote:

> I will work on it and send out patches by next week. If you need it
> earlier please let me know.
>
> Thanks,
> Ravi
>
> On Mon, Mar 30, 2015 at 4:24 PM, Thomas Monjalon <
> thomas.monjalon at 6wind.com> wrote:
>
>> 2015-03-30 16:15, Ravi Kerur:
>> > On Mon, Mar 30, 2015 at 2:29 PM, Thomas Monjalon <
>> thomas.monjalon at 6wind.com> wrote:
>> > > Sorry for not integrating this big refactoring in release 2.0.
>> > > It wasn't easy to merge it during development of some EAL features.
>> > > And it's now too risky to rework it before the release.
>> > > I think this clean-up is needed and should be done at the beginning
>> of 2.1
>> > > cycle. We just need to rebase it and remove the "ifdef BSD/LINUX"
>> which was
>> > > introduced in pci code.
>> > >
>> > > Do you think you'll have time for it?
>> >
>> > Sure, please let me know when it is needed I can work on it.
>>
>> 2.1 cycle is starting in few days.
>> So the sooner is the better.
>>
>> Thanks a lot Ravi
>>
>
>

[dpdk-dev] [PATCH v5 6/8] Move common functions in eal_pci.c

[Ravi Kerur]

Changes in v5
Rebase to latest code.
Removed RTE_EXEC_ENV_BSDAPP from earlier changes.

Changes in v4
Move common functions in eal_pci.c to librte_eal/common/
eal_common_pci.c file.

Following functions are moved to eal_common_pci.c file.

void *pci_map_resource(void *requested_addr, const int vfio_fd,
  const char *devname, off_t offset, size_t size);
int pci_addr_comparison(struct rte_pci_addr *addr,
struct rte_pci_addr *addr2);
int rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
struct rte_pci_device *dev);

Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in
common function.
Fix checkpatch warnings and errors.

Changes in v3
N/A

Changes in v2
N/A

Changes in v1
N/A

Signed-off-by: Ravi Kerur 
---
 lib/librte_eal/bsdapp/eal/eal_pci.c| 122 ---
 lib/librte_eal/common/eal_common_pci.c | 130 -
 lib/librte_eal/common/eal_private.h|  48 +++
 lib/librte_eal/linuxapp/eal/eal_pci.c  | 100 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |   6 --
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  |  36 ++--
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |  17 ++--
 7 files changed, 212 insertions(+), 247 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c 
b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 30f0232..5669592 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -111,7 +111,7 @@ static struct rte_tailq_elem rte_uio_tailq = {
 EAL_REGISTER_TAILQ(rte_uio_tailq)

 /* unbind kernel driver for this device */
-static int
+int
 pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
 {
RTE_LOG(ERR, EAL, "RTE_PCI_DRV_FORCE_UNBIND flag is not implemented "
@@ -119,45 +119,6 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev 
__rte_unused)
return -ENOTSUP;
 }

-/* map a particular resource from a file */
-static void *
-pci_map_resource(void *requested_addr, const char *devname, off_t offset,
-size_t size)
-{
-   int fd;
-   void *mapaddr;
-
-   /*
-* open devname, to mmap it
-*/
-   fd = open(devname, O_RDWR);
-   if (fd < 0) {
-   RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
-   devname, strerror(errno));
-   goto fail;
-   }
-
-   /* Map the PCI memory resource of device */
-   mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
-   MAP_SHARED, fd, offset);
-   close(fd);
-   if (mapaddr == MAP_FAILED ||
-   (requested_addr != NULL && mapaddr != requested_addr)) {
-   RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
-   " %s (%p)\n", __func__, devname, fd, requested_addr,
-   (unsigned long)size, (unsigned long)offset,
-   strerror(errno), mapaddr);
-   goto fail;
-   }
-
-   RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
-
-   return mapaddr;
-
-fail:
-   return NULL;
-}
-
 static int
 pci_uio_map_secondary(struct rte_pci_device *dev)
 {
@@ -172,10 +133,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
continue;

for (i = 0; i != uio_res->nb_maps; i++) {
-   if (pci_map_resource(uio_res->maps[i].addr,
+   if (pci_map_resource(uio_res->maps[i].addr, INT_MIN,
 uio_res->path,
 (off_t)uio_res->maps[i].offset,
-(size_t)uio_res->maps[i].size)
+(size_t)uio_res->maps[i].size, 0)
!= uio_res->maps[i].addr) {
RTE_LOG(ERR, EAL,
"Cannot mmap device resource\n");
@@ -190,7 +151,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 }

 /* map the PCI resource of a PCI device in virtual memory */
-static int
+int
 pci_uio_map_resource(struct rte_pci_device *dev)
 {
int i, j;
@@ -257,8 +218,9 @@ pci_uio_map_resource(struct rte_pci_device *dev)
maps[j].phaddr = dev->mem_resource[i].phys_addr;
maps[j].size = dev->mem_resource[i].len;
if (maps[j].addr != NULL ||
-   (mapaddr = pci_map_resource(NULL, devname, (off_t)offset,
-   (size_t)maps[j].size)
+   (mapaddr = pci_map_resource(NULL, INT_MIN, devname,
+   (off_t)offset,
+   (size_t)maps[j].size, 0)
) == NULL) {
rte_free(uio_res);
return (-1);
@@ -274,6 +236,13 @@ pci_uio_map_resource(struct rte_pci_device 

[dpdk-dev] [PATCH v5 5/8] Move common functions in eal_memory.c

[Ravi Kerur]

Changes in v5
Rebase to latest code.

Changes in v4
Make rte_eal_hugepage_init and rte_eal_hugepage_attach as
wrapper functions for BSD.

Changes in v3
Changed subject to be more explicit on file name inclusion.

Changes in v2
Use common function names rte_eal_hugepage_init and
rte_eal_hugepage_attach for BSD and Linux. Update comments about its
actuality in function declaration.

Changes in v1
Move common functions in eal_memory.c to librte_eal/common/
eal_common_memory.c file.

Following functions are moved to eal_common_memory.c file

static int rte_eal_memdevice_init(void); int rte_eal_memory_init(void);

Fix checkpatch warnings and errors.

Signed-off-by: Ravi Kerur 
---
 lib/librte_eal/bsdapp/eal/eal_memory.c| 47 +++
 lib/librte_eal/common/eal_common_memory.c | 38 +++--
 lib/librte_eal/common/eal_private.h   | 22 +++
 lib/librte_eal/linuxapp/eal/eal_memory.c  | 36 ++-
 4 files changed, 76 insertions(+), 67 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c 
b/lib/librte_eal/bsdapp/eal/eal_memory.c
index 33ebd0f..77c27b3 100644
--- a/lib/librte_eal/bsdapp/eal/eal_memory.c
+++ b/lib/librte_eal/bsdapp/eal/eal_memory.c
@@ -59,7 +59,7 @@ rte_mem_virt2phy(const void *virtaddr)
return RTE_BAD_PHYS_ADDR;
 }

-static int
+static inline int
 rte_eal_contigmem_init(void)
 {
struct rte_mem_config *mcfg;
@@ -131,7 +131,16 @@ rte_eal_contigmem_init(void)
return 0;
 }

-static int
+/*
+ * Wrapper function to initialize contigmem.
+ */
+int
+rte_eal_hugepage_init(void)
+{
+   return rte_eal_contigmem_init();
+}
+
+static inline int
 rte_eal_contigmem_attach(void)
 {
const struct hugepage_info *hpi;
@@ -192,35 +201,11 @@ error:
return -1;
 }

-
-static int
-rte_eal_memdevice_init(void)
-{
-   struct rte_config *config;
-
-   if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-   return 0;
-
-   config = rte_eal_get_configuration();
-   config->mem_config->nchannel = internal_config.force_nchannel;
-   config->mem_config->nrank = internal_config.force_nrank;
-
-   return 0;
-}
-
-/* init memory subsystem */
+/*
+ * Wrapper function to attach contigmem.
+ */
 int
-rte_eal_memory_init(void)
+rte_eal_hugepage_attach(void)
 {
-   RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n");
-   const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ?
-   rte_eal_contigmem_init() :
-   rte_eal_contigmem_attach();
-   if (retval < 0)
-   return -1;
-
-   if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0)
-   return -1;
-
-   return 0;
+   return rte_eal_contigmem_attach();
 }
diff --git a/lib/librte_eal/common/eal_common_memory.c 
b/lib/librte_eal/common/eal_common_memory.c
index 9a07b1e..10ff0bc 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -45,6 +45,7 @@
 #include 

 #include "eal_private.h"
+#include "eal_internal_cfg.h"

 /*
  * Return a pointer to a read-only table of struct rte_physmem_desc
@@ -69,7 +70,7 @@ rte_eal_get_physmem_size(void)
/* get pointer to global configuration */
mcfg = rte_eal_get_configuration()->mem_config;

-   for (i=0; imemseg[i].addr == NULL)
break;

@@ -89,7 +90,7 @@ rte_dump_physmem_layout(FILE *f)
/* get pointer to global configuration */
mcfg = rte_eal_get_configuration()->mem_config;

-   for (i=0; imemseg[i].addr == NULL)
break;

@@ -118,3 +119,36 @@ unsigned rte_memory_get_nrank(void)
 {
return rte_eal_get_configuration()->mem_config->nrank;
 }
+
+static int
+rte_eal_memdevice_init(void)
+{
+   struct rte_config *config;
+
+   if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+   return 0;
+
+   config = rte_eal_get_configuration();
+   config->mem_config->nchannel = internal_config.force_nchannel;
+   config->mem_config->nrank = internal_config.force_nrank;
+
+   return 0;
+}
+
+/* init memory subsystem */
+int
+rte_eal_memory_init(void)
+{
+   RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n");
+   const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ?
+   rte_eal_hugepage_init() :
+   rte_eal_hugepage_attach();
+
+   if (retval < 0)
+   return -1;
+
+   if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0)
+   return -1;
+
+   return 0;
+}
diff --git a/lib/librte_eal/common/eal_private.h 
b/lib/librte_eal/common/eal_private.h
index 7ee1eb2..fde1fc9 100644
--- 

[dpdk-dev] [PATCH v5 4/8] Move common functions in eal_timer.c

[Ravi Kerur]

Changes in v5
Rebase to latest code.

Changes in v4
Removed extern declaration of eal_tsc_resolution_hz,
instead provided _set_ API.
Make set_tsc_freq_from_clock as wrapper function for BSD.

Changes in v3
Changed subject to be more explicit on file name inclusion.

Changes in v2
Use common function name set_tsc_freq_from_sysctl for BSD and Linux.
Update comments about its actuality in function declaration.

Changes in v1
Move common functions in eal_timer.c to librte_eal/common/
eal_common_timer.c file.

Following functions are  moved to eal_common_timer.c file

void rte_delay_us(unsigned us);
uint64_t rte_get_tsc_hz(void);
static void set_tsc_freq_fallback(void);
void set_tsc_freq(void);

Makefile changes to reflect new file added.
Fix checkpatch warnings and errors.

Signed-off-by: Ravi Kerur 
---
 lib/librte_eal/bsdapp/eal/Makefile   |   1 +
 lib/librte_eal/bsdapp/eal/eal_timer.c|  52 +++-
 lib/librte_eal/common/eal_common_timer.c | 101 +++
 lib/librte_eal/common/eal_private.h  |  24 
 lib/librte_eal/linuxapp/eal/Makefile |   1 +
 lib/librte_eal/linuxapp/eal/eal_timer.c  |  55 ++---
 6 files changed, 139 insertions(+), 95 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_timer.c

diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index a28ead3..48ba39a 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -81,6 +81,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_system.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_runtime.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_lcore.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_timer.c

 CFLAGS_eal.o := -D_GNU_SOURCE
 #CFLAGS_eal_thread.o := -D_GNU_SOURCE
diff --git a/lib/librte_eal/bsdapp/eal/eal_timer.c 
b/lib/librte_eal/bsdapp/eal/eal_timer.c
index 7147abe..ee7a5ac 100644
--- a/lib/librte_eal/bsdapp/eal/eal_timer.c
+++ b/lib/librte_eal/bsdapp/eal/eal_timer.c
@@ -55,29 +55,12 @@

 enum timer_source eal_timer_source = EAL_TIMER_TSC;

-/* The frequency of the RDTSC timer resolution */
-static uint64_t eal_tsc_resolution_hz = 0;
-
-void
-rte_delay_us(unsigned us)
-{
-   const uint64_t start = rte_get_timer_cycles();
-   const uint64_t ticks = (uint64_t)us * rte_get_timer_hz() / 1E6;
-   while ((rte_get_timer_cycles() - start) < ticks)
-   rte_pause();
-}
-
-uint64_t
-rte_get_tsc_hz(void)
-{
-   return eal_tsc_resolution_hz;
-}
-
 static int
 set_tsc_freq_from_sysctl(void)
 {
size_t sz;
int tmp;
+   uint64_t tsc_hz;

sz = sizeof(tmp);
tmp = 0;
@@ -94,42 +77,23 @@ set_tsc_freq_from_sysctl(void)
else if (tmp != 1)
RTE_LOG(WARNING, EAL, "TSC is not invariant\n");

-   sz = sizeof(eal_tsc_resolution_hz);
-   if (sysctlbyname("machdep.tsc_freq", _tsc_resolution_hz, , NULL, 
0)) {
+   sz = sizeof(tsc_hz);
+   if (sysctlbyname("machdep.tsc_freq", _hz, , NULL, 0)) {
RTE_LOG(WARNING, EAL, "%s\n", strerror(errno));
return -1;
}
+   rte_set_tsc_hz(tsc_hz);

return 0;
 }

-static void
-set_tsc_freq_fallback(void)
-{
-   RTE_LOG(WARNING, EAL, "WARNING: clock_gettime cannot use "
-   "CLOCK_MONOTONIC_RAW and HPET is not available"
-   " - clock timings may be less accurate.\n");
-   /* assume that the sleep(1) will sleep for 1 second */
-   uint64_t start = rte_rdtsc();
-   sleep(1);
-   eal_tsc_resolution_hz = rte_rdtsc() - start;
-}
-
 /*
- * This function measures the TSC frequency. It uses a variety of approaches.
- *
- * 1. Read the TSC frequency value provided by the kernel
- * 2. If above does not work, just sleep for 1 second and tune off that,
- *printing a warning about inaccuracy of timing
+ * Wrapper function to get TSC frequency from sysctl.
  */
-static void
-set_tsc_freq(void)
+int
+set_tsc_freq_from_clock(void)
 {
-   if (set_tsc_freq_from_sysctl() < 0)
-   set_tsc_freq_fallback();
-
-   RTE_LOG(INFO, EAL, "TSC frequency is ~%"PRIu64" KHz\n",
-   eal_tsc_resolution_hz/1000);
+   return set_tsc_freq_from_sysctl();
 }

 int
diff --git a/lib/librte_eal/common/eal_common_timer.c 
b/lib/librte_eal/common/eal_common_timer.c
new file mode 100644
index 000..2a511d0
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_timer.c
@@ -0,0 +1,101 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the 

[dpdk-dev] [PATCH v5 3/8] Move common functions in eal_lcore.c

[Ravi Kerur]

Changes in v5
Rebase to latest code.

Changes in v4
Implement cpu_detected() for BSD.
Have common RTE_LOG for Linux and BSD in rte_eal_cpu_init().
Remove RTE_EXEC_ENV_BSDAPP in common file.

Changes in v3
Changed subject to be more explicit on file name inclusion.

Changes in v2
None

Changes in v1
Move common function in eal_lcore.c to librte_eal/common/
eal_common_lcore.c file.

Following function is  moved to eal_common_lcore.c file

int rte_eal_cpu_init(void);

Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in
common function.
Makefile changes to reflect new file added.
Fix checkpatch warnings and errors.

Signed-off-by: Ravi Kerur 
---
 lib/librte_eal/bsdapp/eal/Makefile   |   1 +
 lib/librte_eal/bsdapp/eal/eal_lcore.c|  72 +
 lib/librte_eal/common/eal_common_lcore.c | 107 +++
 lib/librte_eal/common/eal_private.h  |  14 
 lib/librte_eal/linuxapp/eal/Makefile |   2 +
 lib/librte_eal/linuxapp/eal/eal_lcore.c  |  66 ++-
 6 files changed, 143 insertions(+), 119 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_lcore.c

diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index d0a7c43..a28ead3 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -80,6 +80,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_system.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_runtime.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_lcore.c

 CFLAGS_eal.o := -D_GNU_SOURCE
 #CFLAGS_eal_thread.o := -D_GNU_SOURCE
diff --git a/lib/librte_eal/bsdapp/eal/eal_lcore.c 
b/lib/librte_eal/bsdapp/eal/eal_lcore.c
index 162fb4f..b47eb1b 100644
--- a/lib/librte_eal/bsdapp/eal/eal_lcore.c
+++ b/lib/librte_eal/bsdapp/eal/eal_lcore.c
@@ -44,11 +44,14 @@
 #include "eal_thread.h"

 /* No topology information available on FreeBSD including NUMA info */
-#define cpu_core_id(X) 0
-#define cpu_socket_id(X) 0
+unsigned
+eal_cpu_core_id(__rte_unused unsigned lcore_id)
+{
+   return 0;
+}

 static int
-get_ncpus(void)
+eal_get_ncpus(void)
 {
int mib[2] = {CTL_HW, HW_NCPU};
int ncpu;
@@ -59,63 +62,18 @@ get_ncpus(void)
return ncpu;
 }

-/*
- * fill the cpu_info structure with as much info as we can get.
- * code is similar to linux version, but sadly available info is less.
- */
-int
-rte_eal_cpu_init(void)
+unsigned
+eal_cpu_socket_id(__rte_unused unsigned cpu_id)
 {
-   /* pointer to global configuration */
-   struct rte_config *config = rte_eal_get_configuration();
-   unsigned lcore_id;
-   unsigned count = 0;
-
-   const unsigned ncpus = get_ncpus();
-   /*
-* Parse the maximum set of logical cores, detect the subset of running
-* ones and enable them by default.
-*/
-   for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-   /* init cpuset for per lcore config */
-   CPU_ZERO(_config[lcore_id].cpuset);
-
-   lcore_config[lcore_id].detected = (lcore_id < ncpus);
-   if (lcore_config[lcore_id].detected == 0) {
-   config->lcore_role[lcore_id] = ROLE_OFF;
-   continue;
-   }
-
-   /* By default, lcore 1:1 map to cpu id */
-   CPU_SET(lcore_id, _config[lcore_id].cpuset);
-
-   /* By default, each detected core is enabled */
-   config->lcore_role[lcore_id] = ROLE_RTE;
-   lcore_config[lcore_id].core_id = cpu_core_id(lcore_id);
-   lcore_config[lcore_id].socket_id = cpu_socket_id(lcore_id);
-   if (lcore_config[lcore_id].socket_id >= RTE_MAX_NUMA_NODES)
-#ifdef RTE_EAL_ALLOW_INV_SOCKET_ID
-   lcore_config[lcore_id].socket_id = 0;
-#else
-   rte_panic("Socket ID (%u) is greater than "
-   "RTE_MAX_NUMA_NODES (%d)\n",
-   lcore_config[lcore_id].socket_id, 
RTE_MAX_NUMA_NODES);
-#endif
-   RTE_LOG(DEBUG, EAL, "Detected lcore %u\n",
-   lcore_id);
-   count++;
-   }
-   /* Set the count of enabled logical cores of the EAL configuration */
-   config->lcore_count = count;
-   RTE_LOG(DEBUG, EAL, "Support maximum %u logical core(s) by 
configuration.\n",
-   RTE_MAX_LCORE);
-   RTE_LOG(DEBUG, EAL, "Detected %u lcore(s)\n", config->lcore_count);
-
return 0;
 }

-unsigned
-eal_cpu_socket_id(__rte_unused unsigned cpu_id)
+/* Check if a cpu is present by the presence of the
+ * cpu information for it.
+ */
+int
+eal_cpu_detected(unsigned lcore_id)
 {
-   return cpu_socket_id(cpu_id);
+   const unsigned ncpus = eal_get_ncpus();
+   return (lcore_id < ncpus);
 }
diff --git 

[dpdk-dev] [PATCH v5 2/8] Move common functions in eal.c

[Ravi Kerur]

Changes in v5
Rebase to latest code.

Changes in v4
Remove eal_externs.h file, instead use  _get_ and _set_ APIS
to access those variables.
Split eal_common.c into eal_common_system.c and
and eal_common_runtime.c
rte_eal prefix functions are moved to _runtime_ and
eal prefix functions are moved to _system_ files respectively.

Changes in v3
Changed subject to be more explicit on file name inclusion.

Changes in v2
In function rte_eal_config_create remove #ifdef _BSDAPP_
and initialize mem_cfg_addr unconditionally.

Changes in v1
Move common functions in eal.c to librte_eal/common/eal_common.c.

Following functions are moved to eal_common.c file.

struct rte_config *rte_eal_get_configuration(void);
int eal_parse_sysfs_value(const char *filename, unsigned long *val);
static void rte_eal_config_create(void);
enum rte_proc_type_t eal_proc_type_detect(void);
void rte_eal_config_init(void);
rte_usage_hook_t rte_set_application_usage_hook(rte_usage_hook_t usage_func);
inline size_t eal_get_hugepage_mem_size(void);
void eal_check_mem_on_local_socket(void);
int sync_func(__attribute__((unused)) void *arg);
inline void rte_eal_mcfg_complete(void);
int rte_eal_has_hugepages(void);
enum rte_lcore_role_t rte_eal_lcore_role(unsigned lcore_id);
enum rte_proc_type_t rte_eal_process_type(void);

Makefile changes to reflect new files added.
Fix checkpatch warnings and errors.

Signed-off-by: Ravi Kerur 
---
 lib/librte_eal/bsdapp/eal/Makefile |   2 +
 lib/librte_eal/bsdapp/eal/eal.c| 271 --
 lib/librte_eal/common/eal_common_runtime.c | 201 
 lib/librte_eal/common/eal_common_system.c  | 203 
 lib/librte_eal/common/eal_hugepages.h  |   1 +
 lib/librte_eal/common/eal_private.h|  78 
 lib/librte_eal/common/include/rte_eal.h|   4 +
 lib/librte_eal/linuxapp/eal/Makefile   |   2 +
 lib/librte_eal/linuxapp/eal/eal.c  | 295 -
 9 files changed, 566 insertions(+), 491 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_runtime.c
 create mode 100644 lib/librte_eal/common/eal_common_system.c

diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index 55971b9..d0a7c43 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -78,6 +78,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_devargs.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_dev.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_system.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_runtime.c

 CFLAGS_eal.o := -D_GNU_SOURCE
 #CFLAGS_eal_thread.o := -D_GNU_SOURCE
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..721dd32 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -80,29 +80,6 @@
 #include "eal_hugepages.h"
 #include "eal_options.h"

-#define MEMSIZE_IF_NO_HUGE_PAGE (64ULL * 1024ULL * 1024ULL)
-
-/* Allow the application to print its usage message too if set */
-static rte_usage_hook_trte_application_usage_hook = NULL;
-/* early configuration structure, when memory config is not mmapped */
-static struct rte_mem_config early_mem_config;
-
-/* define fd variable here, because file needs to be kept open for the
- * duration of the program, as we hold a write lock on it in the primary proc 
*/
-static int mem_cfg_fd = -1;
-
-static struct flock wr_lock = {
-   .l_type = F_WRLCK,
-   .l_whence = SEEK_SET,
-   .l_start = offsetof(struct rte_mem_config, memseg),
-   .l_len = sizeof(early_mem_config.memseg),
-};
-
-/* Address of global and public configuration */
-static struct rte_config rte_config = {
-   .mem_config = _mem_config,
-};
-
 /* internal configuration (per-core) */
 struct lcore_config lcore_config[RTE_MAX_LCORE];

@@ -112,160 +89,57 @@ struct internal_config internal_config;
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;

-/* Return a pointer to the configuration structure */
-struct rte_config *
-rte_eal_get_configuration(void)
-{
-   return _config;
-}
-
-/* parse a sysfs (or other) file containing one integer value */
-int
-eal_parse_sysfs_value(const char *filename, unsigned long *val)
-{
-   FILE *f;
-   char buf[BUFSIZ];
-   char *end = NULL;
-
-   if ((f = fopen(filename, "r")) == NULL) {
-   RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
-   __func__, filename);
-   return -1;
-   }
-
-   if (fgets(buf, sizeof(buf), f) == NULL) {
-   RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
-   __func__, filename);
-   fclose(f);
-   return -1;
-   }
-   *val = strtoul(buf, , 0);
-   

[dpdk-dev] [PATCH v5 1/8] Move common functions in eal_thread.c

[Ravi Kerur]

Changes in v5
Rebase to latest code.

Changes in v4
None

Changes in v3
Changed subject to be more explicit on file name inclusion.

Changes in v2
None

Changes in v1
eal_thread.c has minor differences between Linux and BSD, move
entire file into common directory.
Use RTE_EXEC_ENV_BSDAPP to differentiate on minor differences.
Rename eal_thread.c to eal_common_thread.c
Makefile changes to reflect file move and name change.
Fix checkpatch warnings.

Signed-off-by: Ravi Kerur 
---
 lib/librte_eal/bsdapp/eal/Makefile|   2 +-
 lib/librte_eal/bsdapp/eal/eal_thread.c| 152 
 lib/librte_eal/common/eal_common_thread.c | 158 ++
 lib/librte_eal/linuxapp/eal/eal_thread.c  | 152 
 4 files changed, 159 insertions(+), 305 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index 2357cfa..55971b9 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -87,7 +87,7 @@ CFLAGS_eal_common_log.o := -D_GNU_SOURCE
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
 ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
-CFLAGS_eal_thread.o += -Wno-return-type
+CFLAGS_eal_common_thread.o += -Wno-return-type
 CFLAGS_eal_hpet.o += -Wno-return-type
 endif

diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c 
b/lib/librte_eal/bsdapp/eal/eal_thread.c
index 9a03437..5714b8f 100644
--- a/lib/librte_eal/bsdapp/eal/eal_thread.c
+++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
@@ -35,163 +35,11 @@
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
-#include 
 #include 

-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
 #include "eal_private.h"
 #include "eal_thread.h"

-RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = LCORE_ID_ANY;
-RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
-RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
-
-/*
- * Send a message to a slave lcore identified by slave_id to call a
- * function f with argument arg. Once the execution is done, the
- * remote lcore switch in FINISHED state.
- */
-int
-rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned slave_id)
-{
-   int n;
-   char c = 0;
-   int m2s = lcore_config[slave_id].pipe_master2slave[1];
-   int s2m = lcore_config[slave_id].pipe_slave2master[0];
-
-   if (lcore_config[slave_id].state != WAIT)
-   return -EBUSY;
-
-   lcore_config[slave_id].f = f;
-   lcore_config[slave_id].arg = arg;
-
-   /* send message */
-   n = 0;
-   while (n == 0 || (n < 0 && errno == EINTR))
-   n = write(m2s, , 1);
-   if (n < 0)
-   rte_panic("cannot write on configuration pipe\n");
-
-   /* wait ack */
-   do {
-   n = read(s2m, , 1);
-   } while (n < 0 && errno == EINTR);
-
-   if (n <= 0)
-   rte_panic("cannot read on configuration pipe\n");
-
-   return 0;
-}
-
-/* set affinity for current thread */
-static int
-eal_thread_set_affinity(void)
-{
-   unsigned lcore_id = rte_lcore_id();
-
-   /* acquire system unique id  */
-   rte_gettid();
-
-   /* update EAL thread core affinity */
-   return rte_thread_set_affinity(_config[lcore_id].cpuset);
-}
-
-void eal_thread_init_master(unsigned lcore_id)
-{
-   /* set the lcore ID in per-lcore memory area */
-   RTE_PER_LCORE(_lcore_id) = lcore_id;
-
-   /* set CPU affinity */
-   if (eal_thread_set_affinity() < 0)
-   rte_panic("cannot set affinity\n");
-}
-
-/* main loop of threads */
-__attribute__((noreturn)) void *
-eal_thread_loop(__attribute__((unused)) void *arg)
-{
-   char c;
-   int n, ret;
-   unsigned lcore_id;
-   pthread_t thread_id;
-   int m2s, s2m;
-   char cpuset[RTE_CPU_AFFINITY_STR_LEN];
-
-   thread_id = pthread_self();
-
-   /* retrieve our lcore_id from the configuration structure */
-   RTE_LCORE_FOREACH_SLAVE(lcore_id) {
-   if (thread_id == lcore_config[lcore_id].thread_id)
-   break;
-   }
-   if (lcore_id == RTE_MAX_LCORE)
-   rte_panic("cannot retrieve lcore id\n");
-
-   m2s = lcore_config[lcore_id].pipe_master2slave[0];
-   s2m = lcore_config[lcore_id].pipe_slave2master[1];
-
-   /* set the lcore ID in per-lcore memory area */
-   RTE_PER_LCORE(_lcore_id) = lcore_id;
-
-   /* set CPU affinity */
-   if (eal_thread_set_affinity() < 0)
-   rte_panic("cannot set affinity\n");
-
-   ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
-
-   RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
-   lcore_id, thread_id, cpuset, ret == 0 ? "" : "...");
-
-   /* read on our pipe to get commands */
-   while (1) {
-   void *fct_arg;
-
-   /* wait 

[dpdk-dev] [PATCH v5 0/8] Move common functions in EAL

[Ravi Kerur]

Changes include moving common functions in BSD and Linux into
common directory.
Use appropriate wrapper functions to mask minor differences.

Testing:
Linux - Ubuntu x86_64 14.04
Compilation successful (x86_64-native-linuxapp-gcc).
"make test" results match baseline code.
testpmd utility on I217/I218 Intel chipset.

FreeBSD 10.0 x86_64
Compilation successful (x86_64-native-bsdapp-gcc).
helloworld. 

Ravi Kerur (6):
  Move common functions in eal_thread.c
  Move common functions in eal.c
  Move common functions in eal_lcore.c
  Move common functions in eal_timer.c
  Move common functions in eal_memory.c
  Move common functions in eal_pci.c

 lib/librte_eal/bsdapp/eal/Makefile  |   6 +-
 lib/librte_eal/bsdapp/eal/eal.c | 271 +++---
 lib/librte_eal/bsdapp/eal/eal_lcore.c   |  72 ++
 lib/librte_eal/bsdapp/eal/eal_memory.c  |  47 ++--
 lib/librte_eal/bsdapp/eal/eal_pci.c | 122 ++
 lib/librte_eal/bsdapp/eal/eal_thread.c  | 152 
 lib/librte_eal/bsdapp/eal/eal_timer.c   |  52 +
 lib/librte_eal/common/eal_common_lcore.c| 107 +
 lib/librte_eal/common/eal_common_memory.c   |  38 ++-
 lib/librte_eal/common/eal_common_pci.c  | 130 ++-
 lib/librte_eal/common/eal_common_runtime.c  | 201 
 lib/librte_eal/common/eal_common_system.c   | 203 
 lib/librte_eal/common/eal_common_thread.c   | 158 +
 lib/librte_eal/common/eal_common_timer.c| 101 
 lib/librte_eal/common/eal_hugepages.h   |   1 +
 lib/librte_eal/common/eal_private.h | 186 +++
 lib/librte_eal/common/include/rte_eal.h |   4 +
 lib/librte_eal/common/include/rte_pci_dev_ids.h |   8 +-
 lib/librte_eal/linuxapp/eal/Makefile|   5 +
 lib/librte_eal/linuxapp/eal/eal.c   | 295 
 lib/librte_eal/linuxapp/eal/eal_lcore.c |  66 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c|  36 +--
 lib/librte_eal/linuxapp/eal/eal_pci.c   | 100 +---
 lib/librte_eal/linuxapp/eal/eal_pci_init.h  |   6 -
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c   |  36 +--
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c  |  17 +-
 lib/librte_eal/linuxapp/eal/eal_thread.c| 152 
 lib/librte_eal/linuxapp/eal/eal_timer.c |  55 +
 28 files changed, 1355 insertions(+), 1332 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_lcore.c
 create mode 100644 lib/librte_eal/common/eal_common_runtime.c
 create mode 100644 lib/librte_eal/common/eal_common_system.c
 create mode 100644 lib/librte_eal/common/eal_common_timer.c

-- 
1.9.1

[dpdk-dev] [PATCH v5 0/8] Move common functions in EAL

[Ravi Kerur]

Changes include moving common functions in BSD and Linux into
common directory.
Use appropriate wrapper functions to mask minor differences.

Testing:
Linux - Ubuntu x86_64 14.04
Compilation successful (x86_64-native-linuxapp-gcc).
"make test" results match baseline code.
testpmd utility on I217/I218 Intel chipset.

FreeBSD 10.0 x86_64
Compilation successful (x86_64-native-bsdapp-gcc).
helloworld. 

Ravi Kerur (6):
  Move common functions in eal_thread.c
  Move common functions in eal.c
  Move common functions in eal_lcore.c
  Move common functions in eal_timer.c
  Move common functions in eal_memory.c
  Move common functions in eal_pci.c

 lib/librte_eal/bsdapp/eal/Makefile  |   6 +-
 lib/librte_eal/bsdapp/eal/eal.c | 271 +++---
 lib/librte_eal/bsdapp/eal/eal_lcore.c   |  72 ++
 lib/librte_eal/bsdapp/eal/eal_memory.c  |  47 ++--
 lib/librte_eal/bsdapp/eal/eal_pci.c | 122 ++
 lib/librte_eal/bsdapp/eal/eal_thread.c  | 152 
 lib/librte_eal/bsdapp/eal/eal_timer.c   |  52 +
 lib/librte_eal/common/eal_common_lcore.c| 107 +
 lib/librte_eal/common/eal_common_memory.c   |  38 ++-
 lib/librte_eal/common/eal_common_pci.c  | 130 ++-
 lib/librte_eal/common/eal_common_runtime.c  | 201 
 lib/librte_eal/common/eal_common_system.c   | 203 
 lib/librte_eal/common/eal_common_thread.c   | 158 +
 lib/librte_eal/common/eal_common_timer.c| 101 
 lib/librte_eal/common/eal_hugepages.h   |   1 +
 lib/librte_eal/common/eal_private.h | 186 +++
 lib/librte_eal/common/include/rte_eal.h |   4 +
 lib/librte_eal/common/include/rte_pci_dev_ids.h |   8 +-
 lib/librte_eal/linuxapp/eal/Makefile|   5 +
 lib/librte_eal/linuxapp/eal/eal.c   | 295 
 lib/librte_eal/linuxapp/eal/eal_lcore.c |  66 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c|  36 +--
 lib/librte_eal/linuxapp/eal/eal_pci.c   | 100 +---
 lib/librte_eal/linuxapp/eal/eal_pci_init.h  |   6 -
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c   |  36 +--
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c  |  17 +-
 lib/librte_eal/linuxapp/eal/eal_thread.c| 152 
 lib/librte_eal/linuxapp/eal/eal_timer.c |  55 +
 28 files changed, 1355 insertions(+), 1332 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_lcore.c
 create mode 100644 lib/librte_eal/common/eal_common_runtime.c
 create mode 100644 lib/librte_eal/common/eal_common_system.c
 create mode 100644 lib/librte_eal/common/eal_common_timer.c

-- 
1.9.1

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Avi Kivity]

On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
> On 08/04/2015 19:26, Stephen Hemminger wrote:
>> On Wed,  8 Apr 2015 16:07:21 +0100
>> Sergio Gonzalez Monroy  wrote:
>>
>>> Currently, the target/rules to build combined libraries is different
>>> than the one to build individual libraries.
>>>
>>> By removing the combined library option as a build configuration option
>>> we simplify the build pocess by having a single point for 
>>> linking/archiving
>>> libraries in DPDK.
>>>
>>> This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
>>> removes the makefiles associated with building a combined library.
>>>
>>> The CONFIG_RTE_LIBNAME config option is kept as it will be use to
>>> always generate a linker script that acts as a single combined library.
>>>
>>> Signed-off-by: Sergio Gonzalez Monroy 
>>> 
>> No. We use combined library and it greatly simplfies the application
>> linking process.
>>
> After all the opposition this patch had in v2, I did explain the 
> current issues
> (see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this 
> was the agreed solution.
>
> As I mention in the cover letter (also see patch 2/5), building DPDK 
> (after applying this patch series) will always generate a very simple 
> linker script that behaves as a combined library.
> I encourage you to apply this patch series and try to build your app 
> (which links against combined lib).
> Your app should build without problem unless I messed up somewhere and 
> it needs fixing.

Is it possible to generate a pkgconfig file (dpdk.pc) that contains all 
of the setting needed to compile and link with dpdk?  That will greatly 
simplify usage.

A linker script is just too esoteric.

[dpdk-dev] [RFC PATCH 0/6] DPDK support to bifurcated driver

[贾学涛]

Hi Cunming,
 I applyed bifurc dirver patches and tested it follow your example. But
I can't received packets with testpmd and l2fwd.
Kernel stack can receive packets from 10.0.0.2 before "ethtool -N
XGE4.1 flow-type ip4 src-ip 10.0.0.2 action 12". After "thtool -N XGE4.1
flow-type ip4 src-ip 10.0.0.2 action 12", kernel stack can't receive
packets from 10.0.0.2, but testpmd and l2fwd cannot receive any packets
too.
   queue 0-11 used by kernel and queue 12 used by bifurc dirver.
   How can I make it work?

2014-11-25 22:11 GMT+08:00 Cunming Liang :

>
> This is a RFC patch set to support "bifurcated driver" in DPDK.
>
>
> What is "bifurcated driver"?
> ===
>
> The "bifurcated driver" stands for the kernel NIC driver that supports:
>
> 1. on-demand rx/tx queue pairs split-off and assignment to user space
>
> 2. direct NIC resource(e.g. rx/tx queue registers) access from user space
>
> 3. distributing packets to kernel or user space rx queues by
>NIC's flow director according to the filter rules
>
> Here's the kernel patch set to support.
> http://comments.gmane.org/gmane.linux.network/333615
>
>
> Usage scenario
> =
>
> It's well accepted by industry to use DPDK to process fast path packets in
> user space in a high performance fashion, meanwhile processing slow path
> control packets in kernel space is still needed as those packets usually
> rely on in_kernel TCP/IP stacks and/or socket programming interface.
>
> KNI(Kernel NIC Interface) mechanism in DPDK is designed to meet this
> requirement, with below limitation:
>
>   1) Software classifies packets and distributes them to kernel via DPDK
>  software rings, at the cost of significant CPU cycles and memory
> bandwidth.
>
>   2) Memory copy packets between kernel' socket buffer and mbuf brings
>  significant negative performance impact to KNI performance.
>
> The bifurcated driver provides a alternative approach that not only
> offloads
> flow classification and distribution to NIC but also support packets
> zero_copy.
>
> User can use standard ethtool to add filter rules to the NIC in order to
> distribute specific flows to the queues only accessed by kernel driver and
> stack, and add other rules to distribute packets to the queues assigned to
> user-space.
>
> For those rx/tx queue pairs that directly accessed from user space,
> DPDK takes over the packets rx/tx as well as corresponding DMA operation
> for high performance packet I/O.
>
>
> What's the impact and change to DPDK
> ==
>
> DPDK usually binds PCIe NIC devices by leveraging kernel' user space driver
> mechanism UIO or VFIO to map entire NIC' PCIe I/O space of NIC to user
> space.
> The bifurcated driver PMD talks to a NIC interface using raw socket APIs
> and
> only mmap() limited I/O space (e.g. certain 4K pages) for accessing
> involved
> rx/tx queue pairs. So the impact and changes mainly comes with below:
>
> - netdev
> DPDK needs to create a af_packet socket and bind it to a bifurcated
> netdev.
> The socket fd will be used to request 'queue pairs info',
> 'split/return queue pairs' and etc. The PCIe device ID, netdev MAC
> address,
> numa info are also from the netdev response.
>
> - PCIe device scan and driver probe
> netdev provides the PCIe device ID information. Refer to the device ID,
> the correct driver should be used. And for such netdev device, the
> creation
> of PCIe device is no longer from scan but the on-demand assignment.
>
> - PCIe BAR mapping
> "bifurcated driver" maps several pages for the queue pairs.
> Others BAR register space maps to a fake page. The BAR mapping go
> through
> mmap on sockfd. Which is a little different from what UIO/VFIO does.
>
> - PMD
> The PMD will no longer really initialize and configure NIC.
> Instead, it only takes care the queue pair setup, rx_burst and
> tx_burst.
>
> The patch uses eal '--vdev' parameter to assign netdev iface name and
> number of
> queue pairs. Here's a example about how to configure the bifurcated driver
> and
> run DPDK testpmd with bifurcated PMD.
>
>   1. Set promisc mode
>   > ifconfig eth0 promisc
>
>   2. Turn on fdir
>   > ethtool -K eth0 ntuple on
>
>   3. Setup a flow director rule to distribute packets with source ip
>  0.0.0.0 to rxq No.0
>   > ethtool -N eth0  flow-type udp4 src-ip 0.0.0.0 action 0
>
>   4. Run testpmd on netdev 'eth0' with 1 queue pair.
>   > ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x3 -n 4 \
>   >  --vdev=rte_bifurc,iface=eth0,qpairs=1 -- \
>   >  -i --rxfreet=32 --txfreet=32 --txrst=32
>   Note:
> iface and qpairs arguments above specify the netdev interface name and
> number of qpairs that user space request from the "bifurcated driver"
> respectively.
>
>   5. Setup a flow director rule to distribute packets with source ip
>  1.1.1.1 to rxq No.32. This needs to be done after testpmd starts.
>   > ethtool -N eth0 

[dpdk-dev] Running DPDK Binaries on a different Target

[Ananyev, Konstantin]

Hi Venkat,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Venkat Thummala
> Sent: Thursday, April 09, 2015 10:07 AM
> To: Neil Horman
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] Running DPDK Binaries on a different Target
> 
> I have the following ACL rule configured in the ACL Table.
> 
> Priority - 20
> SIP - 0
> SIP MASK - 0
> DIP - 0
> DIP MASK - 0
> PROTO - 17 [UDP]
> PROTO MASK - 0xFF
> SPORT - 0
> SPORT RANGE - 0x
> DPORT - 0
> DPORT RANGE - 0x
> 
> And pumping UDP traffic.
> 
> On Machine 1, it always HITS the rule as expected.
> 
> But On Machine 2, the same traffic never HITS the rule.
> 
> I have tried this with both SCALAR and SSE classification algorithms, but
> the result is same.

So did I get it right:
You build a binary with RTE_MACHINE="default" on a box with AVX2 support.
Then run that binary on 
a) native box (machine with AVX2 support)
b) machine without AVX2 support

For the same input and same rules, same binary you got different results for a) 
and b).  
If that is the case, then there is something wrong here, either within our code 
or with the compiler you are using.
Could you provide steps to reproduce it?
I did a quick test with l3fwd-acl for that case, 'default' binary that I built 
on HSW board,
works as expected on IVB box for me. 

Build box: HSW (E3-1285), gcc 4.8.3 
SUT: IVB 2.80GHz, gcc 4.8.2

# cat acl_ipv4.rule1
R0.0.0.0/0 0.0.0.0/0 0 : 0x 0 : 0x 17/0xff 0
# cat acl_ipv6.rule1
R0:0:0:0:0:0:0:0/0 0:0:0:0:0:0:0:0/0 0 : 0x 0 : 0x 17/0xff 0

./dpdk.org-acl/examples/l3fwd-acl/x86_64-default-linuxapp-gcc/l3fwd-acl 
--lcores=3 -n 4 --socket-mem=1024,0 -w :04:00.1 -- -p 1 --config "(0, 0, 
3)" --rule_ipv4=./acl_ipv4.rule1 --rule_ipv6=acl_ipv6.rule1 -P

Forwards UDP packets, drops all others.

Anything else in your setup, that I am missing here?

Konstantin

> 
> Could some one, please help me on this?
> 
> DPDK Version:
> ===
> 2.0
> 
> Machine 1 CPU INFO:
> 
> vendor_id: GenuineIntel
> cpu family: 6
> model: 69
> model name: Intel(R) Core(TM) i5-4200U CPU @ 1.60GHz
> stepping: 1
> microcode: 0x16
> cpu MHz: 759.000
> cache size: 3072 KB
> physical id: 0
> siblings: 4
> core id: 1
> cpu cores: 2
> apicid: 3
> initial apicid: 3
> fpu: yes
> fpu_exception: yes
> cpuid level: 13
> wp: yes
> flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca
> cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
> pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
> xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
> ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe popcnt
> tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm ida arat epb
> xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid fsgsbase
> tsc_adjust bmi1 avx2 smep bmi2 erms invpcid
> bogomips: 4589.70
> clflush size: 64
> cache_alignment: 64
> address sizes: 39 bits physical, 48 bits virtual
> power management:
> 
> MACHINE 1 OS:
> =
> Linux vthummala-HP-Pavilion-15-Notebook-PC 3.13.0-48-generic #80-Ubuntu SMP
> Thu Mar 12 11:16:15 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
> 
> MACHINE 2 CPU INFO:
> ==
> vendor_id: GenuineIntel
> cpu family: 6
> model: 62
> model name: Intel(R) Xeon(R) CPU E5-2650 v2 @ 2.60GHz
> stepping: 4
> microcode: 0x416
> cpu MHz: 2599.890
> cache size: 20480 KB
> physical id: 1
> siblings: 16
> core id: 7
> cpu cores: 8
> apicid: 47
> initial apicid: 47
> fpu: yes
> fpu_exception: yes
> cpuid level: 13
> wp: yes
> flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca
> cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
> pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
> xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
> ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic
> popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm ida arat
> xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid fsgsbase smep
> erms
> bogomips: 5201.35
> clflush size: 64
> cache_alignment: 64
> address sizes: 46 bits physical, 48 bits virtual
> power management:
> 
> MACHINE 2 OS:
> =
> Linux vthummala-PowerEdge-R720 3.13.0-24-generic #46-Ubuntu SMP Thu Apr 10
> 19:11:08 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
> 
> 
> On 8 April 2015 at 17:17, Neil Horman  wrote:
> 
> > On Wed, Apr 08, 2015 at 02:03:05PM +0530, Venkat Thummala wrote:
> > > Hi Neil,
> > >
> > > Thanks for the quick response.
> > >
> > > The issue got fixed by setting CONFIG_RTE_MACHINE to "default".
> > >
> > Default is the lowest common demoninator system that dpdk supports (core2
> > 

[dpdk-dev] tools brainstorming

[Jay Rolette]

On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger <
stephen at networkplumber.org> wrote:

> On Wed, 8 Apr 2015 16:29:54 -0600
> Jay Rolette  wrote:
>
> > "C comments" includes //, right? It's been part of the C standard for a
> long time now...
>
> Yes but.
> I like to use checkpatch and checkpatch enforces kernel style which does
> not allow // for
> comments.
>

Fork checkpatch and disable that bit? DPDK isn't the kernel, so no
requirement to follow all of its rules

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Gonzalez Monroy, Sergio]

On 09/04/2015 10:06, Avi Kivity wrote:
>
>
> On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
>> On 08/04/2015 19:26, Stephen Hemminger wrote:
>>> On Wed,  8 Apr 2015 16:07:21 +0100
>>> Sergio Gonzalez Monroy  wrote:
>>>
 Currently, the target/rules to build combined libraries is different
 than the one to build individual libraries.

 By removing the combined library option as a build configuration 
 option
 we simplify the build pocess by having a single point for 
 linking/archiving
 libraries in DPDK.

 This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option 
 and
 removes the makefiles associated with building a combined library.

 The CONFIG_RTE_LIBNAME config option is kept as it will be use to
 always generate a linker script that acts as a single combined 
 library.

 Signed-off-by: Sergio Gonzalez Monroy 
 
>>> No. We use combined library and it greatly simplfies the application
>>> linking process.
>>>
>> After all the opposition this patch had in v2, I did explain the 
>> current issues
>> (see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and 
>> this was the agreed solution.
>>
>> As I mention in the cover letter (also see patch 2/5), building DPDK 
>> (after applying this patch series) will always generate a very simple 
>> linker script that behaves as a combined library.
>> I encourage you to apply this patch series and try to build your app 
>> (which links against combined lib).
>> Your app should build without problem unless I messed up somewhere 
>> and it needs fixing.
>
> Is it possible to generate a pkgconfig file (dpdk.pc) that contains 
> all of the setting needed to compile and link with dpdk? That will 
> greatly simplify usage.
>
> A linker script is just too esoteric.
>
>
That sounds very interesting.

I would be in favor of dropping the linker script for this solution if 
everybody is happy with it.

Sergio

[dpdk-dev] [PATCH] enic: disable debug traces

[Adrien Mazarguil]

On Tue, Apr 07, 2015 at 07:40:19PM +0200, Thomas Monjalon wrote:
> The function name is printed in each enic_ethdev function.
> Disable it by default with a new build option.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  config/common_bsdapp  | 1 +
>  config/common_linuxapp| 1 +
>  lib/librte_pmd_enic/enic_ethdev.c | 4 
>  3 files changed, 6 insertions(+)
> 
> diff --git a/config/common_bsdapp b/config/common_bsdapp
> index a8ba484..c2374c0 100644
> --- a/config/common_bsdapp
> +++ b/config/common_bsdapp
> @@ -214,6 +214,7 @@ CONFIG_RTE_LIBRTE_MLX4_SOFT_COUNTERS=1
>  # Compile burst-oriented Cisco ENIC PMD driver
>  #
>  CONFIG_RTE_LIBRTE_ENIC_PMD=y
> +CONFIG_RTE_LIBRTE_ENIC_DEBUG=n
>  
>  #
>  # Compile burst-oriented VIRTIO PMD driver
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index 0b25f34..0078dc9 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -211,6 +211,7 @@ CONFIG_RTE_LIBRTE_MLX4_SOFT_COUNTERS=1
>  # Compile burst-oriented Cisco ENIC PMD driver
>  #
>  CONFIG_RTE_LIBRTE_ENIC_PMD=y
> +CONFIG_RTE_LIBRTE_ENIC_DEBUG=n
>  
>  #
>  # Compile burst-oriented VIRTIO PMD driver
> diff --git a/lib/librte_pmd_enic/enic_ethdev.c 
> b/lib/librte_pmd_enic/enic_ethdev.c
> index 4950ede..18fadfb 100644
> --- a/lib/librte_pmd_enic/enic_ethdev.c
> +++ b/lib/librte_pmd_enic/enic_ethdev.c
> @@ -48,8 +48,12 @@
>  #include "vnic_enet.h"
>  #include "enic.h"
>  
> +#ifdef RTE_LIBRTE_ENIC_DEBUG
>  #define ENICPMD_FUNC_TRACE() \
>   RTE_LOG(DEBUG, PMD, "ENICPMD trace: %s\n", __func__)
> +#else
> +#define ENICPMD_FUNC_TRACE() do {} while (0)

How about defining it as (void)0 instead of an empty do/while block?

Doing so will prevent warnings if this macro happens to be used in an
expression. RTE_LOG() supports it.

> +#endif
>  
>  /*
>   * The set of PCI devices this driver supports
> -- 
> 2.2.2

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] [PATCH] Add toeplitz hash algorithm

[Gleb Natapov]

On Wed, Apr 08, 2015 at 03:06:13PM -0400, Vladimir Medvedkin wrote:
> Software implementation of the Toeplitz hash function used by RSS.
> Can be used either for packet distribution on single queue NIC
> or for simulating of RSS computation on specific NIC (for example
> after GRE header decapsulating).
> 
> Signed-off-by: Vladimir Medvedkin 
> ---
>  lib/librte_hash/Makefile|   1 +
>  lib/librte_hash/rte_thash.h | 179 
> 
>  2 files changed, 180 insertions(+)
>  create mode 100644 lib/librte_hash/rte_thash.h
> 
> diff --git a/lib/librte_hash/Makefile b/lib/librte_hash/Makefile
> index 3696cb1..083a9e5 100644
> --- a/lib/librte_hash/Makefile
> +++ b/lib/librte_hash/Makefile
> @@ -50,6 +50,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include := rte_hash.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_hash_crc.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_jhash.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_fbk_hash.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_thash.h
>  
>  # this lib needs eal
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_HASH) += lib/librte_eal lib/librte_malloc
> diff --git a/lib/librte_hash/rte_thash.h b/lib/librte_hash/rte_thash.h
> new file mode 100644
> index 000..1acfa3a
> --- /dev/null
> +++ b/lib/librte_hash/rte_thash.h
> @@ -0,0 +1,179 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in
> + *   the documentation and/or other materials provided with the
> + *   distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + *   contributors may be used to endorse or promote products derived
> + *   from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _RTE_THASH_H
> +#define _RTE_THASH_H
> +
> +/**
> + * @file
> + *
> + * toeplitz hash functions.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * Software implementation of the Toeplitz hash function used by RSS.
> + * Can be used either for packet distribution on single queue NIC
> + * or for simulating of RSS computation on specific NIC (for example
> + * after GRE header decapsulating)
> + */
> +
> +#include 
> +#include 
> +
> +enum rte_thash_flag {
> + RTE_THASH_L3 = 0,   //calculate hash tacking into account only l3 
> header
> + RTE_THASH_L4//calculate hash tacking into account l4 + l4 
> headers
> +};
> +
> +/**
> + * Prepare special converted key to use with rte_softrss_be()
> + * @param orig
> + *   pointer to original RSS key
> + * @param targ
> + *   pointer to target RSS key
> + */
> +
> +static inline void
> +rte_convert_rss_key(uint32_t *orig, uint32_t *targ)
> +{
> + int i;
> + for (i = 0; i < 10; i++) {
> + targ[i] = rte_be_to_cpu_32(orig[i]);
> + }
> +}
> +
> +/**
> + * Generic implementation. Can be used with original rss_key
> + * All ip's and ports have to be CPU byte order.
> + * @param sip
> + *   Source ip address.
> + * @param dip
> + *   Destination ip address.
ipv4, what about ipv6? Why not define rss function that works on byte
buffer and let caller build it according to whatever fields it want to
hash?

> + * @param sp
> + *   Source TCP|UDP port.
> + * @param dp
> + *   Destination TCP|UDP port.
> + * @param flag
> + *   RTE_THASH_L3:   calculate hash tacking into account only sip and dip
> + *   RTE_THASH_L4:   calculate hash tacking into account sip, dip, sp and dp
> + * @param *rss_key
> + *   Pointer to 40-byte RSS hash key.
i40e has 52 byte RSS hash key.

> + * @return
> + *   Calculated hash value.
> + */
> 

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Gonzalez Monroy, Sergio]

On 08/04/2015 19:26, Stephen Hemminger wrote:
> On Wed,  8 Apr 2015 16:07:21 +0100
> Sergio Gonzalez Monroy  wrote:
>
>> Currently, the target/rules to build combined libraries is different
>> than the one to build individual libraries.
>>
>> By removing the combined library option as a build configuration option
>> we simplify the build pocess by having a single point for linking/archiving
>> libraries in DPDK.
>>
>> This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
>> removes the makefiles associated with building a combined library.
>>
>> The CONFIG_RTE_LIBNAME config option is kept as it will be use to
>> always generate a linker script that acts as a single combined library.
>>
>> Signed-off-by: Sergio Gonzalez Monroy 
> No. We use combined library and it greatly simplfies the application
> linking process.
>
After all the opposition this patch had in v2, I did explain the current 
issues
(see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this 
was the agreed solution.

As I mention in the cover letter (also see patch 2/5), building DPDK 
(after applying this patch series) will always generate a very simple 
linker script that behaves as a combined library.
I encourage you to apply this patch series and try to build your app 
(which links against combined lib).
Your app should build without problem unless I messed up somewhere and 
it needs fixing.

Sergio

[dpdk-dev] [PATCH] enic: disable debug traces

[Stephen Hemminger]

On Thu, 9 Apr 2015 10:32:24 +0200
Adrien Mazarguil  wrote:

> >  
> > +#ifdef RTE_LIBRTE_ENIC_DEBUG
> >  #define ENICPMD_FUNC_TRACE() \
> > RTE_LOG(DEBUG, PMD, "ENICPMD trace: %s\n", __func__)
> > +#else
> > +#define ENICPMD_FUNC_TRACE() do {} while (0)  
> 
> How about defining it as (void)0 instead of an empty do/while block?
> 
> Doing so will prevent warnings if this macro happens to be used in an
> expression. RTE_LOG() supports it.

I kind of like the Linux printk trick since it then preserves the format 
checking
even if compiled out.

/*
 * Dummy printk for disabled debugging statements to use whilst maintaining
 * gcc's format and side-effect checking.
 */
static inline __printf(1, 2)
int no_printk(const char *fmt, ...)
{
return 0;
}

/* pr_devel() should produce zero code unless DEBUG is defined */
#ifdef DEBUG
#define pr_devel(fmt, ...) \
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#else
#define pr_devel(fmt, ...) \
no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#endif

[dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file.

[Neil Horman]

On Wed, Apr 08, 2015 at 03:58:38PM -0500, Keith Wiles wrote:
> Move a number of device specific define, structures and functions
> into a generic device base set of files for all device not just Ethernet.
> 
> Signed-off-by: Keith Wiles 
> ---
>  lib/librte_eal/common/eal_common_device.c | 185 +++
>  lib/librte_eal/common/include/rte_common_device.h | 617 
> ++
>  2 files changed, 802 insertions(+)
>  create mode 100644 lib/librte_eal/common/eal_common_device.c
>  create mode 100644 lib/librte_eal/common/include/rte_common_device.h
> 
> diff --git a/lib/librte_eal/common/eal_common_device.c 
> b/lib/librte_eal/common/eal_common_device.c
> new file mode 100644
> index 000..a9ef925
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_device.c
> @@ -0,0 +1,185 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2014 6WIND S.A.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in
> + *   the documentation and/or other materials provided with the
> + *   distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + *   contributors may be used to endorse or promote products derived
> + *   from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "rte_common_device.h"
> +
> +void *
> +rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
> + void * fn, void *user_param)
> +{
> + struct rte_dev_rxtx_callback *cb;
> +
> + cb = rte_zmalloc(NULL, sizeof(*cb), 0);
> +
> + if (cb == NULL) {
> + rte_errno = ENOMEM;
> + return NULL;
> + }
> +
> + cb->fn.vp = fn;
> + cb->param = user_param;
> + cb->next = *cbp;
> + *cbp = cb;
> + return cb;
> +}
> +
> +int
> +rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
> + struct rte_dev_rxtx_callback *user_cb)
> +{
> + struct rte_dev_rxtx_callback *cb = *cbp;
> + struct rte_dev_rxtx_callback *prev_cb;
> +
> + /* Reset head pointer and remove user cb if first in the list. */
> + if (cb == user_cb) {
> + *cbp = user_cb->next;
> + return 0;
> + }
> +
> + /* Remove the user cb from the callback list. */
> + do {
> + prev_cb = cb;
> + cb = cb->next;
> +
> + if (cb == user_cb) {
> + prev_cb->next = user_cb->next;
> + return 0;
> + }
> + } while (cb != NULL);
> +
> + /* Callback wasn't found. */
> + return (-EINVAL);
> +}
> +
> +int
> +rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
> + rte_spinlock_t * lock,
> + enum rte_dev_event_type event,
> + rte_dev_cb_fn cb_fn, void *cb_arg)
> +{
> + struct rte_dev_callback *cb;
> +
> + rte_spinlock_lock(lock);
> +
> + TAILQ_FOREACH(cb, cb_list, next) {
> + if (cb->cb_fn == cb_fn &&
> + cb->cb_arg == cb_arg &&
> + cb->event == event) {
> + break;
> + }
> + }
> +
> + /* create a new callback. */
> + if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
> + sizeof(struct rte_dev_callback), 0)) != NULL) {
> + cb->cb_fn = cb_fn;
> + cb->cb_arg = cb_arg;
> + cb->event = event;
> + TAILQ_INSERT_TAIL(cb_list, cb, next);
> + }
> +
> + rte_spinlock_unlock(lock);
> 

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Neil Horman]

On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
> 
> 
> On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
> >On 08/04/2015 19:26, Stephen Hemminger wrote:
> >>On Wed,  8 Apr 2015 16:07:21 +0100
> >>Sergio Gonzalez Monroy  wrote:
> >>
> >>>Currently, the target/rules to build combined libraries is different
> >>>than the one to build individual libraries.
> >>>
> >>>By removing the combined library option as a build configuration option
> >>>we simplify the build pocess by having a single point for
> >>>linking/archiving
> >>>libraries in DPDK.
> >>>
> >>>This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
> >>>removes the makefiles associated with building a combined library.
> >>>
> >>>The CONFIG_RTE_LIBNAME config option is kept as it will be use to
> >>>always generate a linker script that acts as a single combined library.
> >>>
> >>>Signed-off-by: Sergio Gonzalez Monroy
> >>>
> >>No. We use combined library and it greatly simplfies the application
> >>linking process.
> >>
> >After all the opposition this patch had in v2, I did explain the current
> >issues
> >(see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this was
> >the agreed solution.
> >
> >As I mention in the cover letter (also see patch 2/5), building DPDK
> >(after applying this patch series) will always generate a very simple
> >linker script that behaves as a combined library.
> >I encourage you to apply this patch series and try to build your app
> >(which links against combined lib).
> >Your app should build without problem unless I messed up somewhere and it
> >needs fixing.
> 
> Is it possible to generate a pkgconfig file (dpdk.pc) that contains all of
> the setting needed to compile and link with dpdk?  That will greatly
> simplify usage.
> 
> A linker script is just too esoteric.
> 
Why esoteric?  We're not talking about a linker script in the sense of a binary
layout file, we're talking about a prewritten/generated libdpdk_core.so that
contains linker directives to include the appropriate libraries.  You link it
just like you do any other library, but it lets you ignore how they are broken
up.

We could certainly do a pkg-config file, but I don't think thats any more
adventageous than this solution.

Neil

> 
> 

[dpdk-dev] [snabb-devel] Re: memory barriers in virtq.lua?

[Luke Gorrie]

Howdy,

On 8 April 2015 at 17:15, Xie, Huawei  wrote:

> luke:
> 1. host read the flag. 2 guest toggles the flag 3.guest checks the used.
> 4. host update used.
> Is this your case?
>

Yep, that is exactly the case I mean.

Cheers,
-Luke