Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #525

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fixes #525

wants to merge 4 commits into from

Conversation

outscale-mgo
Copy link
Contributor

@outscale-mgo outscale-mgo commented Feb 18, 2020

  • change default optimisation of build system

  • fix a buffer overflow due to a strange behaviour of gcc

  • small clean of code in pg_brick_dot

  • Fix versioning

The function always check if 'n' is not NULL before calling pg_brick_dot_add.

So, I've move the check of 'n' in pg_brick_dot_add which enable code removal.

Signed-off-by: Matthias Gatto <[email protected]>
1: use &~ instead or ^ to unset mac, because if we xor on an alerady unser mac
   xor will set other variables

2: add a comment about how an UB can be ignored

3: use vilatile for 2 variables in mac-table-it-next.h:
  there is this loop:
  for (;i < PG_MAC_TABLE_MASK_SIZE ; m0 = ma->mask[i])
  it seems gcc find it smart to unroll it, and not checking
  properly the condition. (may be an UB somewhere, hope not)
  doing so, without volatile it seems i can be > to PG_MAC_TABLE_MASK_SIZE
  which lead to overflow.

4: move ++i out of the loop, because otherwise it can access an space
   which is undefined

Signed-off-by: Matthias Gatto <[email protected]>
@@ -270,7 +270,7 @@ var_add GLIB_HEADERS "$(pkg-config --cflags glib-2.0)"
var_add GLIB_LIBS "$(pkg-config --libs glib-2.0)"
var_add PG_MARCH "core-avx-i"
var_add PREFIX "/usr/local/"
var_add EXTRA_CFLAGS ""
var_add EXTRA_CFLAGS "-g -O2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag -g is for debugging purpose right? Maybe we should remove it... (I don't now about the -02...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe,
I've use that as default flag because that the default flag on autotool
also, I like debug symbols by default, size doesn't matter that much, debug-ability does

it's the default with autotool, and a better default that no optimisaion
nor debug symboles.

Signed-off-by: Matthias Gatto <[email protected]>
Until recently packetgraph version was hardcoded in the Makefile,
and we had no way to ghet the version in C.

It's now resolve by adding 4 define in "common.h"
those defines are:
       PG_VERSION_YEAR: year of last DPDK release on which packetgraph is bases
       PG_VERSION_MONTH: month of DPDK release
       PG_VERSION_REVISION: number of revisions on the release
       PG_VERSION: number containing all information above

As we need to got thoses informations for the linker in the makefile,
we are compiling an intermediate program in the configure that ghenerate a
version.mk which is then use in the makefile.

So we are now generating a libpacketgraph.so.19.2.0 instead of
libpacketgraph.so.17.5.0.

Signed-off-by: Matthias Gatto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants