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

zebra: fix some boot up warnings on FreeBSD #3419

Merged

Conversation

rzalamena
Copy link
Member

Summary

zebra on FreeBSD displays some boot up log messages that might annoy or cause some insecurity on users. Like:

ZEBRA: ifam_read() doesn't read all socket data

After some investigation I found that (a) we weren't handling all address types and (b) not rounding up the last sockaddr size. This PR implements the fixes for both problems.

I've opened this PR as Work-in-Progress to let people start testing it on plataforms other than FreeBSD (and using routing socket), while I try to fix the following log message:

errors: ZEBRA: [EC 100663303] if_ioctl(SIOCGIFMEDIA) failed: Invalid argument

Related Issue

#2338

Components

zebra.

Move the declaration of ROUNDUP and ROUND_TYPE to outside of
`ifdef SA_SIZE`. We'll use these definitions in the next commit.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Some address types were not being skipped triggering a warning log
message, so lets refactor this code to properly handle known and unknown
types.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
@rzalamena rzalamena changed the title Freebsd route warning fix zebra: fix some boot up warnings on FreeBSD Dec 5, 2018
@LabN-CI

This comment has been minimized.

@NetDEF-CI

This comment has been minimized.

When using `SIOCGIFMEDIA` check for `EINVAL`, otherwise we might print
an error message on an unsupported interface.

FreeBSD source code reference:
https://github.com/freebsd/freebsd/blob/master/sys/net/if_media.c#L300

And:
https://github.com/freebsd/freebsd/blob/8cb4b0c0181bd45318ee8977f77aea90c53bb224/usr.sbin/rtsold/if.c#L211

  /*
   * EINVAL simply means that the interface does not support
   * the SIOCGIFMEDIA ioctl. We regard it alive.
   */

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
@LabN-CI
Copy link
Collaborator

LabN-CI commented Dec 5, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/3419 c69f2c1
Date 12/05/2018
Start 14:40:23
Finish 15:03:54
Run-Time 23:31
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-12-05-14:40:23.txt
Log autoscript-2018-12-05-14:41:05.log.bz2

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-6081/

This is a comment from an EXPERIMENTAL 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 kernel_socket.c | 4 issues
===============================================
< WARNING: externs should be avoided in .c files
< #233: FILE: /tmp/f1-12285/kernel_socket.c:233:
< WARNING: externs should be avoided in .c files
< #234: FILE: /tmp/f1-12285/kernel_socket.c:234:

CLANG Static Analyzer Summary

  • Github Pull Request 3419, comparing to Git base SHA 60e2b4f

No Changes in Static Analysis warnings compared to base

4 Static Analyzer issues remaining.

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

@donaldsharp donaldsharp merged commit 8ba26ec into FRRouting:master Dec 6, 2018
@rzalamena rzalamena deleted the freebsd-route-warning-fix branch December 6, 2018 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants