bgpd: Fix group overrides for inverted AF flags

This commit fixes peer-group overrides for inverted AF flags. This
implementation is currently only being used by the three 'send-community'
flags. Commit 70ee29b4d introduced generic support for overriding AF
flags, but did not support inverted flags.

By introducing an additional array on the BGP peer structure called
'af_flags_invert' all current and future flags which should work in an
inverted way can now also be properly overridden.

The CLI commands will work exactly the same way as before, just that 'no
<command>' now sets the flag and override whereas '<command>' will unset
the flag and remove the override.

Signed-off-by: Pascal Mathis <mail@pascalmathis.com>
(cherry picked from commit 27c05d4d43d14464b15582c700a511156c4ea2af)

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 07dd63c48b9db43ffe3a51cb67213b70997434b4..a22c0a5884a23361819e6369346df0448a28d837 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -4075,6 +4075,7 @@ DEFUN (neighbor_send_community,
        "Send Community attribute to this neighbor\n")
 {
        int idx_peer = 1;
+
        return peer_af_flag_set_vty(vty, argv[idx_peer]->arg, bgp_node_afi(vty),
                                    bgp_node_safi(vty),
                                    PEER_FLAG_SEND_COMMUNITY);
@@ -4094,6 +4095,7 @@ DEFUN (no_neighbor_send_community,
        "Send Community attribute to this neighbor\n")
 {
        int idx_peer = 2;
+
        return peer_af_flag_unset_vty(vty, argv[idx_peer]->arg,
                                      bgp_node_afi(vty), bgp_node_safi(vty),
                                      PEER_FLAG_SEND_COMMUNITY);
@@ -4117,27 +4119,26 @@ DEFUN (neighbor_send_community_type,
        "Send Standard Community attributes\n"
        "Send Large Community attributes\n")
 {
-       int idx = 0;
+       int idx_peer = 1;
        uint32_t flag = 0;
+       const char *type = argv[argc - 1]->text;
 
-       char *peer = argv[1]->arg;
-
-       if (argv_find(argv, argc, "standard", &idx))
+       if (strmatch(type, "standard")) {
                SET_FLAG(flag, PEER_FLAG_SEND_COMMUNITY);
-       else if (argv_find(argv, argc, "extended", &idx))
+       } else if (strmatch(type, "extended")) {
                SET_FLAG(flag, PEER_FLAG_SEND_EXT_COMMUNITY);
-       else if (argv_find(argv, argc, "large", &idx))
+       } else if (strmatch(type, "large")) {
                SET_FLAG(flag, PEER_FLAG_SEND_LARGE_COMMUNITY);
-       else if (argv_find(argv, argc, "both", &idx)) {
+       } else if (strmatch(type, "both")) {
                SET_FLAG(flag, PEER_FLAG_SEND_COMMUNITY);
                SET_FLAG(flag, PEER_FLAG_SEND_EXT_COMMUNITY);
-       } else {
+       } else { /* if (strmatch(type, "all")) */
                SET_FLAG(flag, PEER_FLAG_SEND_COMMUNITY);
                SET_FLAG(flag, PEER_FLAG_SEND_EXT_COMMUNITY);
                SET_FLAG(flag, PEER_FLAG_SEND_LARGE_COMMUNITY);
        }
 
-       return peer_af_flag_set_vty(vty, peer, bgp_node_afi(vty),
+       return peer_af_flag_set_vty(vty, argv[idx_peer]->arg, bgp_node_afi(vty),
                                    bgp_node_safi(vty), flag);
 }
 
@@ -4166,33 +4167,27 @@ DEFUN (no_neighbor_send_community_type,
        "Send Large Community attributes\n")
 {
        int idx_peer = 2;
-
+       uint32_t flag = 0;
        const char *type = argv[argc - 1]->text;
 
-       if (strmatch(type, "standard"))
-               return peer_af_flag_unset_vty(
-                       vty, argv[idx_peer]->arg, bgp_node_afi(vty),
-                       bgp_node_safi(vty), PEER_FLAG_SEND_COMMUNITY);
-       if (strmatch(type, "extended"))
-               return peer_af_flag_unset_vty(
-                       vty, argv[idx_peer]->arg, bgp_node_afi(vty),
-                       bgp_node_safi(vty), PEER_FLAG_SEND_EXT_COMMUNITY);
-       if (strmatch(type, "large"))
-               return peer_af_flag_unset_vty(
-                       vty, argv[idx_peer]->arg, bgp_node_afi(vty),
-                       bgp_node_safi(vty), PEER_FLAG_SEND_LARGE_COMMUNITY);
-       if (strmatch(type, "both"))
-               return peer_af_flag_unset_vty(
-                       vty, argv[idx_peer]->arg, bgp_node_afi(vty),
-                       bgp_node_safi(vty),
-                       PEER_FLAG_SEND_COMMUNITY
-                               | PEER_FLAG_SEND_EXT_COMMUNITY);
-
-       /* if (strmatch (type, "all")) */
-       return peer_af_flag_unset_vty(
-               vty, argv[idx_peer]->arg, bgp_node_afi(vty), bgp_node_safi(vty),
-               (PEER_FLAG_SEND_COMMUNITY | PEER_FLAG_SEND_EXT_COMMUNITY
-                | PEER_FLAG_SEND_LARGE_COMMUNITY));
+       if (strmatch(type, "standard")) {
+               SET_FLAG(flag, PEER_FLAG_SEND_COMMUNITY);
+       } else if (strmatch(type, "extended")) {
+               SET_FLAG(flag, PEER_FLAG_SEND_EXT_COMMUNITY);
+       } else if (strmatch(type, "large")) {
+               SET_FLAG(flag, PEER_FLAG_SEND_LARGE_COMMUNITY);
+       } else if (strmatch(type, "both")) {
+               SET_FLAG(flag, PEER_FLAG_SEND_COMMUNITY);
+               SET_FLAG(flag, PEER_FLAG_SEND_EXT_COMMUNITY);
+       } else { /* if (strmatch(type, "all")) */
+               SET_FLAG(flag, PEER_FLAG_SEND_COMMUNITY);
+               SET_FLAG(flag, PEER_FLAG_SEND_EXT_COMMUNITY);
+               SET_FLAG(flag, PEER_FLAG_SEND_LARGE_COMMUNITY);
+       }
+
+       return peer_af_flag_unset_vty(vty, argv[idx_peer]->arg,
+                                     bgp_node_afi(vty), bgp_node_safi(vty),
+                                     flag);
 }
 
 ALIAS_HIDDEN(

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index ed30d2e6a0467cb82a982a1b4817213044f59062..92b8c8523a787b72025860df303dc4209298bb82 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -807,8 +807,12 @@ void peer_af_flag_inherit(struct peer *peer, afi_t afi, safi_t safi,
 static bool peergroup_af_flag_check(struct peer *peer, afi_t afi, safi_t safi,
                                    uint32_t flag)
 {
-       if (!peer_group_active(peer))
-               return !!peer_af_flag_check(peer, afi, safi, flag);
+       if (!peer_group_active(peer)) {
+               if (CHECK_FLAG(peer->af_flags_invert[afi][safi], flag))
+                       return !peer_af_flag_check(peer, afi, safi, flag);
+               else
+                       return !!peer_af_flag_check(peer, afi, safi, flag);
+       }
 
        return !!CHECK_FLAG(peer->af_flags_override[afi][safi], flag);
 }
@@ -897,6 +901,13 @@ static void peer_af_flag_reset(struct peer *peer, afi_t afi, safi_t safi)
                         PEER_FLAG_SEND_EXT_COMMUNITY);
                SET_FLAG(peer->af_flags[afi][safi],
                         PEER_FLAG_SEND_LARGE_COMMUNITY);
+
+               SET_FLAG(peer->af_flags_invert[afi][safi],
+                        PEER_FLAG_SEND_COMMUNITY);
+               SET_FLAG(peer->af_flags_invert[afi][safi],
+                        PEER_FLAG_SEND_EXT_COMMUNITY);
+               SET_FLAG(peer->af_flags_invert[afi][safi],
+                        PEER_FLAG_SEND_LARGE_COMMUNITY);
        }
 
        /* Clear neighbor default_originate_rmap */
@@ -1170,7 +1181,7 @@ struct peer *peer_new(struct bgp *bgp)
        peer = peer_lock(peer); /* initial reference */
        peer->password = NULL;
 
-       /* Set default flags.  */
+       /* Set default flags. */
        FOREACH_AFI_SAFI (afi, safi) {
                if (!bgp_option_check(BGP_OPT_CONFIG_CISCO)) {
                        SET_FLAG(peer->af_flags[afi][safi],
@@ -1179,8 +1190,14 @@ struct peer *peer_new(struct bgp *bgp)
                                 PEER_FLAG_SEND_EXT_COMMUNITY);
                        SET_FLAG(peer->af_flags[afi][safi],
                                 PEER_FLAG_SEND_LARGE_COMMUNITY);
+
+                       SET_FLAG(peer->af_flags_invert[afi][safi],
+                                PEER_FLAG_SEND_COMMUNITY);
+                       SET_FLAG(peer->af_flags_invert[afi][safi],
+                                PEER_FLAG_SEND_EXT_COMMUNITY);
+                       SET_FLAG(peer->af_flags_invert[afi][safi],
+                                PEER_FLAG_SEND_LARGE_COMMUNITY);
                }
-               peer->orf_plist[afi][safi] = NULL;
        }
        SET_FLAG(peer->sflags, PEER_STATUS_CAPABILITY_OPEN);
 
@@ -1266,6 +1283,8 @@ void peer_xfer_config(struct peer *peer_dst, struct peer *peer_src)
        FOREACH_AFI_SAFI (afi, safi) {
                peer_dst->afc[afi][safi] = peer_src->afc[afi][safi];
                peer_dst->af_flags[afi][safi] = peer_src->af_flags[afi][safi];
+               peer_dst->af_flags_invert[afi][safi] =
+                       peer_src->af_flags_invert[afi][safi];
                peer_dst->allowas_in[afi][safi] =
                        peer_src->allowas_in[afi][safi];
                peer_dst->weight[afi][safi] = peer_src->weight[afi][safi];
@@ -1754,6 +1773,7 @@ static void peer_group2peer_config_copy_af(struct peer_group *group,
 
        /* peer af_flags apply */
        peer->af_flags[afi][safi] = conf->af_flags[afi][safi];
+       peer->af_flags_invert[afi][safi] = conf->af_flags_invert[afi][safi];
 
        /* maximum-prefix */
        peer->pmax[afi][safi] = conf->pmax[afi][safi];
@@ -4018,21 +4038,23 @@ int peer_flag_unset(struct peer *peer, uint32_t flag)
 }
 
 static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi,
-                              uint32_t flag, int set)
+                              uint32_t flag, bool set)
 {
        int found;
        int size;
+       int addpath_tx_used;
+       bool invert;
        struct listnode *node, *nnode;
        struct peer_group *group;
        struct peer_flag_action action;
        struct peer *tmp_peer;
        struct bgp *bgp;
-       int addpath_tx_used;
 
        memset(&action, 0, sizeof(struct peer_flag_action));
        size = sizeof peer_af_flag_action_list
               / sizeof(struct peer_flag_action);
 
+       invert = CHECK_FLAG(peer->af_flags_invert[afi][safi], flag);
        found = peer_flag_action_set(peer_af_flag_action_list, size, &action,
                                     flag);
 
@@ -4057,23 +4079,42 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi,
        /* When current flag configuration is same as requested one.  */
        if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
                if (set && CHECK_FLAG(peer->af_flags[afi][safi], flag)) {
-                       SET_FLAG(peer->af_flags_override[afi][safi], flag);
+                       if (invert)
+                               UNSET_FLAG(peer->af_flags_override[afi][safi],
+                                          flag);
+                       else
+                               SET_FLAG(peer->af_flags_override[afi][safi],
+                                        flag);
                        return 0;
                }
 
                if (!set && !CHECK_FLAG(peer->af_flags[afi][safi], flag)) {
-                       UNSET_FLAG(peer->af_flags_override[afi][safi], flag);
+                       if (invert)
+                               SET_FLAG(peer->af_flags_override[afi][safi],
+                                        flag);
+                       else
+                               UNSET_FLAG(peer->af_flags_override[afi][safi],
+                                          flag);
                        return 0;
                }
        }
 
        /* Set/unset flag or inherit from peer-group if appropriate. */
-       if (set)
-               SET_FLAG(peer->af_flags[afi][safi], flag);
-       else if (peer_group_active(peer))
-               peer_af_flag_inherit(peer, afi, safi, flag);
-       else
-               UNSET_FLAG(peer->af_flags[afi][safi], flag);
+       if (invert) {
+               if (!set)
+                       UNSET_FLAG(peer->af_flags[afi][safi], flag);
+               else if (peer_group_active(peer))
+                       peer_af_flag_inherit(peer, afi, safi, flag);
+               else
+                       SET_FLAG(peer->af_flags[afi][safi], flag);
+       } else {
+               if (set)
+                       SET_FLAG(peer->af_flags[afi][safi], flag);
+               else if (peer_group_active(peer))
+                       peer_af_flag_inherit(peer, afi, safi, flag);
+               else
+                       UNSET_FLAG(peer->af_flags[afi][safi], flag);
+       }
 
        /* Execute action when peer is established.  */
        if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)
@@ -4141,7 +4182,7 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi,
                        }
                }
        } else {
-               if (set)
+               if (set != invert)
                        SET_FLAG(peer->af_flags_override[afi][safi], flag);
                else
                        UNSET_FLAG(peer->af_flags_override[afi][safi], flag);
@@ -6988,6 +7029,7 @@ static void bgp_config_write_peer_af(struct vty *vty, struct bgp *bgp,
 {
        struct peer *g_peer = NULL;
        char *addr;
+       bool flag_scomm, flag_secomm, flag_slcomm;
 
        /* Skip dynamic neighbors. */
        if (peer_dynamic_neighbor(peer))
@@ -7114,79 +7156,51 @@ static void bgp_config_write_peer_af(struct vty *vty, struct bgp *bgp,
        }
 
        /* send-community print. */
-       if (bgp_option_check(BGP_OPT_CONFIG_CISCO)) {
-               if (peergroup_af_flag_check(peer, afi, safi,
-                                           PEER_FLAG_SEND_COMMUNITY)
-                   && peergroup_af_flag_check(peer, afi, safi,
-                                              PEER_FLAG_SEND_EXT_COMMUNITY)
-                   && peergroup_af_flag_check(
-                              peer, afi, safi,
-                              PEER_FLAG_SEND_LARGE_COMMUNITY)) {
-                       vty_out(vty, "  neighbor %s send-community all\n",
-                               addr);
-               } else if (peergroup_af_flag_check(
-                                  peer, afi, safi,
-                                  PEER_FLAG_SEND_LARGE_COMMUNITY)) {
-                       vty_out(vty, "  neighbor %s send-community large\n",
-                               addr);
-               } else if (peergroup_af_flag_check(
-                                  peer, afi, safi,
-                                  PEER_FLAG_SEND_EXT_COMMUNITY)) {
-                       vty_out(vty, "  neighbor %s send-community extended\n",
-                               addr);
-               } else if (peergroup_af_flag_check(peer, afi, safi,
-                                                  PEER_FLAG_SEND_COMMUNITY)) {
-                       vty_out(vty, "  neighbor %s send-community\n", addr);
-               }
-       } else {
-               if (!peer_af_flag_check(peer, afi, safi,
-                                       PEER_FLAG_SEND_COMMUNITY)
-                   && (!g_peer || peer_af_flag_check(g_peer, afi, safi,
-                                                     PEER_FLAG_SEND_COMMUNITY))
-                   && !peer_af_flag_check(peer, afi, safi,
-                                          PEER_FLAG_SEND_EXT_COMMUNITY)
-                   && (!g_peer
-                       || peer_af_flag_check(g_peer, afi, safi,
-                                             PEER_FLAG_SEND_EXT_COMMUNITY))
-                   && !peer_af_flag_check(peer, afi, safi,
-                                          PEER_FLAG_SEND_LARGE_COMMUNITY)
-                   && (!g_peer || peer_af_flag_check(
-                                          g_peer, afi, safi,
-                                          PEER_FLAG_SEND_LARGE_COMMUNITY))) {
+       flag_scomm = peergroup_af_flag_check(peer, afi, safi,
+                                           PEER_FLAG_SEND_COMMUNITY);
+       flag_secomm = peergroup_af_flag_check(peer, afi, safi,
+                                            PEER_FLAG_SEND_EXT_COMMUNITY);
+       flag_slcomm = peergroup_af_flag_check(peer, afi, safi,
+                                            PEER_FLAG_SEND_LARGE_COMMUNITY);
+
+       if (!bgp_option_check(BGP_OPT_CONFIG_CISCO)) {
+               if (flag_scomm && flag_secomm && flag_slcomm) {
                        vty_out(vty, "  no neighbor %s send-community all\n",
                                addr);
                } else {
-                       if (!peer_af_flag_check(peer, afi, safi,
-                                               PEER_FLAG_SEND_LARGE_COMMUNITY)
-                           && (!g_peer
-                               || peer_af_flag_check(
-                                          g_peer, afi, safi,
-                                          PEER_FLAG_SEND_LARGE_COMMUNITY))) {
+                       if (flag_scomm)
                                vty_out(vty,
-                                       "  no neighbor %s send-community large\n",
+                                       "  no neighbor %s send-community\n",
                                        addr);
-                       }
-
-                       if (!peer_af_flag_check(peer, afi, safi,
-                                               PEER_FLAG_SEND_EXT_COMMUNITY)
-                           && (!g_peer
-                               || peer_af_flag_check(
-                                          g_peer, afi, safi,
-                                          PEER_FLAG_SEND_EXT_COMMUNITY))) {
+                       if (flag_secomm)
                                vty_out(vty,
                                        "  no neighbor %s send-community extended\n",
                                        addr);
-                       }
 
-                       if (!peer_af_flag_check(peer, afi, safi,
-                                               PEER_FLAG_SEND_COMMUNITY)
-                           && (!g_peer || peer_af_flag_check(
-                                                  g_peer, afi, safi,
-                                                  PEER_FLAG_SEND_COMMUNITY))) {
+                       if (flag_slcomm)
                                vty_out(vty,
-                                       "  no neighbor %s send-community\n",
+                                       "  no neighbor %s send-community large\n",
+                                       addr);
+               }
+       } else {
+               if (flag_scomm && flag_secomm && flag_slcomm) {
+                       vty_out(vty, "  neighbor %s send-community all\n",
+                               addr);
+               } else if (flag_scomm && flag_secomm) {
+                       vty_out(vty, "  neighbor %s send-community both\n",
+                               addr);
+               } else {
+                       if (flag_scomm)
+                               vty_out(vty, "  neighbor %s send-community\n",
+                                       addr);
+                       if (flag_secomm)
+                               vty_out(vty,
+                                       "  neighbor %s send-community extended\n",
+                                       addr);
+                       if (flag_slcomm)
+                               vty_out(vty,
+                                       "  neighbor %s send-community large\n",
                                        addr);
-                       }
                }
        }
 

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index c866ec6327e2936e2710e418cc94a00470f9f399..b22152c287cbf5a9e08e2983f7390c7e678ee948 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -858,6 +858,17 @@ struct peer {
         *   *peer-specific*.
         */
        uint32_t af_flags_override[AFI_MAX][SAFI_MAX];
+       /*
+        * Parallel array to af_flags that indicates whether each flag should
+        * be treated as regular (defaults to 0) or inverted (defaults to 1).
+        * If a flag is set to 1 by default, the same bit should be set here.
+        *
+        * Notes:
+        * - This does *not* contain the flag values, rather it contains
+        *   whether the flag at the same position in af_flags is *regular* or
+        *   *inverted*.
+        */
+       uint32_t af_flags_invert[AFI_MAX][SAFI_MAX];
        /*
         * Effective flags, computed by applying peer-group flags and then
         * overriding with individual flags