From 6b4830dc1ea8fa38633d7ef65539a7b2d64d6ca7 Mon Sep 17 00:00:00 2001 From: lynne Date: Sun, 29 Mar 2020 13:43:05 -0400 Subject: [PATCH 1/2] tests: adding LDP tests for advertising host-routes. Adding test to verify that proper LDP labels are advertised if host-route configuration is changed. Signed-off-by: Lynne Morrison --- .../r1/show_ldp_all_binding.ref | 61 +++++++++++++++++ .../r2/show_ldp_all_binding.ref | 63 +++++++++++++++++ .../r3/show_ldp_all_binding.ref | 61 +++++++++++++++++ .../r4/show_ldp_all_binding.ref | 68 +++++++++++++++++++ .../ldp-oc-acl-topo1/test_ldp_oc_acl_topo1.py | 16 +++++ 5 files changed, 269 insertions(+) create mode 100644 tests/topotests/ldp-oc-acl-topo1/r1/show_ldp_all_binding.ref create mode 100644 tests/topotests/ldp-oc-acl-topo1/r2/show_ldp_all_binding.ref create mode 100644 tests/topotests/ldp-oc-acl-topo1/r3/show_ldp_all_binding.ref create mode 100644 tests/topotests/ldp-oc-acl-topo1/r4/show_ldp_all_binding.ref diff --git a/tests/topotests/ldp-oc-acl-topo1/r1/show_ldp_all_binding.ref b/tests/topotests/ldp-oc-acl-topo1/r1/show_ldp_all_binding.ref new file mode 100644 index 0000000000..99a59668f8 --- /dev/null +++ b/tests/topotests/ldp-oc-acl-topo1/r1/show_ldp_all_binding.ref @@ -0,0 +1,61 @@ +{ + "bindings":[ + { + "addressFamily":"ipv4", + "prefix":"1.1.1.1/32", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"2.2.2.2/32", + "neighborId":"2.2.2.2", + "remoteLabel":"imp-null", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"3.3.3.3/32", + "neighborId":"2.2.2.2", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"4.4.4.4/32", + "neighborId":"0.0.0.0", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.1.0/24", + "neighborId":"2.2.2.2", + "localLabel":"imp-null", + "remoteLabel":"imp-null", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.2.0/24", + "neighborId":"2.2.2.2", + "remoteLabel":"imp-null", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.3.0/24", + "neighborId":"2.2.2.2", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"123.0.1.0/24", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + } + ] +} diff --git a/tests/topotests/ldp-oc-acl-topo1/r2/show_ldp_all_binding.ref b/tests/topotests/ldp-oc-acl-topo1/r2/show_ldp_all_binding.ref new file mode 100644 index 0000000000..95fb847c1e --- /dev/null +++ b/tests/topotests/ldp-oc-acl-topo1/r2/show_ldp_all_binding.ref @@ -0,0 +1,63 @@ +{ + "bindings":[ + { + "addressFamily":"ipv4", + "prefix":"1.1.1.1/32", + "neighborId":"1.1.1.1", + "remoteLabel":"imp-null", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"2.2.2.2/32", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"3.3.3.3/32", + "neighborId":"3.3.3.3", + "remoteLabel":"imp-null", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"4.4.4.4/32", + "neighborId":"0.0.0.0", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.1.0/24", + "neighborId":"1.1.1.1", + "localLabel":"imp-null", + "remoteLabel":"imp-null", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.2.0/24", + "neighborId":"3.3.3.3", + "localLabel":"imp-null", + "remoteLabel":"imp-null", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.3.0/24", + "neighborId":"3.3.3.3", + "remoteLabel":"imp-null", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"123.0.1.0/24", + "neighborId":"1.1.1.1", + "remoteLabel":"imp-null", + "inUse":1 + } + ] +} diff --git a/tests/topotests/ldp-oc-acl-topo1/r3/show_ldp_all_binding.ref b/tests/topotests/ldp-oc-acl-topo1/r3/show_ldp_all_binding.ref new file mode 100644 index 0000000000..100dd307ea --- /dev/null +++ b/tests/topotests/ldp-oc-acl-topo1/r3/show_ldp_all_binding.ref @@ -0,0 +1,61 @@ +{ + "bindings":[ + { + "addressFamily":"ipv4", + "prefix":"1.1.1.1/32", + "neighborId":"2.2.2.2", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"2.2.2.2/32", + "neighborId":"2.2.2.2", + "remoteLabel":"imp-null", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"3.3.3.3/32", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"4.4.4.4/32", + "neighborId":"0.0.0.0", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.1.0/24", + "neighborId":"2.2.2.2", + "remoteLabel":"imp-null", + "inUse":1 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.2.0/24", + "neighborId":"2.2.2.2", + "localLabel":"imp-null", + "remoteLabel":"imp-null", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.3.0/24", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"123.0.1.0/24", + "neighborId":"2.2.2.2", + "inUse":1 + } + ] +} diff --git a/tests/topotests/ldp-oc-acl-topo1/r4/show_ldp_all_binding.ref b/tests/topotests/ldp-oc-acl-topo1/r4/show_ldp_all_binding.ref new file mode 100644 index 0000000000..2a46c40346 --- /dev/null +++ b/tests/topotests/ldp-oc-acl-topo1/r4/show_ldp_all_binding.ref @@ -0,0 +1,68 @@ +{ + "bindings":[ + { + "addressFamily":"ipv4", + "prefix":"1.1.1.1/32", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"2.2.2.2/32", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"3.3.3.3/32", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"4.4.4.4/32", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.1.0/24", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.2.0/24", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"10.0.3.0/24", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + }, + { + "addressFamily":"ipv4", + "prefix":"123.0.1.0/24", + "neighborId":"0.0.0.0", + "localLabel":"imp-null", + "remoteLabel":"-", + "inUse":0 + } + ] +} diff --git a/tests/topotests/ldp-oc-acl-topo1/test_ldp_oc_acl_topo1.py b/tests/topotests/ldp-oc-acl-topo1/test_ldp_oc_acl_topo1.py index 47b32a16e6..9695c0d345 100755 --- a/tests/topotests/ldp-oc-acl-topo1/test_ldp_oc_acl_topo1.py +++ b/tests/topotests/ldp-oc-acl-topo1/test_ldp_oc_acl_topo1.py @@ -213,6 +213,22 @@ def test_ldp_bindings(): for rname in ['r1', 'r2', 'r3', 'r4']: router_compare_json_output(rname, "show mpls ldp binding json", "show_ldp_binding.ref") +def test_ldp_bindings_all_routes(): + logger.info("Test: verify LDP bindings after host filter removed") + tgen = get_topogen() + + # Skip if previous fatal error condition is raised + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + # remove ACL that blocks advertising everything but host routes */ + cmd = 'vtysh -c \"configure terminal\" -c \"mpls ldp\" -c \"address-family ipv4\" -c \"no label local allocate host-routes\"' + tgen.net['r1'].cmd(cmd) + sleep(2) + + for rname in ['r1', 'r2', 'r3', 'r4']: + router_compare_json_output(rname, "show mpls ldp binding json", "show_ldp_all_binding.ref") + # Memory leak test template def test_memory_leak(): "Run the memory leak test and report results." From 8675356098c9d30b9e566c0434138f1a7578ebfe Mon Sep 17 00:00:00 2001 From: lynne Date: Sun, 29 Mar 2020 13:47:36 -0400 Subject: [PATCH 2/2] ldpd: fixing host-only configuration filter. There is configuration in LDP to only create labels for host-routes. If the user remove this configuration the code was not readvertising non-host routes to it's LDP neighbors. The issue is the same in reverse also. If the user adds this configuration on an active LDP session the non-host routes were not withdrawn. Signed-off-by: Lynne Morrison --- ldpd/lde.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ ldpd/lde.h | 1 + ldpd/ldpd.c | 13 +++++++++++++ 3 files changed, 64 insertions(+) diff --git a/ldpd/lde.c b/ldpd/lde.c index 64c12e0df5..d4113a7869 100644 --- a/ldpd/lde.c +++ b/ldpd/lde.c @@ -1637,6 +1637,56 @@ lde_change_egress_label(int af) NULL, 0); } +void +lde_change_host_label(int af) +{ + struct lde_nbr *ln; + struct fec *f; + struct fec_node *fn; + uint32_t new_label; + + RB_FOREACH(f, fec_tree, &ft) { + fn = (struct fec_node *)f; + + switch (af) { + case AF_INET: + if (fn->fec.type != FEC_TYPE_IPV4) + continue; + break; + case AF_INET6: + if (fn->fec.type != FEC_TYPE_IPV6) + continue; + break; + default: + fatalx("lde_change_host_label: unknown af"); + } + + /* + * If the local label has changed to NO_LABEL, send a label + * withdraw to all peers. + * If the local label has changed and it's different from + * NO_LABEL, send a label mapping to all peers advertising + * the new label. + * If the local label hasn't changed, do nothing + */ + new_label = lde_update_label(fn); + if (fn->local_label != new_label) { + if (new_label == NO_LABEL) + RB_FOREACH(ln, nbr_tree, &lde_nbrs) + lde_send_labelwithdraw(ln, fn, + NULL, NULL); + + fn->local_label = new_label; + if (fn->local_label != NO_LABEL) + RB_FOREACH(ln, nbr_tree, &lde_nbrs) + lde_send_labelmapping(ln, fn, 0); + } + } + RB_FOREACH(ln, nbr_tree, &lde_nbrs) + lde_imsg_compose_ldpe(IMSG_MAPPING_ADD_END, ln->peerid, 0, + NULL, 0); +} + static int lde_address_add(struct lde_nbr *ln, struct lde_addr *lde_addr) { diff --git a/ldpd/lde.h b/ldpd/lde.h index a099f8d286..36196a3d08 100644 --- a/ldpd/lde.h +++ b/ldpd/lde.h @@ -183,6 +183,7 @@ void lde_req_del(struct lde_nbr *, struct lde_req *, int); struct lde_wdraw *lde_wdraw_add(struct lde_nbr *, struct fec_node *); void lde_wdraw_del(struct lde_nbr *, struct lde_wdraw *); void lde_change_egress_label(int); +void lde_change_host_label(int); struct lde_addr *lde_address_find(struct lde_nbr *, int, union ldpd_addr *); diff --git a/ldpd/ldpd.c b/ldpd/ldpd.c index 0f9f055d02..741c8c4655 100644 --- a/ldpd/ldpd.c +++ b/ldpd/ldpd.c @@ -1319,6 +1319,7 @@ merge_af(int af, struct ldpd_af_conf *af_conf, struct ldpd_af_conf *xa) int stop_init_backoff = 0; int remove_dynamic_tnbrs = 0; int change_egress_label = 0; + int change_host_label = 0; int reset_nbrs_ipv4 = 0; int reset_nbrs = 0; int update_sockets = 0; @@ -1349,6 +1350,12 @@ merge_af(int af, struct ldpd_af_conf *af_conf, struct ldpd_af_conf *xa) if ((af_conf->flags & F_LDPD_AF_EXPNULL) != (xa->flags & F_LDPD_AF_EXPNULL)) change_egress_label = 1; + + /* changing config of host only fec filtering */ + if ((af_conf->flags & F_LDPD_AF_ALLOCHOSTONLY) + != (xa->flags & F_LDPD_AF_ALLOCHOSTONLY)) + change_host_label = 1; + af_conf->flags = xa->flags; /* update the transport address */ @@ -1358,6 +1365,10 @@ merge_af(int af, struct ldpd_af_conf *af_conf, struct ldpd_af_conf *xa) } /* update ACLs */ + if (strcmp(af_conf->acl_label_allocate_for, + xa->acl_label_allocate_for)) + change_host_label = 1; + if (strcmp(af_conf->acl_label_advertise_to, xa->acl_label_advertise_to) || strcmp(af_conf->acl_label_advertise_for, @@ -1391,6 +1402,8 @@ merge_af(int af, struct ldpd_af_conf *af_conf, struct ldpd_af_conf *xa) case PROC_LDE_ENGINE: if (change_egress_label) lde_change_egress_label(af); + if (change_host_label) + lde_change_host_label(af); break; case PROC_LDP_ENGINE: if (stop_init_backoff)