bmp: add a interface source to bmp connect command #11353

fdumontet6WIND · 2022-06-07T10:27:11Z

With current release, forcin the source ip address when setting up a BMP
connection is not possible.

The need is to add an extra parameter for the following vty command:

router bgp 65500
bmp targets AAA
bmp connect 2.2.2.2 port 666 min-retry 100 max-retry 700
bmp connect 2:2::2:2 port 666 min-retry 100 max-retry 700 [source-interface lo1]

Signed-off-by: Francois Dumontet francois.dumontet@6wind.com

NetDEF-CI · 2022-06-07T12:34:48Z

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-PULLREQ2-5778/

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 bgp_bmp.c | 30 issues
===============================================
< WARNING: line over 80 characters
< #1791: FILE: /tmp/f1-24923/bgp_bmp.c:1791:
< ERROR: space required after that ',' (ctx:VxO)
< #1798: FILE: /tmp/f1-24923/bgp_bmp.c:1798:
< ERROR: space required before that '&' (ctx:OxV)
< #1798: FILE: /tmp/f1-24923/bgp_bmp.c:1798:
< ERROR: space required after that ',' (ctx:VxV)
< #1801: FILE: /tmp/f1-24923/bgp_bmp.c:1801:
< WARNING: quoted string split across lines
< #1801: FILE: /tmp/f1-24923/bgp_bmp.c:1801:
< WARNING: break quoted strings at a space character
< #1801: FILE: /tmp/f1-24923/bgp_bmp.c:1801:
< WARNING: space prohibited between function name and open parenthesis '('
< #2130: FILE: /tmp/f1-24923/bgp_bmp.c:2130:
< ERROR: space required after that ',' (ctx:VxV)
< #2130: FILE: /tmp/f1-24923/bgp_bmp.c:2130:
< ERROR: space required after that ',' (ctx:VxV)
< #2131: FILE: /tmp/f1-24923/bgp_bmp.c:2131:
< WARNING: quoted string split across lines
< #2132: FILE: /tmp/f1-24923/bgp_bmp.c:2132:
< WARNING: break quoted strings at a space character
< #2132: FILE: /tmp/f1-24923/bgp_bmp.c:2132:
< ERROR: space required after that ',' (ctx:VxV)
< #2423: FILE: /tmp/f1-24923/bgp_bmp.c:2423:
< ERROR: space required after that ',' (ctx:VxV)
< #2520: FILE: /tmp/f1-24923/bgp_bmp.c:2520:
< ERROR: space required after that ',' (ctx:VxV)
< #2520: FILE: /tmp/f1-24923/bgp_bmp.c:2520:
< ERROR: space required after that ',' (ctx:VxV)
< #2522: FILE: /tmp/f1-24923/bgp_bmp.c:2522:

ton31337

Please fix all the new warnings from CI.

ton31337 · 2022-06-08T07:00:19Z

bgpd/bgp_bmp.c

 			continue;
 		case connect_success:
+			sockunion2str(&ba->addrs[ba->addrpos], buf,
+				      sizeof(buf));
+			zlog_warn("bmp[%s]: connected to  %s:%d",


Is this warning or info?

ton31337 · 2022-06-08T07:00:45Z

bgpd/bgp_bmp.c

+						  &ba->addrsrc);
+			if (res_bind < 0) {
+				sockunion2str(&ba->addrsrc, buf, sizeof(buf));
+				zlog_warn(


The same question still, is this supposed to be an error or warning? Failed seems to be related to error?

ton31337 · 2022-06-08T07:01:41Z

bgpd/bgp_bmp.c

+			}
+			if (bgp_update_address(ifp,&ba->addrs[ba->addrpos],
+					       &ba->addrsrc)){
+				zlog_warn("bmp[%s]: failed to find matching"


ton31337 · 2022-06-08T07:02:03Z

bgpd/bgp_bmp.c

+					  "address",ba->ifsrc);
+				continue;
+			}
+			zlog_warn("bmp[%s]: selected source address : %s",


ton31337 · 2022-06-08T07:23:18Z

bgpd/bgp_bmp.c

@@ -1773,8 +1776,38 @@ static void bmp_active_connect(struct bmp_active *ba)
 {
 	enum connect_result res;
 	char buf[SU_ADDRSTRLEN];
+	struct interface *ifp;
+	vrf_id_t vrf_id;


I suggest initializing this to VRF_DEFAULT.

taken into account

ton31337 · 2022-06-08T07:28:35Z

doc/user/bmp.rst

@@ -109,12 +109,14 @@ Inside a ``bmp targets`` block, the following commands control session
 establishment:

 .. clicmd:: bmp connect HOSTNAME port (1-65535) {min-retry MSEC|max-retry MSEC}
+	    [source-interface <WORD>]


Can we keep this in the same line as above parameters?

NetDEF-CI · 2022-06-08T17:11:42Z

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-PULLREQ2-5818/

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 bgp_bmp.c | 18 issues
===============================================
< WARNING: space prohibited between function name and open parenthesis '('
< #2129: FILE: /tmp/f1-4841/bgp_bmp.c:2129:
< ERROR: space required after that ',' (ctx:VxV)
< #2129: FILE: /tmp/f1-4841/bgp_bmp.c:2129:
< ERROR: space required after that ',' (ctx:VxV)
< #2130: FILE: /tmp/f1-4841/bgp_bmp.c:2130:
< WARNING: quoted string split across lines
< #2131: FILE: /tmp/f1-4841/bgp_bmp.c:2131:
< WARNING: break quoted strings at a space character
< #2131: FILE: /tmp/f1-4841/bgp_bmp.c:2131:
< ERROR: space required after that ',' (ctx:VxV)
< #2422: FILE: /tmp/f1-4841/bgp_bmp.c:2422:
< ERROR: space required after that ',' (ctx:VxV)
< #2519: FILE: /tmp/f1-4841/bgp_bmp.c:2519:
< ERROR: space required after that ',' (ctx:VxV)
< #2519: FILE: /tmp/f1-4841/bgp_bmp.c:2519:
< ERROR: space required after that ',' (ctx:VxV)
< #2521: FILE: /tmp/f1-4841/bgp_bmp.c:2521:

NetDEF-CI · 2022-06-08T20:46:26Z

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-PULLREQ2-5822/

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 bgp_bmp.c | 4 issues
===============================================
< ERROR: space required after that ',' (ctx:VxV)
< #2129: FILE: /tmp/f1-28081/bgp_bmp.c:2129:
< WARNING: line over 80 characters
< #2519: FILE: /tmp/f1-28081/bgp_bmp.c:2519:

ton31337

Still, CI warnings are not yet fixed.

ton31337 · 2022-06-09T07:24:34Z

doc/user/bmp.rst


   Add/remove an active outbound BMP session.  HOSTNAME is resolved via DNS,
   if multiple addresses are returned they are tried in nondeterministic
   order.  Only one connection will be established even if multiple addresses
   are returned.  ``min-retry`` and ``max-retry`` specify (in milliseconds)
-   bounds for exponential backoff.
+   bounds for exponential backoff. source-interface is the locale interface on


Please highlight source-interface with double-ticks (``).

Also, typo locale.

ton31337 · 2022-06-09T07:25:33Z

doc/user/bmp.rst

@@ -108,13 +108,15 @@ BMP session configuration
 Inside a ``bmp targets`` block, the following commands control session
 establishment:

-.. clicmd:: bmp connect HOSTNAME port (1-65535) {min-retry MSEC|max-retry MSEC}
+
+.. clicmd:: bmp connect HOSTNAME port (1-65535) {min-retry MSEC|max-retry MSEC} [source-interface <WORD>]


<> not needed wrapping WORD

ton31337 · 2022-06-09T07:29:38Z

bgpd/bgp_bmp.c

@@ -2432,10 +2511,15 @@ static int bmp_config_write(struct bgp *bgp, struct vty *vty)
 				sockunion2str(&bl->addr, buf, SU_ADDRSTRLEN),
 				bl->port);

-		frr_each (bmp_actives, &bt->actives, ba)
+		frr_each (bmp_actives, &bt->actives, ba) {
 			vty_out(vty, "  bmp connect %s port %u min-retry %u max-retry %u\n",


Shouldn't \n be stripped for this vty_out?

NetDEF-CI · 2022-06-09T10:53:55Z

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-PULLREQ2-5831/

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 bgp_bmp.c | 4 issues
===============================================
< ERROR: space required after that ',' (ctx:VxV)
< #2129: FILE: /tmp/f1-20474/bgp_bmp.c:2129:
< WARNING: line over 80 characters
< #2519: FILE: /tmp/f1-20474/bgp_bmp.c:2519:

NetDEF-CI · 2022-06-09T12:03:30Z

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-PULLREQ2-5832/

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.

pguibert6WIND · 2022-06-10T10:16:03Z

ci:rerun

NetDEF-CI · 2022-06-10T12:43:55Z

Continuous Integration Result: FAILED

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

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.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 9: No useful log found

Successful on other platforms/tests

Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 arm8 part 8
Topotests debian 10 amd64 part 4
Topotests debian 10 amd64 part 9
Addresssanitizer topotests part 4
Topotests Ubuntu 18.04 i386 part 7
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 arm8 part 3
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 3
Ubuntu 16.04 deb pkg check
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 8
Topotests Ubuntu 18.04 i386 part 6
Ubuntu 20.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 4
Topotests Ubuntu 18.04 i386 part 1
Topotests Ubuntu 18.04 amd64 part 5
Addresssanitizer topotests part 2
Debian 10 deb pkg check
IPv6 protocols on Ubuntu 18.04
IPv4 ldp protocol on Ubuntu 18.04
Topotests Ubuntu 18.04 amd64 part 4
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 amd64 part 7
IPv4 protocols on Ubuntu 18.04
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 arm8 part 2
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 amd64 part 9
Topotests Ubuntu 18.04 arm8 part 7
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 arm8 part 5
Fedora 29 rpm pkg check
Addresssanitizer topotests part 6
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 i386 part 9
Topotests debian 10 amd64 part 1
Topotests Ubuntu 18.04 amd64 part 8
Topotests debian 10 amd64 part 2
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 amd64 part 0
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 5
Static analyzer (clang)
Topotests Ubuntu 18.04 i386 part 0
Topotests debian 10 amd64 part 3
Debian 9 deb pkg check
Topotests Ubuntu 18.04 amd64 part 1
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Ubuntu 18.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 1
Topotests Ubuntu 18.04 arm8 part 6
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 amd64 part 6
Topotests debian 10 amd64 part 5
Addresssanitizer topotests part 8

fdumontet6WIND · 2022-06-10T13:58:01Z

ci:rerun

fdumontet6WIND · 2022-06-10T13:58:59Z

previous CI tests were working

NetDEF-CI · 2022-06-10T16:32:14Z

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-PULLREQ2-5870/

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.

ton31337

A couple more nits, overall LGTM.

ton31337 · 2022-06-15T18:54:41Z

bgpd/bgp_bmp.c

@@ -1741,6 +1742,7 @@ static struct bmp_active *bmp_active_get(struct bmp_targets *bt,
 	ba->port = port;
 	ba->minretry = BMP_DFLT_MINRETRY;
 	ba->maxretry = BMP_DFLT_MAXRETRY;
+


Can we drop this new line from this change?

ton31337 · 2022-06-15T18:55:12Z

bgpd/bgp_bmp.c

@@ -1773,8 +1776,37 @@ static void bmp_active_connect(struct bmp_active *ba)
 {
 	enum connect_result res;
 	char buf[SU_ADDRSTRLEN];
+	struct interface *ifp;
+	vrf_id_t vrf_id = VRF_DEFAULT;
+	int  res_bind;


Can we drop additional (two) whitespace after int?

ton31337 · 2022-06-15T18:56:06Z

bgpd/bgp_bmp.c

+						  &ba->addrsrc);
+			if (res_bind < 0) {
+				sockunion2str(&ba->addrsrc, buf, sizeof(buf));
+				zlog_warn(


The same question still, is this supposed to be an error or warning? Failed seems to be related to error?

ton31337 · 2022-06-15T18:56:14Z

bgpd/bgp_bmp.c

 {
 	VTY_DECLVAR_CONTEXT_SUB(bmp_targets, bt);
 	struct bmp_active *ba;

+


Can we drop this new line from this change?

fdumontet6WIND · 2022-06-16T10:49:54Z

i changed the message associated to unsucessfull bind now:
zlog_warn(
"bmp[%s]: no bind currently to source address %s:%d",

With current release, forcin the source ip address when setting up a BMP connection is not possible. The need is to add an extra parameter for the following vty command: router bgp 65500 bmp targets AAA bmp connect 2.2.2.2 port 666 min-retry 100 max-retry 700 bmp connect 2:2::2:2 port 666 min-retry 100 max-retry 700 [source-interface lo1] Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>

NetDEF-CI · 2022-06-16T14:26:29Z

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-PULLREQ2-5958/

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.

github-actions bot added the master label Jun 7, 2022

ton31337 requested changes Jun 8, 2022

View reviewed changes

fdumontet6WIND requested a review from ton31337 June 8, 2022 17:13

ton31337 requested changes Jun 9, 2022

View reviewed changes

fdumontet6WIND requested a review from ton31337 June 10, 2022 08:11

ton31337 requested changes Jun 15, 2022

View reviewed changes

ton31337 approved these changes Jun 16, 2022

View reviewed changes

ton31337 merged commit aef69e4 into FRRouting:master Jun 16, 2022

bmp: add a interface source to bmp connect command #11353

bmp: add a interface source to bmp connect command #11353

Conversation

fdumontet6WIND commented Jun 7, 2022

NetDEF-CI commented Jun 7, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

ton31337 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NetDEF-CI commented Jun 8, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

NetDEF-CI commented Jun 8, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

ton31337 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NetDEF-CI commented Jun 9, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

NetDEF-CI commented Jun 9, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

pguibert6WIND commented Jun 10, 2022

NetDEF-CI commented Jun 10, 2022 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

fdumontet6WIND commented Jun 10, 2022

fdumontet6WIND commented Jun 10, 2022

NetDEF-CI commented Jun 10, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

ton31337 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fdumontet6WIND commented Jun 16, 2022

NetDEF-CI commented Jun 16, 2022

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Jun 7, 2022 •

edited

Loading

NetDEF-CI commented Jun 8, 2022 •

edited

Loading

NetDEF-CI commented Jun 8, 2022 •

edited

Loading

NetDEF-CI commented Jun 9, 2022 •

edited

Loading

NetDEF-CI commented Jun 9, 2022 •

edited

Loading

NetDEF-CI commented Jun 10, 2022 •

edited

Loading

NetDEF-CI commented Jun 10, 2022 •

edited

Loading