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.

Sign up for GitHub

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

Jump to bottom

isisd: test_fuzz_isis_tlv unit test is failing when compiled against musl #2375

Closed
ajones-rvbd opened this issue Jun 5, 2018 · 6 comments
Closed

isisd: test_fuzz_isis_tlv unit test is failing when compiled against musl #2375

ajones-rvbd opened this issue Jun 5, 2018 · 6 comments

Comments

@ajones-rvbd
Copy link
Contributor

It looks like inet_ntop has different output on musl vs glibc. In particular, it looks like musl is compressing :0: into :: in the IPv6 output. Discussions on slack w/ @cfra led to a possible solution which I'm looking into now. We're going to try to wrap the inet_ntop on musl only with a custom version that doesn't do the compression.
@ajones-rvbd
Copy link
Contributor Author

ajones-rvbd commented Jun 5, 2018 •
edited

For posterity, the diff:

diff -b -u t1.txt t2.txt
--- t1.txt	2018-06-05 10:12:56.000000000 -0700
+++ t2.txt	2018-06-05 10:13:14.000000000 -0700
@@ -6,9 +6,9 @@
       Unpack IPv6 Interface address...
         IPv6 Interface Address: 6c64:18d6:536c:4f10:6c00:2064:ffe0:e10d
       Unpack IPv6 Interface address...
-        IPv6 Interface Address: e864:0:106c:80:7364:2e00:1200:2
+        IPv6 Interface Address: e864::106c:80:7364:2e00:1200:2
       Unpack IPv6 Interface address...
-        IPv6 Interface Address: ab:1:fe05:40e1:e164:0:106c:80
+        IPv6 Interface Address: ab:1:fe05:40e1:e164::106c:80
       Unpack IPv6 Interface address...
         IPv6 Interface Address: 7364:4000:1200:2:ab:1:feff:40e0
       Unpack IPv6 Interface address...
@@ -23,7 +23,7 @@
       Unpack IPv6 Interface address...
         IPv6 Interface Address: 5340:12:3e8:ffab:ab6c:4dff:e0e1:3e8
       Unpack IPv6 Interface address...
-        IPv6 Interface Address: 6400:10:6c00:8073:6440:12:0:200
+        IPv6 Interface Address: 6400:10:6c00:8073:6440:12::200
       Unpack IPv6 Interface address...
         IPv6 Interface Address: ab00:1fe:540:e1e1:6400:10:6c00:8073
       Unpack IPv6 Interface address...
@@ -31,7 +31,7 @@
       Unpack IPv6 Interface address...
         IPv6 Interface Address: 1c72:80:7353:1a10:1203:e8ff:6400:10
       Unpack IPv6 Interface address...
-        IPv6 Interface Address: 6c00:8073:6440:12:0:200:ab00:1fe
+        IPv6 Interface Address: 6c00:8073:6440:12::200:ab00:1fe
       Unpack IPv6 Interface address...
         IPv6 Interface Address: 540:e1e1:6400:10:6c00:8073:6440:ff
       Unpack IPv6 Interface address...
@@ -41,7 +41,7 @@
       Unpack IPv6 Interface address...
         IPv6 Interface Address: 1a10:1203:e8ff:6400:10:6c00:8073:6440
       Unpack IPv6 Interface address...
-        IPv6 Interface Address: 12:0:200:ab00:1fe:540:e1e1:6400
+        IPv6 Interface Address: 12::200:ab00:1fe:540:e1e1:6400
       Unpack IPv6 Interface address...
         IPv6 Interface Address: 10:6c00:8073:6440:ff:ff00:200:ab00
       Unpack IPv6 Interface address...
@@ -50,3 +50,4 @@
       Not enough data left.(Expected 16 bytes of IPv6 address, got 7)

@ajones-rvbd
Copy link
Contributor Author

Completely randomly and just after I sent a "fix" patch to musl, I read this: http://www.openwall.com/lists/musl/2018/06/04/4

Incredible timing...

Until it is fixed in musl, I think it would be nice to fix it locally...

@ajones-rvbd
Copy link
Contributor Author

After some false starts, looks like our current approach is to let musl fix this issue and skip that test in frr when the inet_ntop is broken. Patch is being tested now...

ajones-rvbd added a commit to ajones-rvbd/frr that referenced this issue Jun 6, 2018
@ajones-rvbd
tests/isisd: bypass test_fuzz_isis_tlv when inet_ntop is broken
1808d46 
On Alpine Linux edge, musl does not seem to be RFC 5952 4.2.2
compliant (how to print a single :0: in the IPv6 address).  Let's
skip that test, as we get false negatives when running against
that version of musl.

Credit for the idea for the fix and how to fix it is due to
chris@opensourcerouting.org.

Testing done:

make check on alpine linux passes now

Issue: FRRouting#2375
Signed-off-by: Arthur Jones <arthur.jones@riverbed.com>
@ajones-rvbd ajones-rvbd mentioned this issue Jun 6, 2018
Merged
@ajones-rvbd
Copy link
Contributor Author

@mwinter-osr -- If this looks good for master, I'd like to get this in dev/5.0 if there is still time...

ajones-rvbd added a commit to ajones-rvbd/frr that referenced this issue Jun 6, 2018
@ajones-rvbd
tests/isisd: bypass test_fuzz_isis_tlv when inet_ntop is broken
0b445db 
On Alpine Linux edge, musl does not seem to be RFC 5952 4.2.2
compliant (how to print a single :0: in the IPv6 address).  Let's
skip that test, as we get false negatives when running against
that version of musl.

Credit for the idea for the fix and how to fix it is due to
chris@opensourcerouting.org.

Testing done:

make check on alpine linux passes now

Issue: FRRouting#2375
Signed-off-by: Arthur Jones <arthur.jones@riverbed.com>
ajones-rvbd added a commit to ajones-rvbd/frr that referenced this issue Jun 6, 2018
@ajones-rvbd
tests/isisd: bypass test_fuzz_isis_tlv when inet_ntop is broken
42d711e 
On Alpine Linux edge, musl does not seem to be RFC 5952 4.2.2
compliant (how to print a single :0: in the IPv6 address).  Let's
skip that test, as we get false negatives when running against
that version of musl.

Credit for the idea for the fix and how to fix it is due to
chris@opensourcerouting.org.

Testing done:

make check on alpine linux passes now

Issue: FRRouting#2375
Signed-off-by: Arthur Jones <arthur.jones@riverbed.com>
@ajones-rvbd ajones-rvbd mentioned this issue Jun 6, 2018
Closed
ajones-rvbd added a commit to ajones-rvbd/frr that referenced this issue Jun 7, 2018
@ajones-rvbd
tests/isisd: bypass test_fuzz_isis_tlv when inet_ntop is broken
f404be7 
On Alpine Linux edge, musl does not seem to be RFC 5952 4.2.2
compliant (how to print a single :0: in the IPv6 address).  Let's
skip that test, as we get false negatives when running against
that version of musl.

Credit for the idea for the fix and how to fix it is due to
chris@opensourcerouting.org.

Testing done:

make check on alpine linux passes now

Issue: FRRouting#2375
Signed-off-by: Arthur Jones <arthur.jones@riverbed.com>
@ajones-rvbd
Copy link
Contributor Author

Fix merged for master and didn't make it into 5.0, so we can close this...

@ajones-rvbd
Copy link
Contributor Author

As a followup, musl has applied:

commit 5c8e69267b9ae919e55eee4b79580224111bc3ba
Author: Arthur Jones arthur.jones@riverbed.com
Date: Tue Jun 5 16:51:27 2018 -0700

inet_ntop: do not compress single zeros in IPv6

So the workaround which has been applied here will not be needed for versions of alpine that use the version of musl with the fix (probably alpine 3.9 in 6mos)...

ajones-rvbd added a commit to ajones-rvbd/frr that referenced this issue Jul 2, 2018
@ajones-rvbd
tests/isisd: bypass test_fuzz_isis_tlv when inet_ntop is broken
67b9744 
On Alpine Linux edge, musl does not seem to be RFC 5952 4.2.2
compliant (how to print a single :0: in the IPv6 address).  Let's
skip that test, as we get false negatives when running against
that version of musl.

Credit for the idea for the fix and how to fix it is due to
chris@opensourcerouting.org.

NB 5.0:

This cherry-pick from master will simplify frr packaging for alpine

Testing done:

make check on alpine linux passes now

Issue: FRRouting#2375
Signed-off-by: Arthur Jones <arthur.jones@riverbed.com>
@ajones-rvbd ajones-rvbd mentioned this issue Jul 2, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ajones-rvbd