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

lib: change M:A:C to X:X:X:X:X:X #3206

Merged
merged 1 commit into from
Oct 27, 2018
Merged

lib: change M:A:C to X:X:X:X:X:X #3206

merged 1 commit into from
Oct 27, 2018

Conversation

qlyoung
Copy link
Member

@qlyoung qlyoung commented Oct 19, 2018 •
edited
Loading

Summary

Change mac address token text from M:A:C to X:X:X:X:X:X for increased clarity

Components

[bgpd, lib]

rwestphal
rwestphal approved these changes Oct 19, 2018
View reviewed changes
Copy link
Member

@rwestphal rwestphal left a comment

Choose a reason for hiding this comment

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

LGTM. While here you could also update these two commands which are using a simple MAC:

$ find . -name "*.c" | xargs grep -En -A2 "DEFUN|DEFPY" | grep MAC
./lib/filter.c-1305-       "mac access-list WORD <deny|permit> MAC",
./lib/filter.c-1319-       "no mac access-list WORD <deny|permit> MAC",

@LabN-CI
Copy link
Collaborator

LabN-CI commented Oct 19, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/3206 6b066b7
Date 10/19/2018
Start 14:40:15
Finish 15:03:22
Run-Time 23:07
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-10-19-14:40:15.txt
Log autoscript-2018-10-19-14:40:52.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-5694/

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.

CLANG Static Analyzer Summary

  • Github Pull Request 3206, comparing to Git base SHA 440c39a

No Changes in Static Analysis warnings compared to base

@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-5695/

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.

CLANG Static Analyzer Summary

  • Github Pull Request 3206, comparing to Git base SHA 440c39a

No Changes in Static Analysis warnings compared to base

@qlyoung
Copy link
Member Author

qlyoung commented Oct 19, 2018

Sounds good @rwestphal

@LabN-CI
Copy link
Collaborator

LabN-CI commented Oct 19, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/3206 c6a942a
Date 10/19/2018
Start 16:50:18
Finish 17:13:39
Run-Time 23:21
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-10-19-16:50:18.txt
Log autoscript-2018-10-19-16:50:57.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-5696/

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.

CLANG Static Analyzer Summary

  • Github Pull Request 3206, comparing to Git base SHA 440c39a

No Changes in Static Analysis warnings compared to base

@eqvinox
Copy link
Contributor

eqvinox commented Oct 20, 2018

why? M:A:C is more concise... we're not using XXXX:XXXX:XXXX:XXXX:XXXX:XXXX:XXXX:XXXX for IPv6 either...

eqvinox
eqvinox requested changes Oct 23, 2018
View reviewed changes
Copy link
Contributor

@eqvinox eqvinox left a comment

Choose a reason for hiding this comment

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

Flagging as "request changes" because I'd really prefer to find a middle spot between short and obvious. The full XX:XX:XX:XX:XX:XX seems excessively long to me.

@eqvinox eqvinox added the submitter action required The author/submitter needs to do something (fix, rebase, add info, etc.) label Oct 23, 2018
@eqvinox
Copy link
Contributor

eqvinox commented Oct 23, 2018

Alternate suggestions: M:A:C:A:D:R or X:X:X:X:X:X? (I really just don't want to make our CLI strings unnecessarily long)

@rwestphal
Copy link
Member

One other option would be to just update the MAC address help string, which is defined like this:

#define MAC_STR "MAC address\n"

We could describe there what's the accepted format (e.g. xx:xx:xx:xx:xx:xx, aaaa.bbbb.cccc or even both in the future).

@qlyoung
lib: change M:A:C to X:X:X:X:X:X
6163c6c 
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
@qlyoung qlyoung changed the title lib: change M:A:C to XX:XX:XX:XX:XX:XX lib: change M:A:C to X:X:X:X:X:X Oct 26, 2018
@LabN-CI
Copy link
Collaborator

LabN-CI commented Oct 26, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/3206 6163c6c
Date 10/26/2018
Start 13:20:18
Finish 13:43:21
Run-Time 23:03
Total 1816
Pass 1816
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-10-26-13:20:18.txt
Log autoscript-2018-10-26-13:20:55.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-5774/

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.

CLANG Static Analyzer Summary

  • Github Pull Request 3206, comparing to Git base SHA 440c39a

No Changes in Static Analysis warnings compared to base

@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-5776/

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.

CLANG Static Analyzer Summary

  • Github Pull Request 3206, comparing to Git base SHA dd5c280

No Changes in Static Analysis warnings compared to base

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5775/

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.

Get source and apply patch from patchwork: Successful

Building Stage: Failed

CentOS 6 amd64 build: Successful
Debian 9 amd64 build: Successful
OmniOS amd64 build: Successful
Ubuntu 12.04 amd64 build: Successful
NetBSD 6 amd64 build: Successful
CentOS 7 amd64 build: Successful
Debian 8 amd64 build: Successful
FreeBSD 10 amd64 build: Successful
Ubuntu 14.04 amd64 build: Successful
OpenBSD 6 amd64 build: Successful
NetBSD 7 amd64 build: Successful
Fedora 24 amd64 build: Successful
Ubuntu 16.04 amd64 build: Successful
Ubuntu 18.04 amd64 build: Successful
FreeBSD 11 amd64 build: Successful

Ubuntu 16.04 i386 build: Failed

Package building failed for Ubuntu 16.04 i386 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5775/artifact/U1604I386/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Ubuntu 16.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5775/artifact/U1604I386/config.status/config.status

@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-5775/

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.

CLANG Static Analyzer Summary

  • Github Pull Request 3206, comparing to Git base SHA 440c39a

No Changes in Static Analysis warnings compared to base

@eqvinox eqvinox merged commit a755ec5 into FRRouting:master Oct 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwestphal rwestphal rwestphal approved these changes

@eqvinox eqvinox eqvinox approved these changes

Assignees
No one assigned
Labels
submitter action required The author/submitter needs to do something (fix, rebase, add info, etc.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@qlyoung @LabN-CI @NetDEF-CI @eqvinox @rwestphal