diff options
30 files changed, 996 insertions, 199 deletions
diff --git a/bfdd/bfd.c b/bfdd/bfd.c index a1fb67d357..4367f253e1 100644 --- a/bfdd/bfd.c +++ b/bfdd/bfd.c @@ -348,7 +348,7 @@ int bfd_session_enable(struct bfd_session *bs) /* Sanity check: don't leak open sockets. */ if (bs->sock != -1) { if (bglobal.debug_peer_event) - zlog_debug("session-enable: previous socket open"); + zlog_debug("%s: previous socket open", __func__); close(bs->sock); bs->sock = -1; @@ -952,7 +952,7 @@ int ptm_bfd_sess_del(struct bfd_peer_cfg *bpc) } if (bglobal.debug_peer_event) - zlog_debug("session-delete: %s", bs_to_string(bs)); + zlog_debug("%s: %s", __func__, bs_to_string(bs)); control_notify_config(BCM_NOTIFY_CONFIG_DELETE, bs); diff --git a/bfdd/bfd_packet.c b/bfdd/bfd_packet.c index 98411a8732..382f78a0f6 100644 --- a/bfdd/bfd_packet.c +++ b/bfdd/bfd_packet.c @@ -578,8 +578,8 @@ ssize_t bfd_recv_ipv4(int sd, uint8_t *msgbuf, size_t msgbuflen, uint8_t *ttl, memcpy(&ttlval, CMSG_DATA(cm), sizeof(ttlval)); if (ttlval > 255) { if (bglobal.debug_network) - zlog_debug("ipv4-recv: invalid TTL: %u", - ttlval); + zlog_debug("%s: invalid TTL: %u", + __func__, ttlval); return -1; } *ttl = ttlval; @@ -686,8 +686,8 @@ ssize_t bfd_recv_ipv6(int sd, uint8_t *msgbuf, size_t msgbuflen, uint8_t *ttl, memcpy(&ttlval, CMSG_DATA(cm), sizeof(ttlval)); if (ttlval > 255) { if (bglobal.debug_network) - zlog_debug("ipv6-recv: invalid TTL: %u", - ttlval); + zlog_debug("%s: invalid TTL: %u", + __func__, ttlval); return -1; } @@ -1127,13 +1127,13 @@ int bp_udp_send_fp(int sd, uint8_t *data, size_t datalen, if (wlen <= 0) { if (bglobal.debug_network) - zlog_debug("udp-send: loopback failure: (%d) %s", errno, - strerror(errno)); + zlog_debug("%s: loopback failure: (%d) %s", __func__, + errno, strerror(errno)); return -1; } else if (wlen < (ssize_t)datalen) { if (bglobal.debug_network) - zlog_debug("udp-send: partial send: %zd expected %zu", - wlen, datalen); + zlog_debug("%s: partial send: %zd expected %zu", + __func__, wlen, datalen); return -1; } @@ -1194,13 +1194,13 @@ int bp_udp_send(int sd, uint8_t ttl, uint8_t *data, size_t datalen, wlen = sendmsg(sd, &msg, 0); if (wlen <= 0) { if (bglobal.debug_network) - zlog_debug("udp-send: loopback failure: (%d) %s", errno, - strerror(errno)); + zlog_debug("%s: loopback failure: (%d) %s", __func__, + errno, strerror(errno)); return -1; } else if (wlen < (ssize_t)datalen) { if (bglobal.debug_network) - zlog_debug("udp-send: partial send: %zd expected %zu", - wlen, datalen); + zlog_debug("%s: partial send: %zd expected %zu", + __func__, wlen, datalen); return -1; } @@ -1221,7 +1221,7 @@ int bp_set_ttl(int sd, uint8_t value) int ttl = value; if (setsockopt(sd, IPPROTO_IP, IP_TTL, &ttl, sizeof(ttl)) == -1) { - zlog_warn("set-ttl: setsockopt(IP_TTL, %d): %s", value, + zlog_warn("%s: setsockopt(IP_TTL, %d): %s", __func__, value, strerror(errno)); return -1; } @@ -1234,7 +1234,7 @@ int bp_set_tos(int sd, uint8_t value) int tos = value; if (setsockopt(sd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos)) == -1) { - zlog_warn("set-tos: setsockopt(IP_TOS, %d): %s", value, + zlog_warn("%s: setsockopt(IP_TOS, %d): %s", __func__, value, strerror(errno)); return -1; } @@ -1247,8 +1247,8 @@ static bool bp_set_reuse_addr(int sd) int one = 1; if (setsockopt(sd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) == -1) { - zlog_warn("set-reuse-addr: setsockopt(SO_REUSEADDR, %d): %s", - one, strerror(errno)); + zlog_warn("%s: setsockopt(SO_REUSEADDR, %d): %s", __func__, one, + strerror(errno)); return false; } return true; @@ -1259,8 +1259,8 @@ static bool bp_set_reuse_port(int sd) int one = 1; if (setsockopt(sd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)) == -1) { - zlog_warn("set-reuse-port: setsockopt(SO_REUSEPORT, %d): %s", - one, strerror(errno)); + zlog_warn("%s: setsockopt(SO_REUSEPORT, %d): %s", __func__, one, + strerror(errno)); return false; } return true; diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index dc15d9c695..58f5e9a226 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -3631,8 +3631,10 @@ static int update_advertise_vni_routes(struct bgp *bgp, struct bgpevpn *vpn) pi->type == ZEBRA_ROUTE_BGP && pi->sub_type == BGP_ROUTE_STATIC) break; - if (!pi) /* unexpected */ + if (!pi) { + bgp_dest_unlock_node(dest); return 0; + } attr = pi->attr; global_dest = bgp_global_evpn_node_get(bgp->rib[afi][safi], diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 1164546df7..a8accc25f5 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -300,11 +300,6 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) peer->afc_recv[afi][safi] = from_peer->afc_recv[afi][safi]; peer->orf_plist[afi][safi] = from_peer->orf_plist[afi][safi]; peer->llgr[afi][safi] = from_peer->llgr[afi][safi]; - if (from_peer->soo[afi][safi]) { - ecommunity_free(&peer->soo[afi][safi]); - peer->soo[afi][safi] = - ecommunity_dup(from_peer->soo[afi][safi]); - } } if (bgp_getsockname(peer) < 0) { diff --git a/bgpd/bgp_labelpool.c b/bgpd/bgp_labelpool.c index fa1dcf33e0..c227a5e41c 100644 --- a/bgpd/bgp_labelpool.c +++ b/bgpd/bgp_labelpool.c @@ -37,18 +37,41 @@ #include "bgpd/bgp_errors.h" #include "bgpd/bgp_route.h" +#define BGP_LABELPOOL_ENABLE_TESTS 0 + +#ifndef VTYSH_EXTRACT_PL +#include "bgpd/bgp_labelpool_clippy.c" +#endif + + /* * Definitions and external declarations. */ extern struct zclient *zclient; +#if BGP_LABELPOOL_ENABLE_TESTS +static void lptest_init(void); +static void lptest_finish(void); +#endif + /* * Remember where pool data are kept */ static struct labelpool *lp; -/* request this many labels at a time from zebra */ -#define LP_CHUNK_SIZE 50 +/* + * Number of labels requested at a time from the zebra label manager. + * We start small but double the request size each time up to a + * maximum size. + * + * The label space is 20 bits which is shared with other FRR processes + * on this host, so to avoid greedily requesting a mostly wasted chunk, + * we limit the chunk size to 1/16 of the label space (that's the -4 bits + * in the definition below). This limit slightly increases our cost of + * finding free labels in our allocated chunks. + */ +#define LP_CHUNK_SIZE_MIN 128 +#define LP_CHUNK_SIZE_MAX (1 << (20 - 4)) DEFINE_MTYPE_STATIC(BGPD, BGP_LABEL_CHUNK, "BGP Label Chunk"); DEFINE_MTYPE_STATIC(BGPD, BGP_LABEL_FIFO, "BGP Label FIFO item"); @@ -58,6 +81,9 @@ DEFINE_MTYPE_STATIC(BGPD, BGP_LABEL_CBQ, "BGP Dynamic Label Callback"); struct lp_chunk { uint32_t first; uint32_t last; + uint32_t nfree; /* un-allocated count */ + uint32_t idx_last_allocated; /* start looking here */ + bitfield_t allocated_map; }; /* @@ -160,6 +186,9 @@ static void lp_lcb_free(void *goner) static void lp_chunk_free(void *goner) { + struct lp_chunk *chunk = (struct lp_chunk *)goner; + + bf_free(chunk->allocated_map); XFREE(MTYPE_BGP_LABEL_CHUNK, goner); } @@ -180,6 +209,12 @@ void bgp_lp_init(struct thread_master *master, struct labelpool *pool) lp->callback_q->spec.workfunc = lp_cbq_docallback; lp->callback_q->spec.del_item_data = lp_cbq_item_free; lp->callback_q->spec.max_retries = 0; + + lp->next_chunksize = LP_CHUNK_SIZE_MIN; + +#if BGP_LABELPOOL_ENABLE_TESTS + lptest_init(); +#endif } /* check if a label callback was for a BGP LU node, and if so, unlock it */ @@ -201,6 +236,9 @@ void bgp_lp_finish(void) struct lp_fifo *lf; struct work_queue_item *item, *titem; +#if BGP_LABELPOOL_ENABLE_TESTS + lptest_finish(); +#endif if (!lp) return; @@ -240,25 +278,53 @@ static mpls_label_t get_label_from_pool(void *labelid) /* * Find a free label - * Linear search is not efficient but should be executed infrequently. */ for (ALL_LIST_ELEMENTS_RO(lp->chunks, node, chunk)) { uintptr_t lbl; + unsigned int index; if (debug) zlog_debug("%s: chunk first=%u last=%u", __func__, chunk->first, chunk->last); - for (lbl = chunk->first; lbl <= chunk->last; ++lbl) { - /* labelid is key to all-request "ledger" list */ - if (!skiplist_insert(lp->inuse, (void *)lbl, labelid)) { - /* - * Success - */ - return lbl; - } + /* + * don't look in chunks with no available labels + */ + if (!chunk->nfree) + continue; + + /* + * roll through bitfield starting where we stopped + * last time + */ + index = bf_find_next_clear_bit_wrap( + &chunk->allocated_map, chunk->idx_last_allocated + 1, + 0); + + /* + * since chunk->nfree is non-zero, we should always get + * a valid index + */ + assert(index != WORD_MAX); + + lbl = chunk->first + index; + if (skiplist_insert(lp->inuse, (void *)lbl, labelid)) { + /* something is very wrong */ + zlog_err("%s: unable to insert inuse label %u (id %p)", + __func__, (uint32_t)lbl, labelid); + return MPLS_LABEL_NONE; } + + /* + * Success + */ + bf_set_bit(chunk->allocated_map, index); + chunk->idx_last_allocated = index; + chunk->nfree -= 1; + + return lbl; } + return MPLS_LABEL_NONE; } @@ -299,10 +365,14 @@ static struct lp_lcb *lcb_alloc( * When connection to zebra is reestablished, previous label assignments * will be invalidated (via callbacks having the "allocated" parameter unset) * and new labels will be automatically reassigned by this labelpool module - * (that is, a requestor does not need to call lp_get() again if it is + * (that is, a requestor does not need to call bgp_lp_get() again if it is * notified via callback that its label has been lost: it will eventually * get another callback with a new label assignment). * + * The callback function should return 0 to accept the allocation + * and non-zero to refuse it. The callback function return value is + * ignored for invalidations (i.e., when the "allocated" parameter is false) + * * Prior requests for a given labelid are detected so that requests and * assignments are not duplicated. */ @@ -392,10 +462,13 @@ void bgp_lp_get( if (lp_fifo_count(&lp->requests) > lp->pending_count) { if (!zclient || zclient->sock < 0) return; - if (zclient_send_get_label_chunk(zclient, 0, LP_CHUNK_SIZE, - MPLS_LABEL_BASE_ANY) - != ZCLIENT_SEND_FAILURE) - lp->pending_count += LP_CHUNK_SIZE; + if (zclient_send_get_label_chunk(zclient, 0, lp->next_chunksize, + MPLS_LABEL_BASE_ANY) != + ZCLIENT_SEND_FAILURE) { + lp->pending_count += lp->next_chunksize; + if ((lp->next_chunksize << 1) <= LP_CHUNK_SIZE_MAX) + lp->next_chunksize <<= 1; + } } } @@ -408,13 +481,36 @@ void bgp_lp_release( if (!skiplist_search(lp->ledger, labelid, (void **)&lcb)) { if (label == lcb->label && type == lcb->type) { + struct listnode *node; + struct lp_chunk *chunk; uintptr_t lbl = label; + bool deallocated = false; /* no longer in use */ skiplist_delete(lp->inuse, (void *)lbl, NULL); /* no longer requested */ skiplist_delete(lp->ledger, labelid, NULL); + + /* + * Find the chunk this label belongs to and + * deallocate the label + */ + for (ALL_LIST_ELEMENTS_RO(lp->chunks, node, chunk)) { + uint32_t index; + + if ((label < chunk->first) || + (label > chunk->last)) + continue; + + index = label - chunk->first; + assert(bf_test_index(chunk->allocated_map, + index)); + bf_release_index(chunk->allocated_map, index); + chunk->nfree += 1; + deallocated = true; + } + assert(deallocated); } } } @@ -427,6 +523,7 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last) struct lp_chunk *chunk; int debug = BGP_DEBUG(labelpool, LABELPOOL); struct lp_fifo *lf; + uint32_t labelcount; if (last < first) { flog_err(EC_BGP_LABEL, @@ -437,19 +534,27 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last) chunk = XCALLOC(MTYPE_BGP_LABEL_CHUNK, sizeof(struct lp_chunk)); + labelcount = last - first + 1; + chunk->first = first; chunk->last = last; + chunk->nfree = labelcount; + bf_init(chunk->allocated_map, labelcount); - listnode_add(lp->chunks, chunk); + /* + * Optimize for allocation by adding the new (presumably larger) + * chunk at the head of the list so it is examined first. + */ + listnode_add_head(lp->chunks, chunk); - lp->pending_count -= (last - first + 1); + lp->pending_count -= labelcount; if (debug) { zlog_debug("%s: %zu pending requests", __func__, lp_fifo_count(&lp->requests)); } - while ((lf = lp_fifo_first(&lp->requests))) { + while (labelcount && (lf = lp_fifo_first(&lp->requests))) { struct lp_lcb *lcb; void *labelid = lf->lcb.labelid; @@ -495,6 +600,8 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last) break; } + labelcount -= 1; + /* * we filled the request from local pool. * Enqueue response work item with new label. @@ -535,8 +642,8 @@ void bgp_lp_event_zebra_down(void) */ void bgp_lp_event_zebra_up(void) { - int labels_needed; - int chunks_needed; + unsigned int labels_needed; + unsigned int chunks_needed; void *labelid; struct lp_lcb *lcb; int lm_init_ok; @@ -548,9 +655,16 @@ void bgp_lp_event_zebra_up(void) labels_needed = lp_fifo_count(&lp->requests) + skiplist_count(lp->inuse); + if (labels_needed > lp->next_chunksize) { + while ((lp->next_chunksize < labels_needed) && + (lp->next_chunksize << 1 <= LP_CHUNK_SIZE_MAX)) + + lp->next_chunksize <<= 1; + } + /* round up */ - chunks_needed = (labels_needed / LP_CHUNK_SIZE) + 1; - labels_needed = chunks_needed * LP_CHUNK_SIZE; + chunks_needed = (labels_needed / lp->next_chunksize) + 1; + labels_needed = chunks_needed * lp->next_chunksize; lm_init_ok = lm_label_manager_connect(zclient, 1) == 0; @@ -937,25 +1051,496 @@ DEFUN(show_bgp_labelpool_chunks, show_bgp_labelpool_chunks_cmd, } json = json_object_new_array(); } else { - vty_out(vty, "First Last\n"); - vty_out(vty, "--------------\n"); + vty_out(vty, "%10s %10s %10s %10s\n", "First", "Last", "Size", + "nfree"); + vty_out(vty, "-------------------------------------------\n"); } for (ALL_LIST_ELEMENTS_RO(lp->chunks, node, chunk)) { + uint32_t size; + + size = chunk->last - chunk->first + 1; + if (uj) { json_elem = json_object_new_object(); json_object_array_add(json, json_elem); json_object_int_add(json_elem, "first", chunk->first); json_object_int_add(json_elem, "last", chunk->last); + json_object_int_add(json_elem, "size", size); + json_object_int_add(json_elem, "numberFree", + chunk->nfree); } else - vty_out(vty, "%-10u %-10u\n", chunk->first, - chunk->last); + vty_out(vty, "%10u %10u %10u %10u\n", chunk->first, + chunk->last, size, chunk->nfree); } if (uj) vty_json(vty, json); return CMD_SUCCESS; } +#if BGP_LABELPOOL_ENABLE_TESTS +/*------------------------------------------------------------------------ + * Testing code start + *------------------------------------------------------------------------*/ + +DEFINE_MTYPE_STATIC(BGPD, LABELPOOL_TEST, "Label pool test"); + +#define LPT_STAT_INSERT_FAIL 0 +#define LPT_STAT_DELETE_FAIL 1 +#define LPT_STAT_ALLOCATED 2 +#define LPT_STAT_DEALLOCATED 3 +#define LPT_STAT_MAX 4 + +const char *lpt_counter_names[] = { + "sl insert failures", + "sl delete failures", + "labels allocated", + "labels deallocated", +}; + +static uint8_t lpt_generation; +static bool lpt_inprogress; +static struct skiplist *lp_tests; +static unsigned int lpt_test_cb_tcb_lookup_fails; +static unsigned int lpt_release_tcb_lookup_fails; +static unsigned int lpt_test_event_tcb_lookup_fails; +static unsigned int lpt_stop_tcb_lookup_fails; + +struct lp_test { + uint8_t generation; + unsigned int request_maximum; + unsigned int request_blocksize; + uintptr_t request_count; /* match type of labelid */ + int label_type; + struct skiplist *labels; + struct timeval starttime; + struct skiplist *timestamps_alloc; + struct skiplist *timestamps_dealloc; + struct thread *event_thread; + unsigned int counter[LPT_STAT_MAX]; +}; + +/* test parameters */ +#define LPT_MAX_COUNT 500000 /* get this many labels in all */ +#define LPT_BLKSIZE 10000 /* this many at a time, then yield */ +#define LPT_TS_INTERVAL 10000 /* timestamp every this many labels */ + + +static int test_cb(mpls_label_t label, void *labelid, bool allocated) +{ + uintptr_t generation; + struct lp_test *tcb; + + generation = ((uintptr_t)labelid >> 24) & 0xff; + + if (skiplist_search(lp_tests, (void *)generation, (void **)&tcb)) { + + /* couldn't find current test in progress */ + ++lpt_test_cb_tcb_lookup_fails; + return -1; /* reject allocation */ + } + + if (allocated) { + ++tcb->counter[LPT_STAT_ALLOCATED]; + if (!(tcb->counter[LPT_STAT_ALLOCATED] % LPT_TS_INTERVAL)) { + uintptr_t time_ms; + + time_ms = monotime_since(&tcb->starttime, NULL) / 1000; + skiplist_insert(tcb->timestamps_alloc, + (void *)(uintptr_t)tcb + ->counter[LPT_STAT_ALLOCATED], + (void *)time_ms); + } + if (skiplist_insert(tcb->labels, labelid, + (void *)(uintptr_t)label)) { + ++tcb->counter[LPT_STAT_INSERT_FAIL]; + return -1; + } + } else { + ++tcb->counter[LPT_STAT_DEALLOCATED]; + if (!(tcb->counter[LPT_STAT_DEALLOCATED] % LPT_TS_INTERVAL)) { + uintptr_t time_ms; + + time_ms = monotime_since(&tcb->starttime, NULL) / 1000; + skiplist_insert(tcb->timestamps_dealloc, + (void *)(uintptr_t)tcb + ->counter[LPT_STAT_ALLOCATED], + (void *)time_ms); + } + if (skiplist_delete(tcb->labels, labelid, 0)) { + ++tcb->counter[LPT_STAT_DELETE_FAIL]; + return -1; + } + } + return 0; +} + +static void labelpool_test_event_handler(struct thread *thread) +{ + struct lp_test *tcb; + + if (skiplist_search(lp_tests, (void *)(uintptr_t)(lpt_generation), + (void **)&tcb)) { + + /* couldn't find current test in progress */ + ++lpt_test_event_tcb_lookup_fails; + return; + } + + /* + * request a bunch of labels + */ + for (unsigned int i = 0; (i < tcb->request_blocksize) && + (tcb->request_count < tcb->request_maximum); + ++i) { + + uintptr_t id; + + ++tcb->request_count; + + /* + * construct 32-bit id from request_count and generation + */ + id = ((uintptr_t)tcb->generation << 24) | + (tcb->request_count & 0x00ffffff); + bgp_lp_get(LP_TYPE_VRF, (void *)id, test_cb); + } + + if (tcb->request_count < tcb->request_maximum) + thread_add_event(bm->master, labelpool_test_event_handler, NULL, + 0, &tcb->event_thread); +} + +static void lptest_stop(void) +{ + struct lp_test *tcb; + + if (!lpt_inprogress) + return; + + if (skiplist_search(lp_tests, (void *)(uintptr_t)(lpt_generation), + (void **)&tcb)) { + + /* couldn't find current test in progress */ + ++lpt_stop_tcb_lookup_fails; + return; + } + + if (tcb->event_thread) + thread_cancel(&tcb->event_thread); + + lpt_inprogress = false; +} + +static int lptest_start(struct vty *vty) +{ + struct lp_test *tcb; + + if (lpt_inprogress) { + vty_out(vty, "test already in progress\n"); + return -1; + } + + if (skiplist_count(lp_tests) >= + (1 << (8 * sizeof(lpt_generation))) - 1) { + /* + * Too many test runs + */ + vty_out(vty, "too many tests: clear first\n"); + return -1; + } + + /* + * We pack the generation and request number into the labelid; + * make sure they fit. + */ + unsigned int n1 = LPT_MAX_COUNT; + unsigned int sh = 0; + unsigned int label_bits; + + label_bits = 8 * (sizeof(tcb->request_count) - sizeof(lpt_generation)); + + /* n1 should be same type as tcb->request_maximum */ + assert(sizeof(n1) == sizeof(tcb->request_maximum)); + + while (n1 >>= 1) + ++sh; + sh += 1; /* number of bits needed to hold LPT_MAX_COUNT */ + + if (sh > label_bits) { + vty_out(vty, + "Sorry, test iteration count too big on this platform (LPT_MAX_COUNT %u, need %u bits, but label_bits is only %u)\n", + LPT_MAX_COUNT, sh, label_bits); + return -1; + } + + lpt_inprogress = true; + ++lpt_generation; + + tcb = XCALLOC(MTYPE_LABELPOOL_TEST, sizeof(*tcb)); + + tcb->generation = lpt_generation; + tcb->label_type = LP_TYPE_VRF; + tcb->request_maximum = LPT_MAX_COUNT; + tcb->request_blocksize = LPT_BLKSIZE; + tcb->labels = skiplist_new(0, NULL, NULL); + tcb->timestamps_alloc = skiplist_new(0, NULL, NULL); + tcb->timestamps_dealloc = skiplist_new(0, NULL, NULL); + thread_add_event(bm->master, labelpool_test_event_handler, NULL, 0, + &tcb->event_thread); + monotime(&tcb->starttime); + + skiplist_insert(lp_tests, (void *)(uintptr_t)tcb->generation, tcb); + return 0; +} + +DEFPY(start_labelpool_perf_test, start_labelpool_perf_test_cmd, + "debug bgp lptest start", + DEBUG_STR BGP_STR + "label pool test\n" + "start\n") +{ + lptest_start(vty); + return CMD_SUCCESS; +} + +static void lptest_print_stats(struct vty *vty, struct lp_test *tcb) +{ + unsigned int i; + + vty_out(vty, "Global Lookup Failures in test_cb: %5u\n", + lpt_test_cb_tcb_lookup_fails); + vty_out(vty, "Global Lookup Failures in release: %5u\n", + lpt_release_tcb_lookup_fails); + vty_out(vty, "Global Lookup Failures in event: %5u\n", + lpt_test_event_tcb_lookup_fails); + vty_out(vty, "Global Lookup Failures in stop: %5u\n", + lpt_stop_tcb_lookup_fails); + vty_out(vty, "\n"); + + if (!tcb) { + if (skiplist_search(lp_tests, (void *)(uintptr_t)lpt_generation, + (void **)&tcb)) { + vty_out(vty, "Error: can't find test %u\n", + lpt_generation); + return; + } + } + + vty_out(vty, "Test Generation %u:\n", tcb->generation); + + vty_out(vty, "Counter Value\n"); + for (i = 0; i < LPT_STAT_MAX; ++i) { + vty_out(vty, "%20s: %10u\n", lpt_counter_names[i], + tcb->counter[i]); + } + vty_out(vty, "\n"); + + if (tcb->timestamps_alloc) { + void *Key; + void *Value; + void *cursor; + + float elapsed; + + vty_out(vty, "%10s %10s\n", "Count", "Seconds"); + + cursor = NULL; + while (!skiplist_next(tcb->timestamps_alloc, &Key, &Value, + &cursor)) { + + elapsed = ((float)(uintptr_t)Value) / 1000; + + vty_out(vty, "%10llu %10.3f\n", + (unsigned long long)(uintptr_t)Key, elapsed); + } + vty_out(vty, "\n"); + } +} + +DEFPY(show_labelpool_perf_test, show_labelpool_perf_test_cmd, + "debug bgp lptest show", + DEBUG_STR BGP_STR + "label pool test\n" + "show\n") +{ + + if (lp_tests) { + void *Key; + void *Value; + void *cursor; + + cursor = NULL; + while (!skiplist_next(lp_tests, &Key, &Value, &cursor)) { + lptest_print_stats(vty, (struct lp_test *)Value); + } + } else { + vty_out(vty, "no test results\n"); + } + return CMD_SUCCESS; +} + +DEFPY(stop_labelpool_perf_test, stop_labelpool_perf_test_cmd, + "debug bgp lptest stop", + DEBUG_STR BGP_STR + "label pool test\n" + "stop\n") +{ + + if (lpt_inprogress) { + lptest_stop(); + lptest_print_stats(vty, NULL); + } else { + vty_out(vty, "no test in progress\n"); + } + return CMD_SUCCESS; +} + +DEFPY(clear_labelpool_perf_test, clear_labelpool_perf_test_cmd, + "debug bgp lptest clear", + DEBUG_STR BGP_STR + "label pool test\n" + "clear\n") +{ + + if (lpt_inprogress) { + lptest_stop(); + } + if (lp_tests) { + while (!skiplist_first(lp_tests, NULL, NULL)) + /* del function of skiplist cleans up tcbs */ + skiplist_delete_first(lp_tests); + } + return CMD_SUCCESS; +} + +/* + * With the "release" command, we can release labels at intervals through + * the ID space. Thus we can to exercise the bitfield-wrapping behavior + * of the allocator in a subsequent test. + */ +/* clang-format off */ +DEFPY(release_labelpool_perf_test, release_labelpool_perf_test_cmd, + "debug bgp lptest release test GENERATION$generation every (1-5)$every_nth", + DEBUG_STR + BGP_STR + "label pool test\n" + "release labels\n" + "\"test\"\n" + "test number\n" + "\"every\"\n" + "label fraction denominator\n") +{ + /* clang-format on */ + + unsigned long testnum; + char *end; + struct lp_test *tcb; + + testnum = strtoul(generation, &end, 0); + if (*end) { + vty_out(vty, "Invalid test number: \"%s\"\n", generation); + return CMD_SUCCESS; + } + if (lpt_inprogress && (testnum == lpt_generation)) { + vty_out(vty, + "Error: Test %lu is still in progress (stop first)\n", + testnum); + return CMD_SUCCESS; + } + + if (skiplist_search(lp_tests, (void *)(uintptr_t)testnum, + (void **)&tcb)) { + + /* couldn't find current test in progress */ + vty_out(vty, "Error: Can't look up test number: \"%lu\"\n", + testnum); + ++lpt_release_tcb_lookup_fails; + return CMD_SUCCESS; + } + + void *Key, *cKey; + void *Value, *cValue; + void *cursor; + unsigned int iteration; + int rc; + + cursor = NULL; + iteration = 0; + rc = skiplist_next(tcb->labels, &Key, &Value, &cursor); + + while (!rc) { + cKey = Key; + cValue = Value; + + /* find next item before we delete this one */ + rc = skiplist_next(tcb->labels, &Key, &Value, &cursor); + + if (!(iteration % every_nth)) { + bgp_lp_release(tcb->label_type, cKey, + (mpls_label_t)(uintptr_t)cValue); + skiplist_delete(tcb->labels, cKey, NULL); + ++tcb->counter[LPT_STAT_DEALLOCATED]; + } + ++iteration; + } + + return CMD_SUCCESS; +} + +static void lptest_delete(void *val) +{ + struct lp_test *tcb = (struct lp_test *)val; + void *Key; + void *Value; + void *cursor; + + if (tcb->labels) { + cursor = NULL; + while (!skiplist_next(tcb->labels, &Key, &Value, &cursor)) + bgp_lp_release(tcb->label_type, Key, + (mpls_label_t)(uintptr_t)Value); + skiplist_free(tcb->labels); + tcb->labels = NULL; + } + if (tcb->timestamps_alloc) { + cursor = NULL; + skiplist_free(tcb->timestamps_alloc); + tcb->timestamps_alloc = NULL; + } + + if (tcb->timestamps_dealloc) { + cursor = NULL; + skiplist_free(tcb->timestamps_dealloc); + tcb->timestamps_dealloc = NULL; + } + + if (tcb->event_thread) + thread_cancel(&tcb->event_thread); + + memset(tcb, 0, sizeof(*tcb)); + + XFREE(MTYPE_LABELPOOL_TEST, tcb); +} + +static void lptest_init(void) +{ + lp_tests = skiplist_new(0, NULL, lptest_delete); +} + +static void lptest_finish(void) +{ + if (lp_tests) { + skiplist_free(lp_tests); + lp_tests = NULL; + } +} + +/*------------------------------------------------------------------------ + * Testing code end + *------------------------------------------------------------------------*/ +#endif /* BGP_LABELPOOL_ENABLE_TESTS */ + void bgp_lp_vty_init(void) { install_element(VIEW_NODE, &show_bgp_labelpool_summary_cmd); @@ -963,4 +1548,12 @@ void bgp_lp_vty_init(void) install_element(VIEW_NODE, &show_bgp_labelpool_inuse_cmd); install_element(VIEW_NODE, &show_bgp_labelpool_requests_cmd); install_element(VIEW_NODE, &show_bgp_labelpool_chunks_cmd); + +#if BGP_LABELPOOL_ENABLE_TESTS + install_element(ENABLE_NODE, &start_labelpool_perf_test_cmd); + install_element(ENABLE_NODE, &show_labelpool_perf_test_cmd); + install_element(ENABLE_NODE, &stop_labelpool_perf_test_cmd); + install_element(ENABLE_NODE, &release_labelpool_perf_test_cmd); + install_element(ENABLE_NODE, &clear_labelpool_perf_test_cmd); +#endif /* BGP_LABELPOOL_ENABLE_TESTS */ } diff --git a/bgpd/bgp_labelpool.h b/bgpd/bgp_labelpool.h index d6a8eec84d..2f3ffe437f 100644 --- a/bgpd/bgp_labelpool.h +++ b/bgpd/bgp_labelpool.h @@ -41,6 +41,7 @@ struct labelpool { struct work_queue *callback_q; uint32_t pending_count; /* requested from zebra */ uint32_t reconnect_count; /* zebra reconnections */ + uint32_t next_chunksize; /* request this many labels */ }; extern void bgp_lp_init(struct thread_master *master, struct labelpool *pool); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index e7fac8153c..9eb7407af6 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1776,10 +1776,20 @@ static void bgp_peer_remove_private_as(struct bgp *bgp, afi_t afi, safi_t safi, static void bgp_peer_as_override(struct bgp *bgp, afi_t afi, safi_t safi, struct peer *peer, struct attr *attr) { + struct aspath *aspath; + if (peer->sort == BGP_PEER_EBGP && - peer_af_flag_check(peer, afi, safi, PEER_FLAG_AS_OVERRIDE)) - attr->aspath = aspath_replace_specific_asn(attr->aspath, - peer->as, bgp->as); + peer_af_flag_check(peer, afi, safi, PEER_FLAG_AS_OVERRIDE)) { + if (attr->aspath->refcnt) + aspath = aspath_dup(attr->aspath); + else + aspath = attr->aspath; + + attr->aspath = aspath_intern( + aspath_replace_specific_asn(aspath, peer->as, bgp->as)); + + aspath_free(aspath); + } } void bgp_attr_add_llgr_community(struct attr *attr) @@ -6482,6 +6492,7 @@ static int bgp_static_set(struct vty *vty, const char *negate, /* Label index cannot be changed. */ if (bgp_static->label_index != label_index) { vty_out(vty, "%% cannot change label-index\n"); + bgp_dest_unlock_node(dest); return CMD_WARNING_CONFIG_FAILED; } @@ -7266,6 +7277,7 @@ static void bgp_aggregate_install( aggregate, atomic_aggregate, p); if (!attr) { + bgp_dest_unlock_node(dest); bgp_aggregate_delete(bgp, p, afi, safi, aggregate); if (BGP_DEBUG(update_groups, UPDATE_GROUPS)) zlog_debug("%s: %pFX null attribute", __func__, @@ -7426,31 +7438,21 @@ void bgp_aggregate_toggle_suppressed(struct bgp_aggregate *aggregate, static void bgp_aggregate_med_update(struct bgp_aggregate *aggregate, struct bgp *bgp, const struct prefix *p, afi_t afi, safi_t safi, - struct bgp_path_info *pi, bool is_adding) + struct bgp_path_info *pi) { /* MED matching disabled. */ if (!aggregate->match_med) return; - /* Aggregation with different MED, nothing to do. */ - if (aggregate->med_mismatched) - return; - - /* - * Test the current entry: - * - * is_adding == true: if the new entry doesn't match then we must - * install all suppressed routes. - * - * is_adding == false: if the entry being removed was the last - * unmatching entry then we can suppress all routes. + /* Aggregation with different MED, recheck if we have got equal MEDs + * now. */ - if (!is_adding) { - if (bgp_aggregate_test_all_med(aggregate, bgp, p, afi, safi) - && aggregate->summary_only) - bgp_aggregate_toggle_suppressed(aggregate, bgp, p, afi, - safi, true); - } else + if (aggregate->med_mismatched && + bgp_aggregate_test_all_med(aggregate, bgp, p, afi, safi) && + aggregate->summary_only) + bgp_aggregate_toggle_suppressed(aggregate, bgp, p, afi, safi, + true); + else bgp_aggregate_med_match(aggregate, bgp, pi); /* No mismatches, just quit. */ @@ -7816,7 +7818,7 @@ static void bgp_add_route_to_aggregate(struct bgp *bgp, */ if (aggregate->match_med) bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi, - pinew, true); + pinew); if (aggregate->summary_only && AGGREGATE_MED_VALID(aggregate)) aggr_suppress_path(aggregate, pinew); @@ -7939,8 +7941,7 @@ static void bgp_remove_route_from_aggregate(struct bgp *bgp, afi_t afi, * "unsuppressing" twice. */ if (aggregate->match_med) - bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi, pi, - true); + bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi, pi); if (aggregate->count > 0) aggregate->count--; @@ -11159,13 +11160,14 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi, if (type == bgp_show_type_route_map) { struct route_map *rmap = output_arg; struct bgp_path_info path; - struct attr dummy_attr; + struct bgp_path_info_extra extra; + struct attr dummy_attr = {}; route_map_result_t ret; dummy_attr = *pi->attr; - path.peer = pi->peer; - path.attr = &dummy_attr; + prep_for_rmap_apply(&path, &extra, dest, pi, + pi->peer, &dummy_attr); ret = route_map_apply(rmap, dest_p, &path); bgp_attr_flush(&dummy_attr); diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 40bbdccff4..64c867f988 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -2270,6 +2270,8 @@ route_set_aspath_replace(void *rule, const struct prefix *dummy, void *object) aspath_new, replace_asn, own_asn); } + aspath_free(aspath_new); + return RMAP_OKAY; } @@ -4143,8 +4145,6 @@ static void bgp_route_map_process_update_cb(char *rmap_name) void bgp_route_map_update_timer(struct thread *thread) { - bm->t_rmap_update = NULL; - route_map_walk_update_list(bgp_route_map_process_update_cb); } diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index de1c559641..b90c09c68b 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -428,14 +428,15 @@ static void bgpd_sync_callback(struct thread *thread) safi_t safi; for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) { - if (!bgp->rib[afi][safi]) + struct bgp_table *table = bgp->rib[afi][safi]; + + if (!table) continue; struct bgp_dest *match; struct bgp_dest *node; - match = bgp_table_subtree_lookup(bgp->rib[afi][safi], - prefix); + match = bgp_table_subtree_lookup(table, prefix); node = match; while (node) { @@ -445,6 +446,9 @@ static void bgpd_sync_callback(struct thread *thread) node = bgp_route_next_until(node, match); } + + if (match) + bgp_dest_unlock_node(match); } } diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h index d832383ab4..86cd4f3da1 100644 --- a/bgpd/bgp_table.h +++ b/bgpd/bgp_table.h @@ -256,28 +256,6 @@ static inline struct bgp_dest *bgp_node_match(const struct bgp_table *table, return bgp_dest_from_rnode(rn); } -/* - * bgp_node_match_ipv4 - */ -static inline struct bgp_dest * -bgp_node_match_ipv4(const struct bgp_table *table, struct in_addr *addr) -{ - struct route_node *rn = route_node_match_ipv4(table->route_table, addr); - - return bgp_dest_from_rnode(rn); -} - -/* - * bgp_node_match_ipv6 - */ -static inline struct bgp_dest * -bgp_node_match_ipv6(const struct bgp_table *table, struct in6_addr *addr) -{ - struct route_node *rn = route_node_match_ipv6(table->route_table, addr); - - return bgp_dest_from_rnode(rn); -} - static inline unsigned long bgp_table_count(const struct bgp_table *const table) { return route_table_count(table->route_table); @@ -292,59 +270,6 @@ static inline struct bgp_dest *bgp_table_get_next(const struct bgp_table *table, return bgp_dest_from_rnode(route_table_get_next(table->route_table, p)); } -/* - * bgp_table_iter_init - */ -static inline void bgp_table_iter_init(bgp_table_iter_t *iter, - struct bgp_table *table) -{ - bgp_table_lock(table); - iter->table = table; - route_table_iter_init(&iter->rt_iter, table->route_table); -} - -/* - * bgp_table_iter_next - */ -static inline struct bgp_dest *bgp_table_iter_next(bgp_table_iter_t *iter) -{ - return bgp_dest_from_rnode(route_table_iter_next(&iter->rt_iter)); -} - -/* - * bgp_table_iter_cleanup - */ -static inline void bgp_table_iter_cleanup(bgp_table_iter_t *iter) -{ - route_table_iter_cleanup(&iter->rt_iter); - bgp_table_unlock(iter->table); - iter->table = NULL; -} - -/* - * bgp_table_iter_pause - */ -static inline void bgp_table_iter_pause(bgp_table_iter_t *iter) -{ - route_table_iter_pause(&iter->rt_iter); -} - -/* - * bgp_table_iter_is_done - */ -static inline int bgp_table_iter_is_done(bgp_table_iter_t *iter) -{ - return route_table_iter_is_done(&iter->rt_iter); -} - -/* - * bgp_table_iter_started - */ -static inline int bgp_table_iter_started(bgp_table_iter_t *iter) -{ - return route_table_iter_started(&iter->rt_iter); -} - /* This would benefit from a real atomic operation... * until then. */ static inline uint64_t bgp_table_next_version(struct bgp_table *table) diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index d198aec983..0219535b0d 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -254,6 +254,8 @@ static void conf_release(struct peer *src, afi_t afi, safi_t safi) XFREE(MTYPE_BGP_FILTER_NAME, srcfilter->advmap.cname); XFREE(MTYPE_BGP_PEER_HOST, src->host); + + ecommunity_free(&src->soo[afi][safi]); } static void peer2_updgrp_copy(struct update_group *updgrp, struct peer_af *paf) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a889a737eb..4c151b2d37 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1473,11 +1473,6 @@ void peer_xfer_config(struct peer *peer_dst, struct peer *peer_src) peer_dst->weight[afi][safi] = peer_src->weight[afi][safi]; peer_dst->addpath_type[afi][safi] = peer_src->addpath_type[afi][safi]; - if (peer_src->soo[afi][safi]) { - ecommunity_free(&peer_dst->soo[afi][safi]); - peer_dst->soo[afi][safi] = - ecommunity_dup(peer_src->soo[afi][safi]); - } } for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) { diff --git a/bgpd/subdir.am b/bgpd/subdir.am index 9e3a095529..b1eeb937e1 100644 --- a/bgpd/subdir.am +++ b/bgpd/subdir.am @@ -232,6 +232,7 @@ clippy_scan += \ bgpd/bgp_bmp.c \ bgpd/bgp_debug.c \ bgpd/bgp_evpn_vty.c \ + bgpd/bgp_labelpool.c \ bgpd/bgp_route.c \ bgpd/bgp_routemap.c \ bgpd/bgp_rpki.c \ diff --git a/eigrpd/eigrp_network.c b/eigrpd/eigrp_network.c index 6b3a0afc12..13db38ce91 100644 --- a/eigrpd/eigrp_network.c +++ b/eigrpd/eigrp_network.c @@ -69,8 +69,8 @@ int eigrp_sock_init(struct vrf *vrf) AF_INET, SOCK_RAW, IPPROTO_EIGRPIGP, vrf->vrf_id, vrf->vrf_id != VRF_DEFAULT ? vrf->name : NULL); if (eigrp_sock < 0) { - zlog_err("eigrp_read_sock_init: socket: %s", - safe_strerror(errno)); + zlog_err("%s: socket: %s", + __func__, safe_strerror(errno)); exit(1); } diff --git a/eigrpd/eigrp_packet.c b/eigrpd/eigrp_packet.c index dd5ba8a164..1ea6f9813b 100644 --- a/eigrpd/eigrp_packet.c +++ b/eigrpd/eigrp_packet.c @@ -727,8 +727,8 @@ static struct stream *eigrp_recv_packet(struct eigrp *eigrp, if ((unsigned int)ret < sizeof(*iph)) /* ret must be > 0 now */ { zlog_warn( - "eigrp_recv_packet: discarding runt packet of length %d (ip header size is %u)", - ret, (unsigned int)sizeof(*iph)); + "%s: discarding runt packet of length %d (ip header size is %u)", + __func__, ret, (unsigned int)sizeof(*iph)); return NULL; } @@ -772,8 +772,8 @@ static struct stream *eigrp_recv_packet(struct eigrp *eigrp, if (ret != ip_len) { zlog_warn( - "eigrp_recv_packet read length mismatch: ip_len is %d, but recvmsg returned %d", - ip_len, ret); + "%s read length mismatch: ip_len is %d, but recvmsg returned %d", + __func__, ip_len, ret); return NULL; } diff --git a/eigrpd/eigrpd.c b/eigrpd/eigrpd.c index 7bc7be9706..0ec9574813 100644 --- a/eigrpd/eigrpd.c +++ b/eigrpd/eigrpd.c @@ -163,7 +163,8 @@ static struct eigrp *eigrp_new(uint16_t as, vrf_id_t vrf_id) if (eigrp->fd < 0) { flog_err_sys( EC_LIB_SOCKET, - "eigrp_new: fatal error: eigrp_sock_init was unable to open a socket"); + "%s: fatal error: eigrp_sock_init was unable to open a socket", + __func__); exit(1); } diff --git a/lib/bitfield.h b/lib/bitfield.h index a3f361ed9d..9af4053daf 100644 --- a/lib/bitfield.h +++ b/lib/bitfield.h @@ -158,6 +158,75 @@ DECLARE_MTYPE(BITFIELD); (b) += (w * WORD_SIZE); \ } while (0) +/* + * Find a clear bit in v and return it + * Start looking in the word containing bit position start_index. + * If necessary, wrap around after bit position max_index. + */ +static inline unsigned int +bf_find_next_clear_bit_wrap(bitfield_t *v, word_t start_index, word_t max_index) +{ + int start_bit; + unsigned long i, offset, scanbits, wordcount_max, index_max; + + if (start_index > max_index) + start_index = 0; + + start_bit = start_index & (WORD_SIZE - 1); + wordcount_max = bf_index(max_index) + 1; + + scanbits = WORD_SIZE; + for (i = bf_index(start_index); i < v->m; ++i) { + if (v->data[i] == WORD_MAX) { + /* if the whole word is full move to the next */ + start_bit = 0; + continue; + } + /* scan one word for clear bits */ + if ((i == v->m - 1) && (v->m >= wordcount_max)) + /* max index could be only part of word */ + scanbits = (max_index % WORD_SIZE) + 1; + for (offset = start_bit; offset < scanbits; ++offset) { + if (!((v->data[i] >> offset) & 1)) + return ((i * WORD_SIZE) + offset); + } + /* move to the next word */ + start_bit = 0; + } + + if (v->m < wordcount_max) { + /* + * We can expand bitfield, so no need to wrap. + * Return the index of the first bit of the next word. + * Assumption is that caller will call bf_set_bit which + * will allocate additional space. + */ + v->m += 1; + v->data = (word_t *)realloc(v->data, v->m * sizeof(word_t)); + v->data[v->m - 1] = 0; + return v->m * WORD_SIZE; + } + + /* + * start looking for a clear bit at the start of the bitfield and + * stop when we reach start_index + */ + scanbits = WORD_SIZE; + index_max = bf_index(start_index - 1); + for (i = 0; i <= index_max; ++i) { + if (i == index_max) + scanbits = ((start_index - 1) % WORD_SIZE) + 1; + for (offset = start_bit; offset < scanbits; ++offset) { + if (!((v->data[i] >> offset) & 1)) + return ((i * WORD_SIZE) + offset); + } + /* move to the next word */ + start_bit = 0; + } + + return WORD_MAX; +} + static inline unsigned int bf_find_next_set_bit(bitfield_t v, word_t start_index) { diff --git a/lib/routemap.c b/lib/routemap.c index 9529b79419..e6310465e3 100644 --- a/lib/routemap.c +++ b/lib/routemap.c @@ -812,17 +812,13 @@ int route_map_mark_updated(const char *name) return (ret); } -static int route_map_clear_updated(struct route_map *map) +static void route_map_clear_updated(struct route_map *map) { - int ret = -1; - if (map) { map->to_be_processed = false; if (map->deleted) route_map_free_map(map); } - - return (ret); } /* Lookup route map. If there isn't route map create one and return diff --git a/tests/topotests/bgp_aggregate_address_matching_med/__init__.py b/tests/topotests/bgp_aggregate_address_matching_med/__init__.py new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/__init__.py diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r1/bgpd.conf b/tests/topotests/bgp_aggregate_address_matching_med/r1/bgpd.conf new file mode 100644 index 0000000000..35597602f7 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r1/bgpd.conf @@ -0,0 +1,21 @@ +! +router bgp 65001 + no bgp ebgp-requires-policy + neighbor 192.168.1.2 remote-as external + neighbor 192.168.1.2 timers 3 10 + address-family ipv4 unicast + redistribute connected + neighbor 192.168.1.2 route-map r2 out + exit-address-family +! +ip prefix-list p1 seq 5 permit 172.16.255.1/32 +ip prefix-list p1 seq 10 permit 172.16.255.2/32 +ip prefix-list p2 seq 15 permit 172.16.255.3/32 +! +route-map r2 permit 10 + match ip address prefix-list p1 + set metric 300 +route-map r2 permit 20 + match ip address prefix-list p2 + set metric 400 +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r1/zebra.conf b/tests/topotests/bgp_aggregate_address_matching_med/r1/zebra.conf new file mode 100644 index 0000000000..685adb3080 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r1/zebra.conf @@ -0,0 +1,11 @@ +! +interface lo + ip address 172.16.255.1/32 + ip address 172.16.255.2/32 + ip address 172.16.255.3/32 +! +interface r1-eth0 + ip address 192.168.1.1/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r2/bgpd.conf b/tests/topotests/bgp_aggregate_address_matching_med/r2/bgpd.conf new file mode 100644 index 0000000000..9bc9a3132f --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r2/bgpd.conf @@ -0,0 +1,11 @@ +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 192.168.1.1 remote-as external + neighbor 192.168.1.1 timers 3 10 + neighbor 192.168.2.1 remote-as external + neighbor 192.168.2.1 timers 3 10 + address-family ipv4 unicast + aggregate-address 172.16.255.0/24 summary-only matching-MED-only + exit-address-family +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r2/zebra.conf b/tests/topotests/bgp_aggregate_address_matching_med/r2/zebra.conf new file mode 100644 index 0000000000..f229954341 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r2/zebra.conf @@ -0,0 +1,9 @@ +! +interface r2-eth0 + ip address 192.168.1.2/24 +! +interface r2-eth1 + ip address 192.168.2.2/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r3/bgpd.conf b/tests/topotests/bgp_aggregate_address_matching_med/r3/bgpd.conf new file mode 100644 index 0000000000..dfb5ac7a3c --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r3/bgpd.conf @@ -0,0 +1,6 @@ +! +router bgp 65003 + no bgp ebgp-requires-policy + neighbor 192.168.2.2 remote-as external + neighbor 192.168.2.2 timers 3 10 +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r3/zebra.conf b/tests/topotests/bgp_aggregate_address_matching_med/r3/zebra.conf new file mode 100644 index 0000000000..11e06d47cc --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r3/zebra.conf @@ -0,0 +1,6 @@ +! +interface r3-eth0 + ip address 192.168.2.1/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/test_bgp_aggregate_address_matching_med.py b/tests/topotests/bgp_aggregate_address_matching_med/test_bgp_aggregate_address_matching_med.py new file mode 100644 index 0000000000..edf50dc9e0 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/test_bgp_aggregate_address_matching_med.py @@ -0,0 +1,148 @@ +#!/usr/bin/env python + +# +# Copyright (c) 2022 by +# Donatas Abraitis <donatas@opensourcerouting.org> +# +# Permission to use, copy, modify, and/or distribute this software +# for any purpose with or without fee is hereby granted, provided +# that the above copyright notice and this permission notice appear +# in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY +# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, +# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS +# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE +# OF THIS SOFTWARE. +# + +""" +Test if aggregate-address command works fine when suppressing summary-only +and using matching-MED-only together. +""" + +import os +import sys +import json +import pytest +import functools +from lib.common_config import ( + step, +) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen + +pytestmark = [pytest.mark.bgpd] + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + + +def build_topo(tgen): + for routern in range(1, 5): + tgen.add_router("r{}".format(routern)) + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r2"]) + + switch = tgen.add_switch("s2") + switch.add_link(tgen.gears["r2"]) + switch.add_link(tgen.gears["r3"]) + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for i, (rname, router) in enumerate(router_list.items(), 1): + router.load_config( + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) + ) + router.load_config( + TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname)) + ) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_aggregate_address_matching_med(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r3 = tgen.gears["r3"] + + def _bgp_converge(): + output = json.loads(r3.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "172.16.255.0/24": None, + "172.16.255.1/32": [{"path": "65002 65001"}], + "172.16.255.2/32": [{"path": "65002 65001"}], + "172.16.255.3/32": [{"path": "65002 65001"}], + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_converge) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed to see unsuppressed routes from R2" + + step("Change MED for 172.16.255.3/32 from 400 to 300") + r1.vtysh_cmd( + """ + configure terminal + route-map r2 permit 20 + set metric 300 + """ + ) + + step("Check if 172.16.255.0/24 aggregated route was created and others suppressed") + + def _bgp_aggregated_summary_only_med_match(): + output = json.loads(r3.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "172.16.255.0/24": [{"path": "65002"}], + "172.16.255.1/32": None, + "172.16.255.2/32": None, + "172.16.255.3/32": None, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_aggregated_summary_only_med_match) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed to see unsuppressed routes from R2" + + step("Change MED for 172.16.255.3/32 back to 400 from 300") + r1.vtysh_cmd( + """ + configure terminal + route-map r2 permit 20 + set metric 400 + """ + ) + test_func = functools.partial(_bgp_converge) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed to see unsuppressed routes from R2" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/tests/topotests/bgp_lu_topo1/R1/labelpool.summ.json b/tests/topotests/bgp_lu_topo1/R1/labelpool.summ.json index 29e6c2cbf7..e2eee513e6 100644 --- a/tests/topotests/bgp_lu_topo1/R1/labelpool.summ.json +++ b/tests/topotests/bgp_lu_topo1/R1/labelpool.summ.json @@ -2,7 +2,7 @@ "Ledger":506, "InUse":506, "Requests":0, - "LabelChunks":11, + "LabelChunks":3, "Pending":0, "Reconnects":0 } diff --git a/tests/topotests/bgp_lu_topo2/R1/labelpool.summ.json b/tests/topotests/bgp_lu_topo2/R1/labelpool.summ.json index 77705e8d35..0dc59b58cf 100644 --- a/tests/topotests/bgp_lu_topo2/R1/labelpool.summ.json +++ b/tests/topotests/bgp_lu_topo2/R1/labelpool.summ.json @@ -2,7 +2,7 @@ "Ledger":51, "InUse":51, "Requests":0, - "LabelChunks":2, + "LabelChunks":1, "Pending":0, "Reconnects":0 } diff --git a/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py b/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py index 7d52048ebe..1108919e13 100644 --- a/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py +++ b/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py @@ -361,13 +361,6 @@ def test_bgp_remove_private_as(): return True return False - def _get_pfx_path_from_nh(router, prefix, nh): - """Return as-path for a specific route + path.""" - output = json.loads(tgen.gears[router].vtysh_cmd(f"show ip bgp {prefix} json")) - for path in output[prefix]: - if path["nexthops"]["ip"] == nh: - return path["aspath"]["string"] - def _routers_up(tx_rtrs, rx_rtrs): """Ensure all BGP sessions are up and all routes are installed.""" # all sessions go through tx_routers, so ensure all their peers are up @@ -408,11 +401,7 @@ def test_bgp_remove_private_as(): for pfx in prefixes: good_path = expected_paths[rtr][remove_type][peer][pfx] real_path = adj_rib_in["receivedRoutes"][pfx]["path"] - msg = ( - f"{rtr} received incorrect AS-Path from {peer} " - f'({p_ip}) for {pfx}. remove_type: "{remove_type}"' - ) - assert real_path == good_path, msg + return real_path == good_path ####################### # Begin Test @@ -424,7 +413,11 @@ def test_bgp_remove_private_as(): # test each variation of remove-private-AS for rmv_type in remove_types: _change_remove_type(rmv_type, "add") - _validate_paths(rmv_type) + + test_func = partial(_validate_paths, rmv_type) + _, result = topotest.run_and_expect(test_func, True, count=60, wait=0.5) + assert result == True, "Not all routes have correct AS-Path values!" + # each variation sets a separate peer flag in bgpd. we need to clear # the old flag after each iteration so we only test the flags we expect. _change_remove_type(rmv_type, "del") diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c index 45a372f88c..a8b56bb8f2 100644 --- a/zebra/kernel_netlink.c +++ b/zebra/kernel_netlink.c @@ -1033,12 +1033,18 @@ static int netlink_parse_error(const struct nlsock *nl, struct nlmsghdr *h, return 1; } - /* Deal with errors that occur because of races in link handling. */ - if (is_cmd - && ((msg_type == RTM_DELROUTE - && (-errnum == ENODEV || -errnum == ESRCH)) - || (msg_type == RTM_NEWROUTE - && (-errnum == ENETDOWN || -errnum == EEXIST)))) { + /* + * Deal with errors that occur because of races in link handling + * or types are not supported in kernel. + */ + if (is_cmd && + ((msg_type == RTM_DELROUTE && + (-errnum == ENODEV || -errnum == ESRCH)) || + (msg_type == RTM_NEWROUTE && + (-errnum == ENETDOWN || -errnum == EEXIST)) || + ((msg_type == RTM_NEWTUNNEL || msg_type == RTM_DELTUNNEL || + msg_type == RTM_GETTUNNEL) && + (-errnum == EOPNOTSUPP)))) { if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug("%s: error: %s type=%s(%u), seq=%u, pid=%u", nl->name, safe_strerror(-errnum), |