-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[FRR]: Port some patches from sonic-quagga repo #3017
Merged
pavel-shirshov
merged 11 commits into
sonic-net:master
from
pavel-shirshov:pavelsh/frr_patches
Jun 23, 2019
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ee4613c
Update sonic-quagga submodule
pavel-shirshov b12e064
Merge branch 'master' of https://github.com/Azure/sonic-buildimage
pavel-shirshov b562508
Merge branch 'master' of https://github.com/Azure/sonic-buildimage
pavel-shirshov 11c2f0c
Merge branch 'master' of https://github.com/Azure/sonic-buildimage
pavel-shirshov 8aaee9a
Port some patches from sonic-quagga
pavel-shirshov a82060f
Fix Makefile
pavel-shirshov b734609
Another patch
pavel-shirshov eb434db
Uncomment bgp test
pavel-shirshov 69f81fc
Downport Nikos's patch
pavel-shirshov 80e6322
Add a patch to alleviate the vendor issue
pavel-shirshov d62e0ff
use patch instead of stg
pavel-shirshov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
141 changes: 141 additions & 0 deletions
141
src/sonic-frr/patch/0001-Add-support-of-bgp-tcp-DSCP-value.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| From ef017a613691a40f32cdaa5396bbd4889b1cb647 Mon Sep 17 00:00:00 2001 | ||
| From: Pavel Shirshov <pavelsh@microsoft.com> | ||
| Date: Fri, 14 Jun 2019 17:45:31 -0700 | ||
| Subject: [PATCH] Add support of bgp tcp DSCP value | ||
|
|
||
| --- | ||
| bgpd/bgp_network.c | 11 ++++------- | ||
| bgpd/bgp_vty.c | 40 ++++++++++++++++++++++++++++++++++++++++ | ||
| bgpd/bgpd.c | 5 ++++- | ||
| bgpd/bgpd.h | 3 +++ | ||
| 4 files changed, 51 insertions(+), 8 deletions(-) | ||
|
|
||
| diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c | ||
| index 4153da5a6..fed133eb7 100644 | ||
| --- a/bgpd/bgp_network.c | ||
| +++ b/bgpd/bgp_network.c | ||
| @@ -569,11 +569,9 @@ int bgp_connect(struct peer *peer) | ||
| #ifdef IPTOS_PREC_INTERNETCONTROL | ||
| frr_elevate_privs(&bgpd_privs) { | ||
| if (sockunion_family(&peer->su) == AF_INET) | ||
| - setsockopt_ipv4_tos(peer->fd, | ||
| - IPTOS_PREC_INTERNETCONTROL); | ||
| + setsockopt_ipv4_tos(peer->fd, peer->bgp->tcp_dscp); | ||
| else if (sockunion_family(&peer->su) == AF_INET6) | ||
| - setsockopt_ipv6_tclass(peer->fd, | ||
| - IPTOS_PREC_INTERNETCONTROL); | ||
| + setsockopt_ipv6_tclass(peer->fd, peer->bgp->tcp_dscp); | ||
| } | ||
| #endif | ||
|
|
||
| @@ -643,10 +641,9 @@ static int bgp_listener(int sock, struct sockaddr *sa, socklen_t salen, | ||
|
|
||
| #ifdef IPTOS_PREC_INTERNETCONTROL | ||
| if (sa->sa_family == AF_INET) | ||
| - setsockopt_ipv4_tos(sock, IPTOS_PREC_INTERNETCONTROL); | ||
| + setsockopt_ipv4_tos(sock, bgp->tcp_dscp); | ||
| else if (sa->sa_family == AF_INET6) | ||
| - setsockopt_ipv6_tclass(sock, | ||
| - IPTOS_PREC_INTERNETCONTROL); | ||
| + setsockopt_ipv6_tclass(sock, bgp->tcp_dscp); | ||
| #endif | ||
|
|
||
| sockopt_v6only(sa->sa_family, sock); | ||
| diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c | ||
| index 7445df883..f91b908a4 100644 | ||
| --- a/bgpd/bgp_vty.c | ||
| +++ b/bgpd/bgp_vty.c | ||
| @@ -1113,6 +1113,42 @@ DEFUN (no_router_bgp, | ||
| return CMD_SUCCESS; | ||
| } | ||
|
|
||
| +/* bgp session-dscp */ | ||
| + | ||
| +DEFUN (bgp_session_dscp, | ||
| + bgp_session_dscp_cmd, | ||
| + "bgp session-dscp DSCP", | ||
| + BGP_STR | ||
| + "Override default (C0) bgp TCP session DSCP value\n" | ||
| + "Manually configured dscp parameter\n") | ||
| +{ | ||
| + struct bgp *bgp = VTY_GET_CONTEXT(bgp); | ||
| + | ||
| + uint8_t value = (uint8_t)strtol(argv[2]->arg, NULL, 16); | ||
| + if ((value == 0 && errno == EINVAL) || (value > 0x3f)) | ||
| + { | ||
| + vty_out (vty, "%% Malformed bgp session-dscp parameter\n"); | ||
| + return CMD_WARNING_CONFIG_FAILED; | ||
| + } | ||
| + | ||
| + bgp->tcp_dscp = value << 2; | ||
| + | ||
| + return CMD_SUCCESS; | ||
| +} | ||
| + | ||
| +DEFUN (no_bgp_session_dscp, | ||
| + no_bgp_session_dscp_cmd, | ||
| + "no bgp session-dscp", | ||
| + NO_STR | ||
| + BGP_STR | ||
| + "Override default (C0) bgp tcp session ip dscp value\n") | ||
| +{ | ||
| + struct bgp *bgp = VTY_GET_CONTEXT(bgp); | ||
| + | ||
| + bgp->tcp_dscp = IPTOS_PREC_INTERNETCONTROL; | ||
| + | ||
| + return CMD_SUCCESS; | ||
| +} | ||
|
|
||
| /* BGP router-id. */ | ||
|
|
||
| @@ -12798,6 +12834,10 @@ void bgp_vty_init(void) | ||
| /* "no router bgp" commands. */ | ||
| install_element(CONFIG_NODE, &no_router_bgp_cmd); | ||
|
|
||
| + /* "bgp session-dscp command */ | ||
| + install_element (BGP_NODE, &bgp_session_dscp_cmd); | ||
| + install_element (BGP_NODE, &no_bgp_session_dscp_cmd); | ||
| + | ||
| /* "bgp router-id" commands. */ | ||
| install_element(BGP_NODE, &bgp_router_id_cmd); | ||
| install_element(BGP_NODE, &no_bgp_router_id_cmd); | ||
| diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c | ||
| index 6fb3a70c7..503e6b4ed 100644 | ||
| --- a/bgpd/bgpd.c | ||
| +++ b/bgpd/bgpd.c | ||
| @@ -2995,7 +2995,7 @@ static struct bgp *bgp_create(as_t *as, const char *name, | ||
|
|
||
| bgp->evpn_info = XCALLOC(MTYPE_BGP_EVPN_INFO, | ||
| sizeof(struct bgp_evpn_info)); | ||
| - | ||
| + bgp->tcp_dscp = IPTOS_PREC_INTERNETCONTROL; | ||
| bgp_evpn_init(bgp); | ||
| bgp_pbr_init(bgp); | ||
| return bgp; | ||
| @@ -7516,6 +7516,9 @@ int bgp_config_write(struct vty *vty) | ||
| if (CHECK_FLAG(bgp->flags, BGP_FLAG_NO_FAST_EXT_FAILOVER)) | ||
| vty_out(vty, " no bgp fast-external-failover\n"); | ||
|
|
||
| + if (bgp->tcp_dscp != IPTOS_PREC_INTERNETCONTROL) | ||
| + vty_out(vty, " bgp session-dscp %02X\n", bgp->tcp_dscp >> 2); | ||
| + | ||
| /* BGP router ID. */ | ||
| if (bgp->router_id_static.s_addr != 0) | ||
| vty_out(vty, " bgp router-id %s\n", | ||
| diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h | ||
| index df3fde840..a6546457c 100644 | ||
| --- a/bgpd/bgpd.h | ||
| +++ b/bgpd/bgpd.h | ||
| @@ -553,6 +553,9 @@ struct bgp { | ||
| /* Count of peers in established state */ | ||
| uint32_t established_peers; | ||
|
|
||
| + /* dscp value for tcp sessions */ | ||
| + uint8_t tcp_dscp; | ||
| + | ||
| QOBJ_FIELDS | ||
| }; | ||
| DECLARE_QOBJ_TYPE(bgp) | ||
| -- | ||
| 2.17.1.windows.2 | ||
|
|
25 changes: 25 additions & 0 deletions
25
src/sonic-frr/patch/0002-Reduce-severity-of-Vty-connected-from-message.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| From 87760a6a04d6ffbcc7a1093549bfb76656002b61 Mon Sep 17 00:00:00 2001 | ||
| From: Pavel Shirshov <pavelsh@microsoft.com> | ||
| Date: Fri, 14 Jun 2019 17:48:50 -0700 | ||
| Subject: [PATCH] Reduce severity of 'Vty connected from' message | ||
|
|
||
| --- | ||
| lib/vty.c | 2 +- | ||
| 1 file changed, 1 insertion(+), 1 deletion(-) | ||
|
|
||
| diff --git a/lib/vty.c b/lib/vty.c | ||
| index 8450922c2..f159d1b4b 100644 | ||
| --- a/lib/vty.c | ||
| +++ b/lib/vty.c | ||
| @@ -1875,7 +1875,7 @@ static int vty_accept(struct thread *thread) | ||
| zlog_info("can't set sockopt to vty_sock : %s", | ||
| safe_strerror(errno)); | ||
|
|
||
| - zlog_info("Vty connection from %s", | ||
| + zlog_debug("Vty connection from %s", | ||
| sockunion2str(&su, buf, SU_ADDRSTRLEN)); | ||
|
|
||
| vty_create(vty_sock, &su); | ||
| -- | ||
| 2.17.1.windows.2 | ||
|
|
156 changes: 156 additions & 0 deletions
156
src/sonic-frr/patch/0003-ignore-nexthop-attribute-when-NLRI-is-present.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| From 7c31178d8f1b8cc3a9627dc7c7bd1d2a51fe54ce Mon Sep 17 00:00:00 2001 | ||
| From: Pavel Shirshov <pavelsh@microsoft.com> | ||
| Date: Tue, 18 Jun 2019 15:27:19 -0700 | ||
| Subject: [PATCH] ignore nexthop attribute when NLRI is present | ||
|
|
||
| Backport of https://github.com/FRRouting/frr/pull/4258 | ||
| --- | ||
| bgpd/bgp_attr.c | 73 ++++++++++++++++++++++++++++++----------------- | ||
| bgpd/bgp_attr.h | 3 ++ | ||
| bgpd/bgp_packet.c | 11 +++++++ | ||
| 3 files changed, 61 insertions(+), 26 deletions(-) | ||
|
|
||
| diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c | ||
| index 05e103142..ea7f761ab 100644 | ||
| --- a/bgpd/bgp_attr.c | ||
| +++ b/bgpd/bgp_attr.c | ||
| @@ -1257,6 +1257,32 @@ static int bgp_attr_as4_path(struct bgp_attr_parser_args *args, | ||
| return BGP_ATTR_PARSE_PROCEED; | ||
| } | ||
|
|
||
| +/* | ||
| + * Check that the nexthop attribute is valid. | ||
| + */ | ||
| +bgp_attr_parse_ret_t | ||
| +bgp_attr_nexthop_valid(struct peer *peer, struct attr *attr) | ||
| +{ | ||
| + in_addr_t nexthop_h; | ||
| + | ||
| + nexthop_h = ntohl(attr->nexthop.s_addr); | ||
| + if ((IPV4_NET0(nexthop_h) || IPV4_NET127(nexthop_h) | ||
| + || IPV4_CLASS_DE(nexthop_h)) | ||
| + && !BGP_DEBUG(allow_martians, ALLOW_MARTIANS)) { | ||
| + char buf[INET_ADDRSTRLEN]; | ||
| + | ||
| + inet_ntop(AF_INET, &attr->nexthop.s_addr, buf, | ||
| + INET_ADDRSTRLEN); | ||
| + flog_err(EC_BGP_ATTR_MARTIAN_NH, "Martian nexthop %s", | ||
| + buf); | ||
| + bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, | ||
| + BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP); | ||
| + return BGP_ATTR_PARSE_ERROR; | ||
| + } | ||
| + | ||
| + return BGP_ATTR_PARSE_PROCEED; | ||
| +} | ||
| + | ||
| /* Nexthop attribute. */ | ||
| static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args) | ||
| { | ||
| @@ -1264,8 +1290,6 @@ static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args) | ||
| struct attr *const attr = args->attr; | ||
| const bgp_size_t length = args->length; | ||
|
|
||
| - in_addr_t nexthop_h, nexthop_n; | ||
| - | ||
| /* Check nexthop attribute length. */ | ||
| if (length != 4) { | ||
| flog_err(EC_BGP_ATTR_LEN, | ||
| @@ -1275,30 +1299,7 @@ static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args) | ||
| args->total); | ||
| } | ||
|
|
||
| - /* According to section 6.3 of RFC4271, syntactically incorrect NEXT_HOP | ||
| - attribute must result in a NOTIFICATION message (this is implemented | ||
| - below). | ||
| - At the same time, semantically incorrect NEXT_HOP is more likely to | ||
| - be just | ||
| - logged locally (this is implemented somewhere else). The UPDATE | ||
| - message | ||
| - gets ignored in any of these cases. */ | ||
| - nexthop_n = stream_get_ipv4(peer->curr); | ||
| - nexthop_h = ntohl(nexthop_n); | ||
| - if ((IPV4_NET0(nexthop_h) || IPV4_NET127(nexthop_h) | ||
| - || IPV4_CLASS_DE(nexthop_h)) | ||
| - && !BGP_DEBUG( | ||
| - allow_martians, | ||
| - ALLOW_MARTIANS)) /* loopbacks may be used in testing */ | ||
| - { | ||
| - char buf[INET_ADDRSTRLEN]; | ||
| - inet_ntop(AF_INET, &nexthop_n, buf, INET_ADDRSTRLEN); | ||
| - flog_err(EC_BGP_ATTR_MARTIAN_NH, "Martian nexthop %s", buf); | ||
| - return bgp_attr_malformed( | ||
| - args, BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP, args->total); | ||
| - } | ||
| - | ||
| - attr->nexthop.s_addr = nexthop_n; | ||
| + attr->nexthop.s_addr = stream_get_ipv4(peer->curr); | ||
| attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP); | ||
|
|
||
| return BGP_ATTR_PARSE_PROCEED; | ||
| @@ -2669,6 +2670,26 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, | ||
| return BGP_ATTR_PARSE_ERROR; | ||
| } | ||
|
|
||
| + /* | ||
| + * RFC4271: If the NEXT_HOP attribute field is syntactically incorrect, | ||
| + * then the Error Subcode MUST be set to Invalid NEXT_HOP Attribute. | ||
| + * This is implemented below and will result in a NOTIFICATION. If the | ||
| + * NEXT_HOP attribute is semantically incorrect, the error SHOULD be | ||
| + * logged, and the route SHOULD be ignored. In this case, a NOTIFICATION | ||
| + * message SHOULD NOT be sent. This is implemented elsewhere. | ||
| + * | ||
| + * RFC4760: An UPDATE message that carries no NLRI, other than the one | ||
| + * encoded in the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP | ||
| + * attribute. If such a message contains the NEXT_HOP attribute, the BGP | ||
| + * speaker that receives the message SHOULD ignore this attribute. | ||
| + */ | ||
| + if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP)) | ||
| + && !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) { | ||
| + if (bgp_attr_nexthop_valid(peer, attr) < 0) { | ||
| + return BGP_ATTR_PARSE_ERROR; | ||
| + } | ||
| + } | ||
| + | ||
| /* Check all mandatory well-known attributes are present */ | ||
| if ((ret = bgp_attr_check(peer, attr)) < 0) { | ||
| if (as4_path) | ||
| diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h | ||
| index 47a4182fe..a86684583 100644 | ||
| --- a/bgpd/bgp_attr.h | ||
| +++ b/bgpd/bgp_attr.h | ||
| @@ -350,6 +350,9 @@ extern void bgp_packet_mpunreach_prefix(struct stream *s, struct prefix *p, | ||
| uint32_t, int, uint32_t, struct attr *); | ||
| extern void bgp_packet_mpunreach_end(struct stream *s, size_t attrlen_pnt); | ||
|
|
||
| +extern bgp_attr_parse_ret_t bgp_attr_nexthop_valid(struct peer *peer, | ||
| + struct attr *attr); | ||
| + | ||
| static inline int bgp_rmap_nhop_changed(uint32_t out_rmap_flags, | ||
| uint32_t in_rmap_flags) | ||
| { | ||
| diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c | ||
| index fe8a1a256..13f4cf436 100644 | ||
| --- a/bgpd/bgp_packet.c | ||
| +++ b/bgpd/bgp_packet.c | ||
| @@ -1533,6 +1533,17 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) | ||
| nlris[NLRI_UPDATE].nlri = stream_pnt(s); | ||
| nlris[NLRI_UPDATE].length = update_len; | ||
| stream_forward_getp(s, update_len); | ||
| + | ||
| + if (CHECK_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) { | ||
| + /* | ||
| + * We skipped nexthop attribute validation earlier so | ||
| + * validate the nexthop now. | ||
| + */ | ||
| + if (bgp_attr_nexthop_valid(peer, &attr) < 0) { | ||
| + bgp_attr_unintern_sub(&attr); | ||
| + return BGP_Stop; | ||
| + } | ||
| + } | ||
| } | ||
|
|
||
| if (BGP_DEBUG(update, UPDATE_IN)) | ||
| -- | ||
| 2.17.1.windows.2 | ||
|
|
27 changes: 27 additions & 0 deletions
27
src/sonic-frr/patch/0004-Allow-BGP-attr-NEXT_HOP-to-be-0.0.0.0-due-to-allevia.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| From 4cd83e56e4f67fdc06325d92a82534fb4cb69952 Mon Sep 17 00:00:00 2001 | ||
| From: Pavel Shirshov <pavelsh@microsoft.com> | ||
| Date: Thu, 20 Jun 2019 15:35:50 -0700 | ||
| Subject: [PATCH] Allow BGP attr NEXT_HOP to be 0.0.0.0 due to alleviate the | ||
| vendor bug | ||
|
|
||
| --- | ||
| bgpd/bgp_route.c | 3 +-- | ||
| 1 file changed, 1 insertion(+), 2 deletions(-) | ||
|
|
||
| diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c | ||
| index 38f3cad5a..55240eab8 100644 | ||
| --- a/bgpd/bgp_route.c | ||
| +++ b/bgpd/bgp_route.c | ||
| @@ -2873,8 +2873,7 @@ static int bgp_update_martian_nexthop(struct bgp *bgp, afi_t afi, safi_t safi, | ||
|
|
||
| /* If NEXT_HOP is present, validate it. */ | ||
| if (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP)) { | ||
| - if (attr->nexthop.s_addr == 0 | ||
| - || IPV4_CLASS_DE(ntohl(attr->nexthop.s_addr)) | ||
| + if (IPV4_CLASS_DE(ntohl(attr->nexthop.s_addr)) | ||
| || bgp_nexthop_self(bgp, attr->nexthop)) | ||
| return 1; | ||
| } | ||
| -- | ||
| 2.17.1.windows.2 | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| 0001-Add-support-of-bgp-tcp-DSCP-value.patch | ||
| 0002-Reduce-severity-of-Vty-connected-from-message.patch | ||
| 0003-ignore-nexthop-attribute-when-NLRI-is-present.patch | ||
| 0004-Allow-BGP-attr-NEXT_HOP-to-be-0.0.0.0-due-to-allevia.patch |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What failure are you seeing with the presence of the previous patch above that's forcing you to make this change?
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.
https://github.com/Azure/sonic-buildimage/blob/master/platform/vs/tests/bgp/test_invalid_nexthop.py to pass the test
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.
@pavel-shirshov @lguohan This isn't the right way to address the case of an IPv6 NLRI being sent and a nexthop attribute of 0. The flag needs to be either cleared or the martian check needs to happen earlier for the relevant AFI/SAFIs. The diffs are just punching a hole in the code but the nexthop attribute flag still remains set. So for AFI/SAFI 1/1 with IPv6 nexthop and also the nexthop attribute present, this is still a problem. Same goes for any AFI/SAFI that is sent along with the 0 nexthop attribute - we will be skipping a valid check.
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.
@nikos-github Currently, when we allow bgpd to have 0.0.0.0 as next-hop ip address, bgpd tries to resolve the next-hop using zebra, zebra responds with the message "unresolved" and this next-hop is not used.
Also, why do you consider that this check was valid? I tried to find in the RFC why NEXT_HOP attribute can't be 0.0.0.0 and I didn't find anything. Probably I miss something. Can you please point me to the place in the RFC which says us we should filter-out next_hops equal to 0.0.0.0?
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.
@paulnice There is no such thing as nexthop of 0.0.0.0 for a prefix learned via a neighbor. Nexthops for prefixes received by peers, correspond to an interface address. 0.0.0.0 is not a valid interface address. It's all described in the RFC.