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>

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 257adda3f3f228dcffa4d670a473f6ea299e889a..f83f357e5343d4fa93dfc27023102bfd9a24fd4d 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -4073,6 +4073,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);
@@ -4092,6 +4093,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);
@@ -4115,27 +4117,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);
 }
 
@@ -4164,33 +4165,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 7299654d25db392df98f951b406e3de0ee1f522a..8dd756bec0f8dfbc616e487f148ac5c7f6d1e7a6 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -824,8 +824,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);
 }
@@ -914,6 +918,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 */
@@ -1187,7 +1198,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],
@@ -1196,8 +1207,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 nexthop-unchanged for l2vpn evpn by default */
@@ -1288,6 +1305,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];
@@ -1776,6 +1795,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];
@@ -4058,21 +4078,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);
 
@@ -4097,12 +4119,22 @@ 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;
                }
        }
@@ -4137,12 +4169,21 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi,
        }
 
        /* 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)
@@ -4210,7 +4251,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);
@@ -7049,6 +7090,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))
@@ -7175,79 +7217,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 bfa80c3ce2a2aa96c9a563ea95ec4afe387e3e1a..97061681f00d5299f4d95446507a236f1403a215 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