-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[RFC] zebra: add option to specify interfaces to work with #7659
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/d3c2167b19e2b727bb4bfcc7afcbcb4b/raw/a8709bd228476c5c34f3aa7703dd5ee474f02c58/cr_7659_1606930975.diff | git apply
diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index c5221a429..6178c567b 100644
--- a/zebra/if_netlink.c
+++ b/zebra/if_netlink.c
@@ -784,8 +784,7 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup)
if (!zebra_if_allowed(name)) {
if (IS_ZEBRA_DEBUG_KERNEL)
- zlog_debug("%s: ignoring interface %s",
- __func__, name);
+ zlog_debug("%s: ignoring interface %s", __func__, name);
return 0;
}
@@ -1452,8 +1451,9 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
if (!zebra_if_allowed(name)) {
if (IS_ZEBRA_DEBUG_KERNEL)
- zlog_debug("%s: ignoring interface %s",
- __func__, name);
+ zlog_debug(
+ "%s: ignoring interface %s",
+ __func__, name);
return 0;
}
diff --git a/zebra/interface.c b/zebra/interface.c
index dda48c7ba..ff8aa1ae4 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -299,31 +299,30 @@ void if_unlink_per_ns(struct interface *ifp)
bool zebra_if_allowed(const char *name)
{
- char *tokens, *token;
- bool allowed;
+ char *tokens, *token;
+ bool allowed;
- if (!interface_pattern)
- return true;
+ if (!interface_pattern)
+ return true;
- tokens = strdup(interface_pattern);
- if (!tokens)
- return true; /* it is safer to be permissive */
+ tokens = strdup(interface_pattern);
+ if (!tokens)
+ return true; /* it is safer to be permissive */
- for (token = strtok(tokens, ",");
- token != NULL;
- token = strtok(NULL, ",")) {
- allowed = true;
+ for (token = strtok(tokens, ","); token != NULL;
+ token = strtok(NULL, ",")) {
+ allowed = true;
- if (token[0] == '-') {
- allowed = false;
- token++;
- }
+ if (token[0] == '-') {
+ allowed = false;
+ token++;
+ }
- if (fnmatch(token, name, 0) == 0)
- return allowed;
- }
+ if (fnmatch(token, name, 0) == 0)
+ return allowed;
+ }
- return false;
+ return false;
}
/* Look up an interface by identifier within a NS */
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
can you describe your use case? |
I have a system with a large number of interfaces and I want to run FRR only on a few of them. |
put those interfaces into their own namespace and run FRR in that namespace? |
Unfortunately, it's not an option in my case. We can discuss in DM if you're interested. From a general point of view, your solution will work, but it seems easier to just pass one additional option to zebra. |
fe0f64c
to
670c54f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/558c8490006567dc9af59aee83071b4c/raw/a8709bd228476c5c34f3aa7703dd5ee474f02c58/cr_7659_1606932908.diff | git apply
diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index c5221a429..6178c567b 100644
--- a/zebra/if_netlink.c
+++ b/zebra/if_netlink.c
@@ -784,8 +784,7 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup)
if (!zebra_if_allowed(name)) {
if (IS_ZEBRA_DEBUG_KERNEL)
- zlog_debug("%s: ignoring interface %s",
- __func__, name);
+ zlog_debug("%s: ignoring interface %s", __func__, name);
return 0;
}
@@ -1452,8 +1451,9 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
if (!zebra_if_allowed(name)) {
if (IS_ZEBRA_DEBUG_KERNEL)
- zlog_debug("%s: ignoring interface %s",
- __func__, name);
+ zlog_debug(
+ "%s: ignoring interface %s",
+ __func__, name);
return 0;
}
diff --git a/zebra/interface.c b/zebra/interface.c
index dda48c7ba..ff8aa1ae4 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -299,31 +299,30 @@ void if_unlink_per_ns(struct interface *ifp)
bool zebra_if_allowed(const char *name)
{
- char *tokens, *token;
- bool allowed;
+ char *tokens, *token;
+ bool allowed;
- if (!interface_pattern)
- return true;
+ if (!interface_pattern)
+ return true;
- tokens = strdup(interface_pattern);
- if (!tokens)
- return true; /* it is safer to be permissive */
+ tokens = strdup(interface_pattern);
+ if (!tokens)
+ return true; /* it is safer to be permissive */
- for (token = strtok(tokens, ",");
- token != NULL;
- token = strtok(NULL, ",")) {
- allowed = true;
+ for (token = strtok(tokens, ","); token != NULL;
+ token = strtok(NULL, ",")) {
+ allowed = true;
- if (token[0] == '-') {
- allowed = false;
- token++;
- }
+ if (token[0] == '-') {
+ allowed = false;
+ token++;
+ }
- if (fnmatch(token, name, 0) == 0)
- return allowed;
- }
+ if (fnmatch(token, name, 0) == 0)
+ return allowed;
+ }
- return false;
+ return false;
}
/* Look up an interface by identifier within a NS */
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
670c54f
to
3e6c8b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/5f36a993a071619653009e1599e93a27/raw/817c9da67a312655f7c95ce3e7bc8a823ad40b7a/cr_7659_1606933154.diff | git apply
diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index 8e3e451ce..253be57ba 100644
--- a/zebra/if_netlink.c
+++ b/zebra/if_netlink.c
@@ -1452,8 +1452,8 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
if (!zebra_if_allowed(name)) {
if (IS_ZEBRA_DEBUG_KERNEL)
zlog_debug(
-+ "%s: ignoring interface %s",
-+ __func__, name);
+ +"%s: ignoring interface %s",
+ +__func__, name);
return 0;
}
diff --git a/zebra/interface.c b/zebra/interface.c
index 2b3527336..ff8aa1ae4 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -309,8 +309,7 @@ bool zebra_if_allowed(const char *name)
if (!tokens)
return true; /* it is safer to be permissive */
- for (token = strtok(tokens, ",");
- token != NULL;
+ for (token = strtok(tokens, ","); token != NULL;
token = strtok(NULL, ",")) {
allowed = true;
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
3e6c8b1
to
47fa782
Compare
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedFreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build:
FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/FBSD12AMD64/config.status/config.status FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build:
FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI009BUILD/config.status/config.status OpenBSD 6 amd64 build: Failed (click for details)Make failed for OpenBSD 6 amd64 build:
OpenBSD 6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI011BUILD/config.status/config.status NetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build:
NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI012BUILD/config.status/config.status Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsFreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build:
FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/FBSD12AMD64/config.status/config.status FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build:
FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI009BUILD/config.status/config.status OpenBSD 6 amd64 build: Failed (click for details)Make failed for OpenBSD 6 amd64 build:
OpenBSD 6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI011BUILD/config.status/config.status NetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build:
NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15797/artifact/CI012BUILD/config.status/config.status
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedUbuntu 16.04 arm8 build: Failed (click for details)Make failed for Ubuntu 16.04 arm8 build:
Ubuntu 16.04 arm8 build: Unknown Log <config.log.gz> Fedora 29 amd64 build: Failed (click for details)Make failed for Fedora 29 amd64 build:
Fedora 29 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/F29BUILD/config.status/config.status CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CENTOS8BUILD/config.status/config.statusMake failed for CentOS 8 amd64 build:
CentOS 8 amd64 build: Unknown Log <config.log.gz> Ubuntu 16.04 arm7 build: Failed (click for details)Ubuntu 16.04 arm7 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI101BUILD/config.status/config.status Ubuntu 16.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI101BUILD/config.log/config.log.gzMake failed for Ubuntu 16.04 arm7 build:
Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/DEB10BUILD/config.log/config.log.gzMake failed for Debian 10 amd64 build:
Debian 10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/DEB10BUILD/config.status/config.status Ubuntu 16.04 i386 build: Failed (click for details)Make failed for Ubuntu 16.04 i386 build:
Ubuntu 16.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U1604I386/config.status/config.status Ubuntu 16.04 amd64 build: Failed (click for details)Make failed for Ubuntu 16.04 amd64 build:
Ubuntu 16.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI014BUILD/config.status/config.status Ubuntu 18.04 amd64 build: Failed (click for details)Make failed for Ubuntu 18.04 amd64 build:
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U1804AMD64/config.status/config.status Ubuntu 18.04 arm8 build: Failed (click for details)Ubuntu 18.04 arm8 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U18ARM8BUILD/config.status/config.status Ubuntu 18.04 arm8 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U18ARM8BUILD/config.log/config.log.gzMake failed for Ubuntu 18.04 arm8 build:
Ubuntu 18.04 arm7 build: Failed (click for details)Make failed for Ubuntu 18.04 arm7 build:
Ubuntu 18.04 arm7 build: Unknown Log <config.log.gz> CentOS 7 amd64 build: Failed (click for details)Make failed for CentOS 7 amd64 build:
CentOS 7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI005BUILD/config.status/config.status Debian 8 amd64 build: Failed (click for details)Debian 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI008BLD/config.log/config.log.gzMake failed for Debian 8 amd64 build:
Debian 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI008BLD/config.status/config.status Debian 9 amd64 build: Failed (click for details)Make failed for Debian 9 amd64 build:
Debian 9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI021BUILD/config.status/config.status Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U2004AMD64BUILD/config.status/config.statusMake failed for Ubuntu 20.04 amd64 build:
Ubuntu 20.04 amd64 build: Unknown Log <config.log.gz> Ubuntu 18.04 ppc64le build: Failed (click for details)Make failed for Ubuntu 18.04 ppc64le build:
Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U1804PPC64LEBUILD/config.status/config.status Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsUbuntu 16.04 arm8 build: Failed (click for details)Make failed for Ubuntu 16.04 arm8 build:
Ubuntu 16.04 arm8 build: Unknown Log <config.log.gz> Fedora 29 amd64 build: Failed (click for details)Make failed for Fedora 29 amd64 build:
Fedora 29 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/F29BUILD/config.status/config.status CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CENTOS8BUILD/config.status/config.statusMake failed for CentOS 8 amd64 build:
CentOS 8 amd64 build: Unknown Log <config.log.gz> Ubuntu 16.04 arm7 build: Failed (click for details)Ubuntu 16.04 arm7 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI101BUILD/config.status/config.status Ubuntu 16.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI101BUILD/config.log/config.log.gzMake failed for Ubuntu 16.04 arm7 build:
Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/DEB10BUILD/config.log/config.log.gzMake failed for Debian 10 amd64 build:
Debian 10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/DEB10BUILD/config.status/config.status Ubuntu 16.04 i386 build: Failed (click for details)Make failed for Ubuntu 16.04 i386 build:
Ubuntu 16.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U1604I386/config.status/config.status Ubuntu 16.04 amd64 build: Failed (click for details)Make failed for Ubuntu 16.04 amd64 build:
Ubuntu 16.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI014BUILD/config.status/config.status Ubuntu 18.04 amd64 build: Failed (click for details)Make failed for Ubuntu 18.04 amd64 build:
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U1804AMD64/config.status/config.status Ubuntu 18.04 arm8 build: Failed (click for details)Ubuntu 18.04 arm8 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U18ARM8BUILD/config.status/config.status Ubuntu 18.04 arm8 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U18ARM8BUILD/config.log/config.log.gzMake failed for Ubuntu 18.04 arm8 build:
Ubuntu 18.04 arm7 build: Failed (click for details)Make failed for Ubuntu 18.04 arm7 build:
Ubuntu 18.04 arm7 build: Unknown Log <config.log.gz> CentOS 7 amd64 build: Failed (click for details)Make failed for CentOS 7 amd64 build:
CentOS 7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI005BUILD/config.status/config.status Debian 8 amd64 build: Failed (click for details)Debian 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI008BLD/config.log/config.log.gzMake failed for Debian 8 amd64 build:
Debian 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI008BLD/config.status/config.status Debian 9 amd64 build: Failed (click for details)Make failed for Debian 9 amd64 build:
Debian 9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/CI021BUILD/config.status/config.status Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U2004AMD64BUILD/config.status/config.statusMake failed for Ubuntu 20.04 amd64 build:
Ubuntu 20.04 amd64 build: Unknown Log <config.log.gz> Ubuntu 18.04 ppc64le build: Failed (click for details)Make failed for Ubuntu 18.04 ppc64le build:
Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15800/artifact/U1804PPC64LEBUILD/config.status/config.status
|
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15799/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
CLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15801/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
CLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
I've spoken offline w/ @idryzhov about this and I believe that this should be discussed at the next technical meeting for group input. Ideas/thoughts I have had that could help ( or approach the problem differently ) and I wanted to write them down before I forget them: (a) This is linux only. Should we care about this being generic for all platforms? ( brought up by @sworleys ) |
We could use |
@idryzhov any more opinions on this one? I think we discussed in the meeting seeing if we could move the code up so it works on all platforms. Are you okay with that or were you against it? I like this functionality so I wanna see it get merged in. |
@sworleys yes, we agreed to make it OS independent by moving the check to a higher level. I just didn't have time yet. I'll update this or next week. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Closing this PR. Can be re-opened when submitter has time to work on it again |
This PR introduces a new option
-I
/--interfaces
for zebra, which allowsuser to tell zebra to work only with specific interfaces.
The option itself is a list of patterns to match interface names. It allows using
an asterisk (
*
) to match any symbols and a hyphen (-
) to block interfacesthat match the pattern. Patterns are checked in a specified order. There is an
implicit "block any" (
-*
) rule at the end.Examples:
To allow only names starting with "eth":
-I eth*
To allow all names except starting with "ens":
-I -ens*,*
This currently works only with netlink-based kernel communication as I don't
have any OS other than Linux.
If the idea is accepted by the community, I will add the docs.