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

inet_ntop but faster #4480

Merged
merged 3 commits into from
Jun 7, 2019
Merged

inet_ntop but faster #4480

merged 3 commits into from
Jun 7, 2019

Conversation

eqvinox
Copy link
Contributor

@eqvinox eqvinox commented Jun 6, 2019

AF_INET
glibc:  0.187803364 (192.168.9.1)
musl:   0.121625838 (192.168.9.1)
custom: 0.032223014 (192.168.9.1)
funky:  0.022845125 (192.168.9.1)

AF_INET6
glibc:  0.215974851 (2001:db8:1234::)
musl:   0.325782698 (2001:db8:1234::)
custom: 0.315118340 (2001:db8:1234::)
funky:  0.022269407 (2001:db8:1234::)

(this is "funky", PR #4469 is "custom")

@eqvinox eqvinox added tests Topotests, make check, etc libfrr labels Jun 6, 2019
@eqvinox eqvinox requested a review from qlyoung June 6, 2019 17:40
@NetDEF-CI

This comment has been minimized.

@LabN-CI

This comment has been minimized.

eqvinox added 3 commits June 6, 2019 20:58
Signed-off-by: David Lamparter <equinox@diac24.net>
This commit is brought to you by: "Manuel - Gas Gas Gas" (Initial D)

Signed-off-by: David Lamparter <equinox@diac24.net>
Signed-off-by: David Lamparter <equinox@diac24.net>
@LabN-CI
Copy link
Collaborator

LabN-CI commented Jun 6, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/4480 874035b
Date 06/06/2019
Start 15:43:05
Finish 16:05:16
Run-Time 22:11
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2019-06-06-15:43:05.txt
Log autoscript-2019-06-06-15:44:03.log.bz2
Memory 411 406 360

For details, please contact louberger

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7967/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings
Report for ntop.c | 39 issues
===============================================
ERROR: do not use assignment in if condition
#43: FILE: /tmp/f1-22132/ntop.c:43:
+	if ((tmp = byte - 200) >= 0) {

ERROR: do not use assignment in if condition
#47: FILE: /tmp/f1-22132/ntop.c:47:
+	} else if ((tmp = byte - 100) >= 0) {

WARNING: Missing a blank line after declarations
#71: FILE: /tmp/f1-22132/ntop.c:71:
+	const char *digits = "0123456789abcdef";
+	if (word >= 0x1000)

ERROR: "foo * bar" should be "foo *bar"
#82: FILE: /tmp/f1-22132/ntop.c:82:
+const char *frr_inet_ntop(int af, const void * restrict src,

ERROR: "foo * bar" should be "foo *bar"
#83: FILE: /tmp/f1-22132/ntop.c:83:
+			  char * restrict dst, socklen_t size)

ERROR: "foo * bar" should be "foo *bar"
#86: FILE: /tmp/f1-22132/ntop.c:86:
+const char *frr_inet_ntop(int af, const void * restrict src,

ERROR: "foo * bar" should be "foo *bar"
#87: FILE: /tmp/f1-22132/ntop.c:87:
+			  char * restrict dst, socklen_t size)

WARNING: braces {} are not necessary for single statement blocks
#143: FILE: /tmp/f1-22132/ntop.c:143:
+			if (i > best && i < best + bestlen) {
+				continue;
+			}

WARNING: space prohibited between function name and open parenthesis '('
#173: FILE: /tmp/f1-22132/ntop.c:173:
+	__attribute__((alias ("frr_inet_ntop"))) DSO_SELF;
Report for test_ntop.c | 5 issues
===============================================
WARNING: Missing a blank line after declarations
#64: FILE: /tmp/f1-22132/test_ntop.c:64:
+		uint16_t *i6w = (uint16_t *)&i6;
+		for (j = 0; j < 8; j++)

CLANG Static Analyzer Summary

  • Github Pull Request 4480, comparing to Git base SHA 0b5cfde
  • Base image data for Git 0b5cfde does not exist - compare skipped

13 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7967/artifact/shared/static_analysis/index.html

Copy link
Member

@donaldsharp donaldsharp left a comment

Choose a reason for hiding this comment

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

LGTM

@qlyoung qlyoung merged commit 9f9307c into FRRouting:master Jun 7, 2019
@rwestphal
Copy link
Member

For the record, with this PR I could notice a significant performance improvement when fetching millions of routes using the gRPC northbound plugin. Amazing work!

@qlyoung
Copy link
Member

qlyoung commented Jun 7, 2019

That was the idea ;)

This was referenced Jul 16, 2019
@eqvinox eqvinox deleted the inet_ntop branch April 18, 2021 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libfrr performance tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants