From d5f5671fe0521176a52e2e3cc0ca8447c4963aac Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Fri, 16 Mar 2018 17:44:37 -0400 Subject: [PATCH 1/2] pbrd, lib: properly handle nexthop-group changes * Directly handle addition / deletion of nexthop-groups * Directly handle addition / deletion of nexthops for a nexthop-group * Actually delete nexthop-groups Signed-off-by: Quentin Young --- lib/nexthop.c | 2 +- lib/nexthop.h | 2 +- pbrd/pbr_event.c | 46 +------------- pbrd/pbr_nht.c | 155 ++++++++++++++++++++++++++++++++++------------- pbrd/pbr_nht.h | 2 +- 5 files changed, 116 insertions(+), 91 deletions(-) diff --git a/lib/nexthop.c b/lib/nexthop.c index 24e52f78b607..1f2f1019a000 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -241,7 +241,7 @@ void nexthop_del_labels(struct nexthop *nexthop) } } -const char *nexthop2str(struct nexthop *nexthop, char *str, int size) +const char *nexthop2str(const struct nexthop *nexthop, char *str, int size) { switch (nexthop->type) { case NEXTHOP_TYPE_IFINDEX: diff --git a/lib/nexthop.h b/lib/nexthop.h index 9a556fb54c39..07505d7c2693 100644 --- a/lib/nexthop.h +++ b/lib/nexthop.h @@ -143,7 +143,7 @@ extern int nexthop_same_no_recurse(const struct nexthop *next1, extern int nexthop_labels_match(struct nexthop *nh1, struct nexthop *nh2); extern int nexthop_same_firsthop(struct nexthop *next1, struct nexthop *next2); -extern const char *nexthop2str(struct nexthop *nexthop, char *str, int size); +extern const char *nexthop2str(const struct nexthop *nexthop, char *str, int size); extern struct nexthop *nexthop_next(struct nexthop *nexthop); extern unsigned int nexthop_level(struct nexthop *nexthop); #endif /*_LIB_NEXTHOP_H */ diff --git a/pbrd/pbr_event.c b/pbrd/pbr_event.c index bd02175e27df..8e3f6a28c076 100644 --- a/pbrd/pbr_event.c +++ b/pbrd/pbr_event.c @@ -40,26 +40,7 @@ struct work_queue *pbr_event_wq; static const char *pbr_event_wqentry2str(struct pbr_event *pbre, char *buffer, size_t buflen) { - switch (pbre->event) { - case PBR_NHG_NEW: - snprintf(buffer, buflen, "Nexthop Group Added %s", - pbre->name); - break; - case PBR_NHG_ADD_NEXTHOP: - snprintf(buffer, buflen, "Nexthop Group Nexthop Added %s", - pbre->name); - break; - case PBR_NHG_DEL_NEXTHOP: - snprintf(buffer, buflen, "Nexthop Group Nexthop Deleted %s", - pbre->name); - break; - case PBR_NHG_DELETE: - snprintf(buffer, buflen, "Nexthop Group Deleted %s", - pbre->name); - break; - } - - return buffer; + return "Event not found."; } void pbr_event_free(struct pbr_event **pbre) @@ -76,31 +57,6 @@ static void pbr_event_delete_wq(struct work_queue *wq, void *data) static wq_item_status pbr_event_process_wq(struct work_queue *wq, void *data) { - struct pbr_event *pbre = (struct pbr_event *)data; - char buffer[256]; - - DEBUGD(&pbr_dbg_event, "%s: Handling event %s", __PRETTY_FUNCTION__, - pbr_event_wqentry2str(pbre, buffer, sizeof(buffer))); - - switch (pbre->event) { - case PBR_NHG_NEW: - pbr_nht_add_group(pbre->name); - pbr_map_check_nh_group_change(pbre->name); - break; - case PBR_NHG_ADD_NEXTHOP: - pbr_nht_change_group(pbre->name); - pbr_map_check_nh_group_change(pbre->name); - break; - case PBR_NHG_DEL_NEXTHOP: - pbr_nht_change_group(pbre->name); - pbr_map_check_nh_group_change(pbre->name); - break; - case PBR_NHG_DELETE: - pbr_nht_delete_group(pbre->name); - pbr_map_check_nh_group_change(pbre->name); - break; - } - return WQ_SUCCESS; } diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index b675463f294b..43c14cc977c1 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "pbrd/pbr_nht.h" #include "pbrd/pbr_map.h" @@ -47,6 +48,9 @@ static uint32_t pbr_nhg_low_rule; static uint32_t pbr_nhg_high_rule; static bool nhg_tableid[65535]; +static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc, + struct nexthop_group nhg); + /* * Nexthop refcount. */ @@ -119,6 +123,11 @@ static void pbr_nh_delete(struct pbr_nexthop_cache **pnhc) XFREE(MTYPE_PBR_NHG, *pnhc); } +static void pbr_nh_delete_iterate(struct hash_backet *b, void *p) +{ + pbr_nh_delete((struct pbr_nexthop_cache **)&b->data); +} + static uint32_t pbr_nh_hash_key(void *arg) { uint32_t key; @@ -166,50 +175,118 @@ static int pbr_nh_hash_equal(const void *arg1, const void *arg2) return 0; } +static void pbr_nhgc_delete(struct pbr_nexthop_group_cache *p) +{ + hash_iterate(p->nhh, pbr_nh_delete_iterate, NULL); + hash_free(p->nhh); + XFREE(MTYPE_PBR_NHG, p); +} + +static void *pbr_nhgc_alloc(void *p) +{ + struct pbr_nexthop_group_cache *new; + struct pbr_nexthop_group_cache *pnhgc = + (struct pbr_nexthop_group_cache *)p; + + new = XCALLOC(MTYPE_PBR_NHG, sizeof(*new)); + + strcpy(new->name, pnhgc->name); + new->table_id = pbr_nht_get_next_tableid(); + + DEBUGD(&pbr_dbg_nht, "%s: NHT: %s assigned Table ID: %u", + __PRETTY_FUNCTION__, new->name, new->table_id); + + new->nhh = hash_create_size(8, pbr_nh_hash_key, pbr_nh_hash_equal, + "PBR NH Cache Hash"); + return new; +} + + void pbr_nhgroup_add_cb(const char *name) { - struct pbr_event *pbre; + struct pbr_nexthop_group_cache *pnhgc; + struct nexthop_group_cmd *nhgc; - pbre = pbr_event_new(PBR_NHG_NEW, name); + nhgc = nhgc_find(name); + pnhgc = pbr_nht_add_group(name); - pbr_event_enqueue(pbre); - DEBUGD(&pbr_dbg_nht, "%s: Received ADD cb for %s", __PRETTY_FUNCTION__, + DEBUGD(&pbr_dbg_nht, "%s: Added nexthop-group %s", __PRETTY_FUNCTION__, name); + + pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg); + pbr_map_check_nh_group_change(name); } -void pbr_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhg, +void pbr_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc, const struct nexthop *nhop) { - struct pbr_event *pbre; + char debugstr[256]; + struct pbr_nexthop_group_cache pnhgc_find = {}; + struct pbr_nexthop_group_cache *pnhgc; + struct pbr_nexthop_cache pnhc_find = {}; + struct pbr_nexthop_cache *pnhc; + + /* find pnhgc by name */ + strlcpy(pnhgc_find.name, nhgc->name, sizeof(pnhgc_find.name)); + 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 = hash_get(pnhgc->nhh, &pnhc_find, pbr_nh_alloc); + pnhc_find.nexthop = NULL; - pbre = pbr_event_new(PBR_NHG_ADD_NEXTHOP, nhg->name); + /* set parent pnhgc */ + pnhc->parent = pnhgc; + + if (DEBUG_MODE_CHECK(&pbr_dbg_nht, DEBUG_MODE_ALL)) { + nexthop2str(nhop, debugstr, sizeof(debugstr)); + DEBUGD(&pbr_dbg_nht, "%s: Added %s to nexthop-group %s", + __PRETTY_FUNCTION__, debugstr, nhgc->name); + } - pbr_event_enqueue(pbre); - DEBUGD(&pbr_dbg_nht, "%s: Received NEXTHOP_ADD cb for %s", - __PRETTY_FUNCTION__, nhg->name); + pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg); + pbr_map_check_nh_group_change(nhgc->name); } -void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhg, +void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, const struct nexthop *nhop) { - struct pbr_event *pbre; + char debugstr[256]; + struct pbr_nexthop_group_cache pnhgc_find = {}; + struct pbr_nexthop_group_cache *pnhgc; + struct pbr_nexthop_cache pnhc_find = {}; + struct pbr_nexthop_cache *pnhc; + + /* find pnhgc by name */ + strlcpy(pnhgc_find.name, nhgc->name, sizeof(pnhgc_find.name)); + pnhgc = hash_get(pbr_nhg_hash, &pnhgc_find, pbr_nhgc_alloc); - pbre = pbr_event_new(PBR_NHG_DEL_NEXTHOP, nhg->name); + /* delete pnhc from pnhgc->nhh */ + pnhc_find.nexthop = (struct nexthop *)nhop; + pnhc = hash_release(pnhgc->nhh, &pnhc_find); - pbr_event_enqueue(pbre); - DEBUGD(&pbr_dbg_nht, "%s: Received NEXTHOP_DEL cb for %s", - __PRETTY_FUNCTION__, nhg->name); + /* delete pnhc */ + pbr_nh_delete(&pnhc); + + if (DEBUG_MODE_CHECK(&pbr_dbg_nht, DEBUG_MODE_ALL)) { + nexthop2str(nhop, debugstr, sizeof(debugstr)); + DEBUGD(&pbr_dbg_nht, "%s: Removed %s from nexthop-group %s", + __PRETTY_FUNCTION__, debugstr, nhgc->name); + } + + pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg); + pbr_map_check_nh_group_change(nhgc->name); } void pbr_nhgroup_delete_cb(const char *name) { - struct pbr_event *pbre; - - pbre = pbr_event_new(PBR_NHG_DELETE, name); + /* delete group from all pbrms's */ + pbr_nht_delete_group(name); - pbr_event_enqueue(pbre); - DEBUGD(&pbr_dbg_nht, "%s: Received DELETE cb for %s", + DEBUGD(&pbr_dbg_nht, "%s: Removed nexthop-group %s", __PRETTY_FUNCTION__, name); + + pbr_map_check_nh_group_change(name); } #if 0 @@ -371,25 +448,6 @@ char *pbr_nht_nexthop_make_name(char *name, uint32_t seqno, char *buffer) return buffer; } -static void *pbr_nhgc_alloc(void *p) -{ - struct pbr_nexthop_group_cache *new; - struct pbr_nexthop_group_cache *pnhgc = - (struct pbr_nexthop_group_cache *)p; - - new = XCALLOC(MTYPE_PBR_NHG, sizeof(*new)); - - strcpy(new->name, pnhgc->name); - new->table_id = pbr_nht_get_next_tableid(); - - DEBUGD(&pbr_dbg_nht, "%s: NHT: %s assigned Table ID: %u", - __PRETTY_FUNCTION__, new->name, new->table_id); - - new->nhh = hash_create_size(8, pbr_nh_hash_key, pbr_nh_hash_equal, - "PBR NH Cache Hash"); - return new; -} - void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms) { struct pbr_nexthop_group_cache *pnhgc; @@ -438,7 +496,7 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms) XFREE(MTYPE_TMP, pbrms->internal_nhg_name); } -void pbr_nht_add_group(const char *name) +struct pbr_nexthop_group_cache *pbr_nht_add_group(const char *name) { struct nexthop *nhop; struct nexthop_group_cmd *nhgc; @@ -450,7 +508,7 @@ void pbr_nht_add_group(const char *name) if (!nhgc) { zlog_warn("%s: Could not find group %s to add", __PRETTY_FUNCTION__, name); - return; + return NULL; } strcpy(lookup.name, name); @@ -469,6 +527,8 @@ void pbr_nht_add_group(const char *name) pnhc->parent = pnhgc; } } + + return pnhgc; } void pbr_nht_delete_group(const char *name) @@ -476,16 +536,25 @@ void pbr_nht_delete_group(const char *name) struct pbr_map_sequence *pbrms; struct listnode *snode; struct pbr_map *pbrm; + struct pbr_nexthop_group_cache pnhgc_find; + struct pbr_nexthop_group_cache *pnhgc; RB_FOREACH (pbrm, pbr_map_entry_head, &pbr_maps) { for (ALL_LIST_ELEMENTS_RO(pbrm->seqnumbers, snode, pbrms)) { if (pbrms->nhgrp_name - && strcmp(pbrms->nhgrp_name, name) == 0) { + && strmatch(pbrms->nhgrp_name, name)) { pbrms->reason |= PBR_MAP_INVALID_NO_NEXTHOPS; + nexthop_group_delete(&pbrms->nhg); + pbrms->nhg = NULL; + pbrms->internal_nhg_name = NULL; pbrm->valid = false; } } } + + strlcpy(pnhgc_find.name, name, sizeof(pnhgc_find.name)); + pnhgc = hash_release(pbr_nhg_hash, &pnhgc_find); + pbr_nhgc_delete(pnhgc); } bool pbr_nht_nexthop_valid(struct nexthop_group *nhg) diff --git a/pbrd/pbr_nht.h b/pbrd/pbr_nht.h index 407e7393355c..870660b060a0 100644 --- a/pbrd/pbr_nht.h +++ b/pbrd/pbr_nht.h @@ -80,7 +80,7 @@ extern void pbr_nhgroup_delete_cb(const char *name); extern bool pbr_nht_nexthop_valid(struct nexthop_group *nhg); extern bool pbr_nht_nexthop_group_valid(const char *name); -extern void pbr_nht_add_group(const char *name); +extern struct pbr_nexthop_group_cache *pbr_nht_add_group(const char *name); extern void pbr_nht_change_group(const char *name); extern void pbr_nht_delete_group(const char *name); From c2754ca5208f645e5af178a4f095edbbad3a2240 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Fri, 16 Mar 2018 17:49:00 -0400 Subject: [PATCH 2/2] pbrd: remove pbr_event.[ch] No longer needed. Signed-off-by: Quentin Young --- pbrd/pbr_event.c | 91 ------------------------------------------------ pbrd/pbr_event.h | 74 --------------------------------------- pbrd/pbr_main.c | 3 -- pbrd/pbr_nht.c | 1 - pbrd/subdir.am | 2 -- 5 files changed, 171 deletions(-) delete mode 100644 pbrd/pbr_event.c delete mode 100644 pbrd/pbr_event.h diff --git a/pbrd/pbr_event.c b/pbrd/pbr_event.c deleted file mode 100644 index 8e3f6a28c076..000000000000 --- a/pbrd/pbr_event.c +++ /dev/null @@ -1,91 +0,0 @@ -/* - * PBR-event Code - * Copyright (C) 2018 Cumulus Networks, Inc. - * Donald Sharp - * - * This file is part of FRR. - * - * FRR is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the - * Free Software Foundation; either version 2, or (at your option) any - * later version. - * - * FRR is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; see the file COPYING; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA - */ -#include - -#include -#include -#include -#include -#include - -#include "pbrd/pbr_event.h" -#include "pbrd/pbr_map.h" -#include "pbrd/pbr_nht.h" -#include "pbrd/pbr_memory.h" -#include "pbrd/pbr_debug.h" - -DEFINE_MTYPE_STATIC(PBRD, PBR_EVENT, "Event WorkQueue") - -struct work_queue *pbr_event_wq; - -static const char *pbr_event_wqentry2str(struct pbr_event *pbre, - char *buffer, size_t buflen) -{ - return "Event not found."; -} - -void pbr_event_free(struct pbr_event **pbre) -{ - XFREE(MTYPE_PBR_EVENT, *pbre); -} - -static void pbr_event_delete_wq(struct work_queue *wq, void *data) -{ - struct pbr_event *pbre = (struct pbr_event *)data; - - XFREE(MTYPE_PBR_EVENT, pbre); -} - -static wq_item_status pbr_event_process_wq(struct work_queue *wq, void *data) -{ - return WQ_SUCCESS; -} - -void pbr_event_enqueue(struct pbr_event *pbre) -{ - work_queue_add(pbr_event_wq, pbre); -} - -struct pbr_event *pbr_event_new(enum pbr_events ev, const char *name) -{ - struct pbr_event *event; - - event = XCALLOC(MTYPE_PBR_EVENT, sizeof(struct pbr_event)); - event->event = ev; - if (name) - strlcpy(event->name, name, sizeof(event->name)); - return event; -} - -extern struct thread_master *master; - -void pbr_event_init(void) -{ - pbr_event_wq = work_queue_new(master, "PBR Main Work Queue"); - pbr_event_wq->spec.workfunc = &pbr_event_process_wq; - pbr_event_wq->spec.del_item_data = &pbr_event_delete_wq; -} - -void pbr_event_stop(void) -{ - work_queue_free_and_null(&pbr_event_wq); -} diff --git a/pbrd/pbr_event.h b/pbrd/pbr_event.h deleted file mode 100644 index 5fce00e9add5..000000000000 --- a/pbrd/pbr_event.h +++ /dev/null @@ -1,74 +0,0 @@ -/* - * PBR-event Header - * Copyright (C) 2018 Cumulus Networks, Inc. - * Donald Sharp - * - * This file is part of FRR. - * - * FRR is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the - * Free Software Foundation; either version 2, or (at your option) any - * later version. - * - * FRR is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; see the file COPYING; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA - */ -#ifndef __PBR_EVENT_H__ -#define __PBR_EVENT_H__ - -enum pbr_events { - /* - * A NHG has been added to the system, handle it - */ - PBR_NHG_NEW, - - /* - * A NHG has been modified( added a new nexthop ) - */ - PBR_NHG_ADD_NEXTHOP, - - /* - * A NHG has been modified( deleted a nexthop ) - */ - PBR_NHG_DEL_NEXTHOP, - - /* - * A NHG has been deleted from the system - */ - PBR_NHG_DELETE, - -}; - -struct pbr_event { - enum pbr_events event; - - char name[100]; - union g_addr addr; - uint32_t seqno; -}; - -/* - * Return a event structure that can be filled in and enqueued. - * Assume this memory is owned by the event subsystem. - */ -extern struct pbr_event *pbr_event_new(enum pbr_events ev, const char *name); - -/* - * Free the associated pbr_event item - */ -extern void pbr_event_free(struct pbr_event **pbre); - -/* - * Enqueue an event for later processing - */ -void pbr_event_enqueue(struct pbr_event *pbre); - -extern void pbr_event_init(void); -extern void pbr_event_stop(void); -#endif diff --git a/pbrd/pbr_main.c b/pbrd/pbr_main.c index e9b7fb7ac3d2..44d19660c4ee 100644 --- a/pbrd/pbr_main.c +++ b/pbrd/pbr_main.c @@ -48,7 +48,6 @@ #include "pbr_nht.h" #include "pbr_map.h" #include "pbr_zebra.h" -#include "pbr_event.h" #include "pbr_vty.h" #include "pbr_debug.h" @@ -84,7 +83,6 @@ static void sigint(void) { zlog_notice("Terminating on signal"); - pbr_event_stop(); exit(0); } @@ -156,7 +154,6 @@ int main(int argc, char **argv, char **envp) pbr_nhgroup_del_nexthop_cb, pbr_nhgroup_delete_cb); - pbr_event_init(); pbr_nht_init(); pbr_map_init(); pbr_zebra_init(); diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index 43c14cc977c1..9aaefdc31f53 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -32,7 +32,6 @@ #include "pbrd/pbr_nht.h" #include "pbrd/pbr_map.h" -#include "pbrd/pbr_event.h" #include "pbrd/pbr_zebra.h" #include "pbrd/pbr_memory.h" #include "pbrd/pbr_debug.h" diff --git a/pbrd/subdir.am b/pbrd/subdir.am index 361e6c1fdeb1..42ab393218ff 100644 --- a/pbrd/subdir.am +++ b/pbrd/subdir.am @@ -14,12 +14,10 @@ pbrd_libpbr_a_SOURCES = \ pbrd/pbr_map.c \ pbrd/pbr_memory.c \ pbrd/pbr_nht.c \ - pbrd/pbr_event.c \ pbrd/pbr_debug.c \ # end noinst_HEADERS += \ - pbrd/pbr_event.h \ pbrd/pbr_map.h \ pbrd/pbr_memory.h \ pbrd/pbr_nht.h \