]> git.proxmox.com Git - mirror_frr.git/commitdiff
pbrd: Convert pnhc->nexthop to it's own data
authorDonald Sharp <sharpd@cumulusnetworks.com>
Mon, 24 Aug 2020 19:25:34 +0000 (15:25 -0400)
committerDonald Sharp <sharpd@cumulusnetworks.com>
Fri, 28 Aug 2020 11:51:06 +0000 (07:51 -0400)
The pnhc->nexthop was a pointer copy.  Causing issues
with the ability to move pointers around for the
different pnhc since the pnhc mirrored the nexthop
caches.  When we received a vrf change if we shared
pointers it was impossible to know if we had
already updated the code.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
pbrd/pbr_nht.c
pbrd/pbr_nht.h
pbrd/pbr_vty.c

index 9722300d98f1c22274f1a9a05db1e59c3d7fd387..e615818a107cb473b8247a2e0d520013251d1bf6 100644 (file)
@@ -91,15 +91,15 @@ static void *pbr_nh_alloc(void *p)
        struct nhrc *nhrc;
 
        new = XCALLOC(MTYPE_PBR_NHG, sizeof(*new));
-       nhrc = hash_get(pbr_nhrc_hash, pnhc->nexthop, pbr_nhrc_hash_alloc);
-       new->nexthop = &nhrc->nexthop;
+       nhrc = hash_get(pbr_nhrc_hash, &pnhc->nexthop, pbr_nhrc_hash_alloc);
+       new->nexthop = nhrc->nexthop;
 
        /* Decremented again in pbr_nh_delete */
        ++nhrc->refcount;
 
        DEBUGD(&pbr_dbg_nht, "%s: Sending nexthop to Zebra", __func__);
 
-       pbr_send_rnh(new->nexthop, true);
+       pbr_send_rnh(&new->nexthop, true);
 
        new->valid = false;
        return new;
@@ -109,14 +109,14 @@ static void pbr_nh_delete(struct pbr_nexthop_cache **pnhc)
 {
        struct nhrc *nhrc;
 
-       nhrc = hash_lookup(pbr_nhrc_hash, (*pnhc)->nexthop);
+       nhrc = hash_lookup(pbr_nhrc_hash, &((*pnhc)->nexthop));
 
        if (nhrc)
                --nhrc->refcount;
        if (!nhrc || nhrc->refcount == 0) {
                DEBUGD(&pbr_dbg_nht, "%s: Removing nexthop from Zebra",
                       __func__);
-               pbr_send_rnh((*pnhc)->nexthop, false);
+               pbr_send_rnh(&((*pnhc)->nexthop), false);
        }
        if (nhrc && nhrc->refcount == 0) {
                hash_release(pbr_nhrc_hash, nhrc);
@@ -136,7 +136,7 @@ static uint32_t pbr_nh_hash_key(const void *arg)
        uint32_t key;
        const struct pbr_nexthop_cache *pbrnc = arg;
 
-       key = nexthop_hash(pbrnc->nexthop);
+       key = nexthop_hash(&pbrnc->nexthop);
 
        return key;
 }
@@ -148,28 +148,28 @@ static bool pbr_nh_hash_equal(const void *arg1, const void *arg2)
        const struct pbr_nexthop_cache *pbrnc2 =
                (const struct pbr_nexthop_cache *)arg2;
 
-       if (pbrnc1->nexthop->vrf_id != pbrnc2->nexthop->vrf_id)
+       if (pbrnc1->nexthop.vrf_id != pbrnc2->nexthop.vrf_id)
                return false;
 
-       if (pbrnc1->nexthop->ifindex != pbrnc2->nexthop->ifindex)
+       if (pbrnc1->nexthop.ifindex != pbrnc2->nexthop.ifindex)
                return false;
 
-       if (pbrnc1->nexthop->type != pbrnc2->nexthop->type)
+       if (pbrnc1->nexthop.type != pbrnc2->nexthop.type)
                return false;
 
-       switch (pbrnc1->nexthop->type) {
+       switch (pbrnc1->nexthop.type) {
        case NEXTHOP_TYPE_IFINDEX:
-               return pbrnc1->nexthop->ifindex == pbrnc2->nexthop->ifindex;
+               return pbrnc1->nexthop.ifindex == pbrnc2->nexthop.ifindex;
        case NEXTHOP_TYPE_IPV4_IFINDEX:
        case NEXTHOP_TYPE_IPV4:
-               return pbrnc1->nexthop->gate.ipv4.s_addr
-                      == pbrnc2->nexthop->gate.ipv4.s_addr;
+               return pbrnc1->nexthop.gate.ipv4.s_addr
+                      == pbrnc2->nexthop.gate.ipv4.s_addr;
        case NEXTHOP_TYPE_IPV6_IFINDEX:
        case NEXTHOP_TYPE_IPV6:
-               return !memcmp(&pbrnc1->nexthop->gate.ipv6,
-                              &pbrnc2->nexthop->gate.ipv6, 16);
+               return !memcmp(&pbrnc1->nexthop.gate.ipv6,
+                              &pbrnc2->nexthop.gate.ipv6, 16);
        case NEXTHOP_TYPE_BLACKHOLE:
-               return pbrnc1->nexthop->bh_type == pbrnc2->nexthop->bh_type;
+               return pbrnc1->nexthop.bh_type == pbrnc2->nexthop.bh_type;
        }
 
        /*
@@ -249,9 +249,8 @@ void pbr_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc,
        pnhgc = hash_get(pbr_nhg_hash, &pnhgc_find, pbr_nhgc_alloc);
 
        /* create & insert new pnhc into pnhgc->nhh */
-       pnhc_find.nexthop = (struct nexthop *)nhop;
+       pnhc_find.nexthop = *nhop;
        pnhc = hash_get(pnhgc->nhh, &pnhc_find, pbr_nh_alloc);
-       pnhc_find.nexthop = NULL;
 
        /* set parent pnhgc */
        pnhc->parent = pnhgc;
@@ -291,7 +290,7 @@ void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc,
        pnhgc = hash_lookup(pbr_nhg_hash, &pnhgc_find);
 
        /* delete pnhc from pnhgc->nhh */
-       pnhc_find.nexthop = (struct nexthop *)nhop;
+       pnhc_find.nexthop = *nhop;
        pnhc = hash_release(pnhgc->nhh, &pnhc_find);
 
        /* delete pnhc */
@@ -499,7 +498,7 @@ void pbr_nht_change_group(const char *name)
                struct pbr_nexthop_cache lookup;
                struct pbr_nexthop_cache *pnhc;
 
-               lookup.nexthop = nhop;
+               lookup.nexthop = *nhop;
                pnhc = hash_lookup(pnhgc->nhh, &lookup);
                if (!pnhc) {
                        pnhc = hash_get(pnhgc->nhh, &lookup, pbr_nh_alloc);
@@ -553,7 +552,7 @@ void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms,
 
        pnhgc = hash_get(pbr_nhg_hash, &find, pbr_nhgc_alloc);
 
-       lookup.nexthop = pbrms->nhg->nexthop;
+       lookup.nexthop = *pbrms->nhg->nexthop;
        pnhc = hash_get(pnhgc->nhh, &lookup, pbr_nh_alloc);
        pnhc->parent = pnhgc;
        if (nhop->vrf_id != VRF_DEFAULT) {
@@ -590,7 +589,7 @@ static void pbr_nht_release_individual_nexthop(struct pbr_map_sequence *pbrms)
 
        nh = pbrms->nhg->nexthop;
        nh_type = nh->type;
-       lup.nexthop = nh;
+       lup.nexthop = *nh;
        pnhc = hash_lookup(pnhgc->nhh, &lup);
        pnhc->parent = NULL;
        hash_release(pnhgc->nhh, pnhc);
@@ -641,7 +640,7 @@ struct pbr_nexthop_group_cache *pbr_nht_add_group(const char *name)
                struct pbr_nexthop_cache lookupc;
                struct pbr_nexthop_cache *pnhc;
 
-               lookupc.nexthop = nhop;
+               lookupc.nexthop = *nhop;
                pnhc = hash_lookup(pnhgc->nhh, &lookupc);
                if (!pnhc) {
                        pnhc = hash_get(pnhgc->nhh, &lookupc, pbr_nh_alloc);
@@ -725,12 +724,12 @@ pbr_nht_individual_nexthop_gw_update(struct pbr_nexthop_cache *pnhc,
 
        switch (pnhi->nhr->prefix.family) {
        case AF_INET:
-               if (pnhc->nexthop->gate.ipv4.s_addr
+               if (pnhc->nexthop.gate.ipv4.s_addr
                    != pnhi->nhr->prefix.u.prefix4.s_addr)
                        goto done; /* Unrelated change */
                break;
        case AF_INET6:
-               if (memcmp(&pnhc->nexthop->gate.ipv6,
+               if (memcmp(&pnhc->nexthop.gate.ipv6,
                           &pnhi->nhr->prefix.u.prefix6, 16)
                    != 0)
                        goto done; /* Unrelated change */
@@ -742,8 +741,8 @@ pbr_nht_individual_nexthop_gw_update(struct pbr_nexthop_cache *pnhc,
                goto done;
        }
 
-       if (pnhc->nexthop->type == NEXTHOP_TYPE_IPV4_IFINDEX
-           || pnhc->nexthop->type == NEXTHOP_TYPE_IPV6_IFINDEX) {
+       if (pnhc->nexthop.type == NEXTHOP_TYPE_IPV4_IFINDEX
+           || pnhc->nexthop.type == NEXTHOP_TYPE_IPV6_IFINDEX) {
 
                /* GATEWAY_IFINDEX type shouldn't resolve to group */
                if (pnhi->nhr->nexthop_num > 1) {
@@ -754,7 +753,7 @@ pbr_nht_individual_nexthop_gw_update(struct pbr_nexthop_cache *pnhc,
                /* If whatever we resolved to wasn't on the interface we
                 * specified. (i.e. not a connected route), its invalid.
                 */
-               if (pnhi->nhr->nexthops[0].ifindex != pnhc->nexthop->ifindex) {
+               if (pnhi->nhr->nexthops[0].ifindex != pnhc->nexthop.ifindex) {
                        is_valid = false;
                        goto done;
                }
@@ -776,7 +775,7 @@ static bool pbr_nht_individual_nexthop_interface_update(
        if (!pnhi->ifp) /* It doesn't care about non-interface updates */
                goto done;
 
-       if (pnhc->nexthop->ifindex
+       if (pnhc->nexthop.ifindex
            != pnhi->ifp->ifindex) /* Un-related interface */
                goto done;
 
@@ -800,12 +799,12 @@ pbr_nht_individual_nexthop_update(struct pbr_nexthop_cache *pnhc,
 {
        assert(pnhi->nhr || pnhi->ifp); /* Either nexthop or interface update */
 
-       switch (pnhc->nexthop->type) {
+       switch (pnhc->nexthop.type) {
        case NEXTHOP_TYPE_IFINDEX:
                pbr_nht_individual_nexthop_interface_update(pnhc, pnhi);
                break;
        case NEXTHOP_TYPE_IPV6_IFINDEX:
-               if (IN6_IS_ADDR_LINKLOCAL(&pnhc->nexthop->gate.ipv6)) {
+               if (IN6_IS_ADDR_LINKLOCAL(&pnhc->nexthop.gate.ipv6)) {
                        pbr_nht_individual_nexthop_interface_update(pnhc, pnhi);
                        break;
                }
@@ -848,7 +847,7 @@ static void pbr_nexthop_group_cache_iterate_to_group(struct hash_bucket *b,
        struct nexthop_group *nhg = data;
        struct nexthop *nh = NULL;
 
-       copy_nexthops(&nh, pnhc->nexthop, NULL);
+       copy_nexthops(&nh, &pnhc->nexthop, NULL);
 
        _nexthop_add(&nhg->nexthop, nh);
 }
@@ -926,7 +925,7 @@ static int pbr_nht_individual_nexthop_vrf_handle(struct hash_bucket *b,
        if (pnhc->looked_at == true)
                return HASHWALK_CONTINUE;
 
-       if (pnhc->nexthop->vrf_id == VRF_DEFAULT)
+       if (pnhc->nexthop.vrf_id == VRF_DEFAULT)
                return HASHWALK_CONTINUE;
 
        if (strncmp(pnhc->vrf_name, pbr_vrf_name(pnhi->pbr_vrf),
@@ -934,15 +933,15 @@ static int pbr_nht_individual_nexthop_vrf_handle(struct hash_bucket *b,
            == 0) {
                pnhi->pnhc = pnhc;
 
-               if (pnhc->nexthop->vrf_id != pbr_vrf_id(pnhi->pbr_vrf)) {
+               if (pnhc->nexthop.vrf_id != pbr_vrf_id(pnhi->pbr_vrf)) {
                        struct nhrc_vrf_info nhrcvi;
 
                        memset(&nhrcvi, 0, sizeof(nhrcvi));
                        nhrcvi.pbr_vrf = pnhi->pbr_vrf;
-                       nhrcvi.old_vrf_id = pnhc->nexthop->vrf_id;
+                       nhrcvi.old_vrf_id = pnhc->nexthop.vrf_id;
 
                        pnhi->nhr_matched = true;
-                       pnhi->old_vrf_id = pnhc->nexthop->vrf_id;
+                       pnhi->old_vrf_id = pnhc->nexthop.vrf_id;
 
                        do {
                                nhrcvi.nhrc = NULL;
@@ -995,14 +994,14 @@ static void pbr_nht_nexthop_vrf_handle(struct hash_bucket *b, void *data)
                        continue;
 
                pnhc = pnhi.pnhc;
-               pnhc->nexthop->vrf_id = pnhi.old_vrf_id;
+               pnhc->nexthop.vrf_id = pnhi.old_vrf_id;
                pnhi.pnhc = hash_release(pnhgc->nhh, pnhi.pnhc);
                if (pnhi.pnhc) {
-                       pnhi.pnhc->nexthop->vrf_id = pbr_vrf_id(pbr_vrf);
+                       pnhi.pnhc->nexthop.vrf_id = pbr_vrf_id(pbr_vrf);
 
                        hash_get(pnhgc->nhh, pnhi.pnhc, hash_alloc_intern);
                } else
-                       pnhc->nexthop->vrf_id = pbr_vrf_id(pbr_vrf);
+                       pnhc->nexthop.vrf_id = pbr_vrf_id(pbr_vrf);
 
                pbr_map_check_vrf_nh_group_change(pnhgc->name, pbr_vrf,
                                                  pnhi.old_vrf_id);
@@ -1020,12 +1019,12 @@ static void pbr_nht_individual_nexthop_interface_handle(struct hash_bucket *b,
        struct pbr_nexthop_cache *pnhc = b->data;
        struct pbr_nht_individual *pnhi = data;
 
-       if (pnhc->nexthop->ifindex == 0)
+       if (pnhc->nexthop.ifindex == 0)
                return;
 
        if ((strncmp(pnhc->intf_name, pnhi->ifp->name, sizeof(pnhc->intf_name))
             == 0)
-           && pnhc->nexthop->ifindex != pnhi->ifp->ifindex)
+           && pnhc->nexthop.ifindex != pnhi->ifp->ifindex)
                pnhi->pnhc = pnhc;
 }
 
@@ -1048,15 +1047,15 @@ static void pbr_nht_nexthop_interface_handle(struct hash_bucket *b, void *data)
                        continue;
 
                pnhi.pnhc = hash_release(pnhgc->nhh, pnhi.pnhc);
-               old_ifindex = pnhi.pnhc->nexthop->ifindex;
+               old_ifindex = pnhi.pnhc->nexthop.ifindex;
 
-               nhrc = hash_lookup(pbr_nhrc_hash, pnhi.pnhc->nexthop);
+               nhrc = hash_lookup(pbr_nhrc_hash, &pnhi.pnhc->nexthop);
                if (nhrc) {
                        hash_release(pbr_nhrc_hash, nhrc);
                        nhrc->nexthop.ifindex = ifp->ifindex;
                        hash_get(pbr_nhrc_hash, nhrc, hash_alloc_intern);
                }
-               pnhi.pnhc->nexthop->ifindex = ifp->ifindex;
+               pnhi.pnhc->nexthop.ifindex = ifp->ifindex;
 
                hash_get(pnhgc->nhh, pnhi.pnhc, hash_alloc_intern);
 
@@ -1229,7 +1228,7 @@ static void pbr_nht_show_nhg_nexthops(struct hash_bucket *b, void *data)
        struct vty *vty = data;
 
        vty_out(vty, "\tValid: %d ", pnhc->valid);
-       nexthop_group_write_nexthop(vty, pnhc->nexthop);
+       nexthop_group_write_nexthop(vty, &pnhc->nexthop);
 }
 
 static void pbr_nht_json_nhg_nexthops(struct hash_bucket *b, void *data)
@@ -1239,7 +1238,7 @@ static void pbr_nht_json_nhg_nexthops(struct hash_bucket *b, void *data)
        json_object *this_hop;
 
        this_hop = json_object_new_object();
-       nexthop_group_json_nexthop(this_hop, pnhc->nexthop);
+       nexthop_group_json_nexthop(this_hop, &pnhc->nexthop);
        json_object_boolean_add(this_hop, "valid", pnhc->valid);
 
        json_object_array_add(all_hops, this_hop);
index b58486323330b038e677728b09b89f0097b3bfd0..bcd770c2cfe53c26c36162e643067929d785cae4 100644 (file)
@@ -51,7 +51,7 @@ struct pbr_nexthop_cache {
        char vrf_name[VRF_NAMSIZ + 1];
        char intf_name[INTERFACE_NAMSIZ + 1];
 
-       struct nexthop *nexthop;
+       struct nexthop nexthop;
 
        bool looked_at;
        bool valid;
index cf9185e7c4713161272a98e2a8d03229424dffb1..9f966f617ff4911733d9df781ed23a34750f1584 100644 (file)
@@ -633,13 +633,13 @@ pbrms_nexthop_group_write_individual_nexthop(
        pnhgc = hash_lookup(pbr_nhg_hash, &find);
        assert(pnhgc);
 
-       lookup.nexthop = pbrms->nhg->nexthop;
+       lookup.nexthop = *pbrms->nhg->nexthop;
        pnhc = hash_lookup(pnhgc->nhh, &lookup);
 
        nexthop_group_write_nexthop_simple(
                vty, pbrms->nhg->nexthop,
-               pnhc->nexthop->ifindex != 0 ? pnhc->intf_name : NULL);
-       if (pnhc->nexthop->vrf_id != VRF_DEFAULT)
+               pnhc->nexthop.ifindex != 0 ? pnhc->intf_name : NULL);
+       if (pnhc->nexthop.vrf_id != VRF_DEFAULT)
                vty_out(vty, " nexthop-vrf %s", pnhc->vrf_name);
 
        vty_out(vty, "\n");