]> git.proxmox.com Git - mirror_frr.git/blobdiff - bgpd/bgp_attr.c
bgpd: IPv6 session flapping with MP_REACH_NLRI and 0.0.0.0 in NEXT_HOP attribute
[mirror_frr.git] / bgpd / bgp_attr.c
index aa271da07f102f3a21bee75a61b980b6755db2b0..cbfa166cf36620daa333b3a39d9375c7df7c12d3 100644 (file)
@@ -54,6 +54,7 @@
 #include "bgp_encap_types.h"
 #include "bgp_evpn.h"
 #include "bgp_flowspec_private.h"
+#include "bgp_mac.h"
 
 /* Attribute strings for logging. */
 static const struct message attr_str[] = {
@@ -158,8 +159,7 @@ static bool cluster_hash_cmp(const void *p1, const void *p2)
 
 static void cluster_free(struct cluster_list *cluster)
 {
-       if (cluster->list)
-               XFREE(MTYPE_CLUSTER_VAL, cluster->list);
+       XFREE(MTYPE_CLUSTER_VAL, cluster->list);
        XFREE(MTYPE_CLUSTER, cluster);
 }
 
@@ -400,8 +400,7 @@ static struct hash *transit_hash;
 
 static void transit_free(struct transit *transit)
 {
-       if (transit->val)
-               XFREE(MTYPE_TRANSIT_VAL, transit->val);
+       XFREE(MTYPE_TRANSIT_VAL, transit->val);
        XFREE(MTYPE_TRANSIT, transit);
 }
 
@@ -592,9 +591,9 @@ static void attrhash_finish(void)
        attrhash = NULL;
 }
 
-static void attr_show_all_iterator(struct hash_backet *backet, struct vty *vty)
+static void attr_show_all_iterator(struct hash_bucket *bucket, struct vty *vty)
 {
-       struct attr *attr = backet->data;
+       struct attr *attr = bucket->data;
 
        vty_out(vty, "attr[%ld] nexthop %s\n", attr->refcnt,
                inet_ntoa(attr->nexthop));
@@ -605,7 +604,7 @@ static void attr_show_all_iterator(struct hash_backet *backet, struct vty *vty)
 
 void attr_show_all(struct vty *vty)
 {
-       hash_iterate(attrhash, (void (*)(struct hash_backet *,
+       hash_iterate(attrhash, (void (*)(struct hash_bucket *,
                                         void *))attr_show_all_iterator,
                     vty);
 }
@@ -1257,6 +1256,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 +1289,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 +1298,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;
@@ -1716,7 +1716,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
                stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
                if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) {
                        if (!peer->nexthop.ifp) {
-                               zlog_warn("%s: interface not set appropriately to handle some attributes",
+                               zlog_warn("%s: Received a V6/VPNV6 Global attribute but address is a V6 LL and we have no peer interface information, withdrawing",
                                          peer->host);
                                return BGP_ATTR_PARSE_WITHDRAW;
                        }
@@ -1733,7 +1733,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
                stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
                if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) {
                        if (!peer->nexthop.ifp) {
-                               zlog_warn("%s: interface not set appropriately to handle some attributes",
+                               zlog_warn("%s: Received V6/VPNV6 Global and LL attribute but global address is a V6 LL and we have no peer interface information, withdrawing",
                                          peer->host);
                                return BGP_ATTR_PARSE_WITHDRAW;
                        }
@@ -1763,7 +1763,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
                        attr->mp_nexthop_len = IPV6_MAX_BYTELEN;
                }
                if (!peer->nexthop.ifp) {
-                       zlog_warn("%s: Interface not set appropriately to handle this some attributes",
+                       zlog_warn("%s: Received a V6 LL nexthop and we have no peer interface information, withdrawing",
                                  peer->host);
                        return BGP_ATTR_PARSE_WITHDRAW;
                }
@@ -1944,7 +1944,18 @@ bgp_attr_ext_communities(struct bgp_attr_parser_args *args)
        bgp_attr_evpn_na_flag(attr, &attr->router_flag);
 
        /* Extract the Rmac, if any */
-       bgp_attr_rmac(attr, &attr->rmac);
+       if (bgp_attr_rmac(attr, &attr->rmac)) {
+               if (bgp_debug_update(peer, NULL, NULL, 1) &&
+                   bgp_mac_exist(&attr->rmac)) {
+                       char buf1[ETHER_ADDR_STRLEN];
+
+                       zlog_debug("%s: router mac %s is self mac",
+                                  __func__,
+                                  prefix_mac2str(&attr->rmac, buf1,
+                                                 sizeof(buf1)));
+               }
+
+       }
 
        return BGP_ATTR_PARSE_PROCEED;
 }
@@ -2671,6 +2682,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)
@@ -3016,6 +3047,22 @@ void bgp_packet_mpattr_end(struct stream *s, size_t sizep)
        stream_putw_at(s, sizep, (stream_get_endp(s) - sizep) - 2);
 }
 
+static int bgp_append_local_as(struct peer *peer, afi_t afi, safi_t safi)
+{
+       if (!BGP_AS_IS_PRIVATE(peer->local_as)
+           || (BGP_AS_IS_PRIVATE(peer->local_as)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS_ALL)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE)))
+               return 1;
+       return 0;
+}
+
 /* Make attribute packet. */
 bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
                                struct stream *s, struct attr *attr,
@@ -3081,12 +3128,12 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
                                /* If replace-as is specified, we only use the
                                   change_local_as when
                                   advertising routes. */
-                               if (!CHECK_FLAG(
-                                           peer->flags,
-                                           PEER_FLAG_LOCAL_AS_REPLACE_AS)) {
-                                       aspath = aspath_add_seq(aspath,
-                                                               peer->local_as);
-                               }
+                               if (!CHECK_FLAG(peer->flags,
+                                               PEER_FLAG_LOCAL_AS_REPLACE_AS))
+                                       if (bgp_append_local_as(peer, afi,
+                                                               safi))
+                                               aspath = aspath_add_seq(
+                                                       aspath, peer->local_as);
                                aspath = aspath_add_seq(aspath,
                                                        peer->change_local_as);
                        } else {