zebra: add mpls configuration knob per interface #9505
Conversation
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
pguibert6WIND
force-pushed
the
mpls_config
branch
from
9c0a654 to
607f018
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
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-21429/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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: SuccessfulBasic Tests: FailedTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9DEB10AMD64-21431/test Topology Tests failed for Topotests debian 10 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21431/artifact/TOPO9DEB10AMD64/ErrorLog/log_topotests.txt Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18AMD64-21431/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21431/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9DEB10AMD64-21431/test Topology Tests failed for Topotests debian 10 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21431/artifact/TOPO9DEB10AMD64/ErrorLog/log_topotests.txt Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18AMD64-21431/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21431/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt |
|
ci.rerun |
|
ci:rerun |
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-21446/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
|
Documentation changes? |
|
@polychaeta rereview |
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/dbd981998c8244b5406c145567bd5142/raw/f3e4d116c5e0f0c335377cf96d4d50bcdf882116/cr_9505_1630291653.diff | git apply
diff --git a/lib/mpls.c b/lib/mpls.c
index 14587460a..377be3ffc 100644
--- a/lib/mpls.c
+++ b/lib/mpls.c
@@ -105,8 +105,7 @@ int mpls_interface_set(const char *name, bool val)
FILE *fp;
int ret = 0, mplsinput = 0;
- snprintf(buf, BUFSIZ,
- "/proc/sys/net/mpls/conf/%s/input", name);
+ snprintf(buf, BUFSIZ, "/proc/sys/net/mpls/conf/%s/input", name);
fp = fopen(buf, "w");
if (fp == NULL)
return -1;
diff --git a/zebra/interface.c b/zebra/interface.c
index e2a1deac3..a0fc797c2 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -3770,7 +3770,7 @@ static void mpls_enable_if_set(struct interface *ifp)
struct zebra_if *if_data;
if_data = ifp->info;
- frr_with_privs(&zserv_privs) {
+ frr_with_privs (&zserv_privs) {
vrf_switch_to_netns(ifp->vrf_id);
if (if_data->mpls == IF_ZEBRA_MPLS_ON)
mpls_interface_set(ifp->name, true);
@@ -3796,11 +3796,8 @@ static int mpls_enable_interface(struct interface *ifp, uint8_t flag)
return CMD_SUCCESS;
}
-DEFUN (mpls_enable_if,
- mpls_enable_if_cmd,
- "[no] mpls",
- NO_STR
- "Enable MPLS traffic reception on this interface\n")
+DEFUN(mpls_enable_if, mpls_enable_if_cmd, "[no] mpls",
+ NO_STR "Enable MPLS traffic reception on this interface\n")
{
int idx = 0;
VTY_DECLVAR_CONTEXT(interface, ifp);
@@ -4240,11 +4237,9 @@ static int if_config_write(struct vty *vty)
== IF_ZEBRA_MULTICAST_ON
? ""
: "no ");
- if (if_data->mpls
- != IF_ZEBRA_MPLS_UNSPEC)
+ if (if_data->mpls != IF_ZEBRA_MPLS_UNSPEC)
vty_out(vty, " %smpls\n",
- if_data->mpls
- == IF_ZEBRA_MPLS_ON
+ if_data->mpls == IF_ZEBRA_MPLS_ON
? ""
: "no ");
}
diff --git a/zebra/interface.h b/zebra/interface.h
index 0643642c8..d344b69eb 100644
--- a/zebra/interface.h
+++ b/zebra/interface.h
@@ -46,8 +46,8 @@ extern "C" {
/* For interface MPLS input configuration */
#define IF_ZEBRA_MPLS_UNSPEC 0
-#define IF_ZEBRA_MPLS_ON 1
-#define IF_ZEBRA_MPLS_OFF 2
+#define IF_ZEBRA_MPLS_ON 1
+#define IF_ZEBRA_MPLS_OFF 2
#define IF_VLAN_BITMAP_MAX 4096
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.
There was a problem hiding this comment.
@donaldsharp is manually writing to procfs appropriate here? I see your name in the authors, asking you
lib/mpls.c
Outdated
| FILE *fp; | ||
| int ret = 0, mplsinput = 0; | ||
|
|
||
| snprintf(buf, BUFSIZ, |
There was a problem hiding this comment.
sizeof(buf), don't hardcode buffer sizes when you can use sizeof
There was a problem hiding this comment.
you re right. fixing it.
lib/mpls.c
Outdated
|
|
||
| int mpls_interface_set(const char *name, bool val) | ||
| { | ||
| char buf[BUFSIZ]; |
There was a problem hiding this comment.
why is this BUFSIZ and not MAXPATHLEN
There was a problem hiding this comment.
you re right. fixing it.
lib/mpls.c
Outdated
| if (val) | ||
| fprintf(fp, "1\n"); | ||
| else | ||
| fprintf(fp, "0\n"); |
There was a problem hiding this comment.
Not a blocking comment but fprintf(fp, "%d\n", !!val)? Why branch
There was a problem hiding this comment.
exact. I apply your recommendation.
lib/mpls.c
Outdated
| * 1 => mpls input is enabled. | ||
| * 0 => mpls input is disabled. | ||
| */ | ||
| if (!fgets(buf, 6, fp)) |
There was a problem hiding this comment.
It it inherited from ipforward_proc.c file. it reads also a proc/sys. in /proc/sys, a 6 bytes sentence was forged. I don' need that. reduced to 2 bytes length.
pguibert6WIND
force-pushed
the
mpls_config
branch
from
607f018 to
b6b1852
Compare
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/8ac6201ac1dcfddf7973aabaf6818fdf/raw/8c395c56d7dc2313af31429a749aa246448becab/cr_9505_1630319271.diff | git apply
diff --git a/lib/mpls.c b/lib/mpls.c
index e26ebf5e5..3af9f794a 100644
--- a/lib/mpls.c
+++ b/lib/mpls.c
@@ -105,8 +105,7 @@ int mpls_interface_set(const char *name, bool val)
FILE *fp;
int ret = 0, mplsinput = 0;
- snprintf(buf, sizeof(buf),
- "/proc/sys/net/mpls/conf/%s/input", name);
+ snprintf(buf, sizeof(buf), "/proc/sys/net/mpls/conf/%s/input", name);
fp = fopen(buf, "w");
if (fp == NULL)
return -1;
diff --git a/zebra/interface.c b/zebra/interface.c
index e2a1deac3..a0fc797c2 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -3770,7 +3770,7 @@ static void mpls_enable_if_set(struct interface *ifp)
struct zebra_if *if_data;
if_data = ifp->info;
- frr_with_privs(&zserv_privs) {
+ frr_with_privs (&zserv_privs) {
vrf_switch_to_netns(ifp->vrf_id);
if (if_data->mpls == IF_ZEBRA_MPLS_ON)
mpls_interface_set(ifp->name, true);
@@ -3796,11 +3796,8 @@ static int mpls_enable_interface(struct interface *ifp, uint8_t flag)
return CMD_SUCCESS;
}
-DEFUN (mpls_enable_if,
- mpls_enable_if_cmd,
- "[no] mpls",
- NO_STR
- "Enable MPLS traffic reception on this interface\n")
+DEFUN(mpls_enable_if, mpls_enable_if_cmd, "[no] mpls",
+ NO_STR "Enable MPLS traffic reception on this interface\n")
{
int idx = 0;
VTY_DECLVAR_CONTEXT(interface, ifp);
@@ -4240,11 +4237,9 @@ static int if_config_write(struct vty *vty)
== IF_ZEBRA_MULTICAST_ON
? ""
: "no ");
- if (if_data->mpls
- != IF_ZEBRA_MPLS_UNSPEC)
+ if (if_data->mpls != IF_ZEBRA_MPLS_UNSPEC)
vty_out(vty, " %smpls\n",
- if_data->mpls
- == IF_ZEBRA_MPLS_ON
+ if_data->mpls == IF_ZEBRA_MPLS_ON
? ""
: "no ");
}
diff --git a/zebra/interface.h b/zebra/interface.h
index 0643642c8..d344b69eb 100644
--- a/zebra/interface.h
+++ b/zebra/interface.h
@@ -46,8 +46,8 @@ extern "C" {
/* For interface MPLS input configuration */
#define IF_ZEBRA_MPLS_UNSPEC 0
-#define IF_ZEBRA_MPLS_ON 1
-#define IF_ZEBRA_MPLS_OFF 2
+#define IF_ZEBRA_MPLS_ON 1
+#define IF_ZEBRA_MPLS_OFF 2
#define IF_VLAN_BITMAP_MAX 4096
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.
pguibert6WIND
force-pushed
the
mpls_config
branch
from
b6b1852 to
c6d4563
Compare
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/fc38ff91a668ab73187d4c19a0cff74e/raw/58cf5c3a2d85be907e2a5303aadaf5e3c92cca8e/cr_9505_1630319474.diff | git apply
diff --git a/zebra/interface.c b/zebra/interface.c
index 9b450489d..a0fc797c2 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -3796,8 +3796,8 @@ static int mpls_enable_interface(struct interface *ifp, uint8_t flag)
return CMD_SUCCESS;
}
-DEFUN (mpls_enable_if, mpls_enable_if_cmd, "[no] mpls",
- NO_STR "Enable MPLS traffic reception on this interface\n")
+DEFUN(mpls_enable_if, mpls_enable_if_cmd, "[no] mpls",
+ NO_STR "Enable MPLS traffic reception on this interface\n")
{
int idx = 0;
VTY_DECLVAR_CONTEXT(interface, ifp);
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.
|
I recall a discussion with Donald, where I was questioning whether configure mpls sysctl on vty was ok , could be accepted on upstream. |
pguibert6WIND
force-pushed
the
mpls_config
branch
from
c6d4563 to
ee59c75
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
pguibert6WIND
force-pushed
the
mpls_config
branch
2 times, most recently
from
d9aa0a5 to
2bdccfa
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
|
@qlyoung, what Donald does is the netlink part of configuring MPLS flag. |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteAddresssanitizer topotests part 8: Incomplete(check logs for details)Successful on other platforms/tests
|
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-21460/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 4: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 4: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 arm8 part 4: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 4: No useful log found |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
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-21465/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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-21466/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
| @@ -98,3 +98,35 @@ char *mpls_label2str(uint8_t num_labels, const mpls_label_t *labels, char *buf, | |||
|
|
|||
| return buf; | |||
| } | |||
|
|
|||
| int mpls_interface_set(const char *name, bool val) | |||
There was a problem hiding this comment.
I personally would prefer that we use the RTNLGRP_MPLS_NETCONF netlink messages to gather / set this data instead of manually setting via changing the proc file system.
There was a problem hiding this comment.
if you have something ready to pull, pls pick up this commit.
otherwise, I would prefer to push this, and in a second step, we link to RTNLGRP ?
This configuration knob permits to set value of : proc/sys/net/mpls/conf/<iface>/input. Even if underlying interface is not available at the moment of the call. Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
pguibert6WIND
force-pushed
the
mpls_config
branch
from
2bdccfa to
cca9940
Compare
I updated doc/../zebra.rst file. |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
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-21583/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
| @@ -168,6 +168,12 @@ Standard Commands | |||
| configuration. | |||
|
|
|||
|
|
|||
| .. clicmd:: mpls | |||
|
|
|||
|
|
|||
|
@frrbot rereview |
|
This PR is stale because it has been open 180 days with no activity. Comment or remove the |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
already merged by an other pull request |
This configuration knob permits to set value of :
proc/sys/net/mpls/conf//input.
Even if underlying interface is not available at the moment of the call.
Signed-off-by: Philippe Guibert philippe.guibert@6wind.com