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

Fix subnet prefix calculation #48

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

Conversation

ekacnet
Copy link
Contributor

@ekacnet ekacnet commented Apr 29, 2022

Prefix calculation was wrong, and testing was complicated so I fixed it.
I was seeing errors like RTNETLINK answers: File exists before because my ISP is delegating a /60 and I'm asking for 3 interfaces to be delegated but in reality the prefixes where the same on the first 64 bits.

ekacnet added 9 commits June 26, 2022 21:14
find . -name "*.h" -o -name "*.cpp" -exec clang-format -i {} \;

Signed-off-by: Matthieu Patou <[email protected]>
prefix length and validity were swapped causing wrong prefix length to
be requested (often 255) during prefix delegation in SOLICIT messages.

Signed-off-by: Matthieu Patou <[email protected]>
=================================================================
==157330==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 576 byte(s) in 1 object(s) allocated from:
    #0 0x7f7ecd050628 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    tomaszmrugalski#1 0x561569188e82 in if_list_get /dibbler/Port-linux/lowlevel-linux.c:222

Direct leak of 38 byte(s) in 1 object(s) allocated from:
    #0 0x7f7ecd05236f in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10936f)
    tomaszmrugalski#1 0x56156914a104 in yyFlexLexer::yylex() /dibbler/ClntCfgMgr/ClntLexer.cpp:2318

Direct leak of 22 byte(s) in 3 object(s) allocated from:
    #0 0x7f7ecd05236f in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10936f)
    tomaszmrugalski#1 0x56156914a203 in yyFlexLexer::yylex() /dibbler/ClntCfgMgr/ClntLexer.cpp:2341

Indirect leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x7f7ecd050628 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    tomaszmrugalski#1 0x561569188af8 in ipaddr_local_get /dibbler/Port-linux/lowlevel-linux.c:289
Fix the file searched to be libgtest.a instead of libgtests.a, only
build libgtest.a in tests/utils if we have a source gtest.
Remove the hard coded use of `$(top_builddir)/tests/utils/libgtest.a`.
Move `-pthread` in the Clnt/Srv Makefiles

Signed-off-by: Matthieu Patou <[email protected]>
Previously prefixes like /60 would not work

Signed-off-by: Matthieu Patou <[email protected]>
1) use the monotonic clock everywhere (if avail) instead of mostly
   everywhere leaving a few places where it's still using the time(NULL)
   no matter what. This avoid issues when we end up comparing number of
   second since unix epoch (time(NULL)) and number of second since the
   system booted (without taking clock change)

2) Don't override ts by the value of clock for T2 timeout

Signed-off-by: Matthieu Patou <[email protected]>
@ekacnet ekacnet force-pushed the fix-subnet-prefix-calculation branch from 9fe72fe to 8c8246e Compare March 23, 2023 06:09
@tomaszmrugalski
Copy link
Owner

Hey, thanks a lot for the PR. Not sure if that's intentional, but you proposed to change close to 100k lines of code. Can you resubmit this PR without editing every single file in Dibbler tree? :)

Seriously, though, I gave up on Dibbler a long time ago. I may merge small stuff like #46 or #47, but I won't do a release.

@tomaszmrugalski
Copy link
Owner

The more I look at this, the more I like it. I need to figure out a way how to merge it, without sending all other branches to conflict hell. Stay tuned.

@tomaszmrugalski
Copy link
Owner

I've processed all the other PRs and rebased other branches that are possibly useful. The next here is to untangle the 05fe17c commit and pick the remaining changes that look good so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants