From d0a59722135ec77e637097ef99edb6865bc38929 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Sun, 15 Oct 2017 17:41:21 +0000 Subject: [PATCH] Revert "Enforce FIB table creation before use" This reverts commit f9342023c19887da656133e2688a90d70383b0c5. Reverting to unblock master. No idea why jjb +1ed this patch! On closer inspection it looks like it -1ed it and subsequently changed opinion. CSIT tests should be fixed before re-merging. Change-Id: I26608912a962c52083073e16c7c9d2cc44a3cc8d Signed-off-by: Florin Coras --- src/vat/api_format.c | 22 +++++++++++++++++++--- src/vnet/interface_api.c | 43 +++++++++++++++++++++++++++++++++++-------- src/vnet/ip/ip.api | 3 +++ src/vnet/ip/ip_api.c | 37 ++++++++++++++++++++++++++++++++++--- src/vnet/mfib/mfib_table.h | 1 + src/vnet/mpls/mpls.api | 5 +++++ test/test_dvr.py | 2 ++ test/test_srv6.py | 19 ++----------------- 8 files changed, 101 insertions(+), 31 deletions(-) diff --git a/src/vat/api_format.c b/src/vat/api_format.c index 35b46f49fb0..e7382c29720 100644 --- a/src/vat/api_format.c +++ b/src/vat/api_format.c @@ -7562,6 +7562,7 @@ api_ip_add_del_route (vat_main_t * vam) u8 is_ipv6 = 0; u8 is_local = 0, is_drop = 0; u8 is_unreach = 0, is_prohibit = 0; + u8 create_vrf_if_needed = 0; u8 is_add = 1; u32 next_hop_weight = 1; u8 not_last = 0; @@ -7658,6 +7659,8 @@ api_ip_add_del_route (vat_main_t * vam) is_multipath = 1; else if (unformat (i, "vrf %d", &vrf_id)) ; + else if (unformat (i, "create-vrf")) + create_vrf_if_needed = 1; else if (unformat (i, "count %d", &count)) ; else if (unformat (i, "lookup-in-vrf %d", &next_hop_table_id)) @@ -7744,6 +7747,7 @@ api_ip_add_del_route (vat_main_t * vam) mp->next_hop_sw_if_index = ntohl (sw_if_index); mp->table_id = ntohl (vrf_id); + mp->create_vrf_if_needed = create_vrf_if_needed; mp->is_add = is_add; mp->is_drop = is_drop; @@ -7857,6 +7861,7 @@ api_ip_mroute_add_del (vat_main_t * vam) u32 sw_if_index = ~0, vrf_id = 0; u8 is_ipv6 = 0; u8 is_local = 0; + u8 create_vrf_if_needed = 0; u8 is_add = 1; u8 address_set = 0; u32 grp_address_length = 0; @@ -7913,6 +7918,8 @@ api_ip_mroute_add_del (vat_main_t * vam) is_add = 1; else if (unformat (i, "vrf %d", &vrf_id)) ; + else if (unformat (i, "create-vrf")) + create_vrf_if_needed = 1; else if (unformat (i, "%U", unformat_mfib_itf_flags, &iflags)) ; else if (unformat (i, "%U", unformat_mfib_entry_flags, &eflags)) @@ -7935,6 +7942,7 @@ api_ip_mroute_add_del (vat_main_t * vam) mp->next_hop_sw_if_index = ntohl (sw_if_index); mp->table_id = ntohl (vrf_id); + mp->create_vrf_if_needed = create_vrf_if_needed; mp->is_add = is_add; mp->is_ipv6 = is_ipv6; @@ -7975,12 +7983,12 @@ api_mpls_table_add_del (vat_main_t * vam) /* Parse args required to build the message */ while (unformat_check_input (i) != UNFORMAT_END_OF_INPUT) { - if (unformat (i, "del")) + if (unformat (i, "table %d", &table_id)) + ; + else if (unformat (i, "del")) is_add = 0; else if (unformat (i, "add")) is_add = 1; - else if (unformat (i, "table-id %d", &table_id)) - ; else { clib_warning ("parse error '%U'", format_unformat_error, i); @@ -8015,6 +8023,7 @@ api_mpls_route_add_del (vat_main_t * vam) unformat_input_t *i = vam->input; vl_api_mpls_route_add_del_t *mp; u32 sw_if_index = ~0, table_id = 0; + u8 create_table_if_needed = 0; u8 is_add = 1; u32 next_hop_weight = 1; u8 is_multipath = 0; @@ -8064,6 +8073,8 @@ api_mpls_route_add_del (vat_main_t * vam) } else if (unformat (i, "weight %d", &next_hop_weight)) ; + else if (unformat (i, "create-table")) + create_table_if_needed = 1; else if (unformat (i, "classify %d", &classify_table_index)) { is_classify = 1; @@ -8131,6 +8142,7 @@ api_mpls_route_add_del (vat_main_t * vam) mp->mr_next_hop_sw_if_index = ntohl (sw_if_index); mp->mr_table_id = ntohl (table_id); + mp->mr_create_table_if_needed = create_table_if_needed; mp->mr_is_add = is_add; mp->mr_next_hop_proto = next_hop_proto; @@ -8236,6 +8248,7 @@ api_mpls_ip_bind_unbind (vat_main_t * vam) unformat_input_t *i = vam->input; vl_api_mpls_ip_bind_unbind_t *mp; u32 ip_table_id = 0; + u8 create_table_if_needed = 0; u8 is_bind = 1; u8 is_ip4 = 1; ip4_address_t v4_address; @@ -8262,6 +8275,8 @@ api_mpls_ip_bind_unbind (vat_main_t * vam) } else if (unformat (i, "%d", &local_label)) ; + else if (unformat (i, "create-table")) + create_table_if_needed = 1; else if (unformat (i, "table-id %d", &ip_table_id)) ; else if (unformat (i, "unbind")) @@ -8290,6 +8305,7 @@ api_mpls_ip_bind_unbind (vat_main_t * vam) /* Construct the API message */ M (MPLS_IP_BIND_UNBIND, mp); + mp->mb_create_table_if_needed = create_table_if_needed; mp->mb_is_bind = is_bind; mp->mb_is_ip4 = is_ip4; mp->mb_ip_table_id = ntohl (ip_table_id); diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c index ee13135f086..c19f0a84297 100644 --- a/src/vnet/interface_api.c +++ b/src/vnet/interface_api.c @@ -365,14 +365,6 @@ ip_table_bind (fib_protocol_t fproto, fib_source_t src; mfib_source_t msrc; - fib_index = fib_table_find (fproto, table_id); - mfib_index = mfib_table_find (fproto, table_id); - - if (~0 == fib_index || ~0 == mfib_index) - { - return (VNET_API_ERROR_NO_SUCH_FIB); - } - if (is_api) { src = FIB_SOURCE_API; @@ -384,6 +376,32 @@ ip_table_bind (fib_protocol_t fproto, msrc = MFIB_SOURCE_CLI; } + /* + * This is temporary whilst I do the song and dance with the CSIT version + */ + if (0 != table_id) + { + fib_index = fib_table_find_or_create_and_lock (fproto, table_id, src); + mfib_index = + mfib_table_find_or_create_and_lock (fproto, table_id, msrc); + } + else + { + fib_index = 0; + mfib_index = 0; + } + + /* + * This if table does not exist = error is what we want in the end. + */ + /* fib_index = fib_table_find (fproto, table_id); */ + /* mfib_index = mfib_table_find (fproto, table_id); */ + + /* if (~0 == fib_index || ~0 == mfib_index) */ + /* { */ + /* return (VNET_API_ERROR_NO_SUCH_FIB); */ + /* } */ + if (FIB_PROTOCOL_IP6 == fproto) { /* @@ -490,6 +508,15 @@ ip_table_bind (fib_protocol_t fproto, ip4_main.mfib_index_by_sw_if_index[sw_if_index] = mfib_index; } + /* + * Temporary. undo the locks from the find and create at the staart + */ + if (0 != table_id) + { + fib_table_unlock (fib_index, fproto, src); + mfib_table_unlock (mfib_index, fproto, msrc); + } + return (0); } diff --git a/src/vnet/ip/ip.api b/src/vnet/ip/ip.api index f5341667892..8501eb26030 100644 --- a/src/vnet/ip/ip.api +++ b/src/vnet/ip/ip.api @@ -362,6 +362,7 @@ autoreply define sw_interface_ip6_set_link_local_address @param vrf_id - fib table /vrf associated with the route @param lookup_in_vrf - @param classify_table_index - + @param create_vrf_if_needed - @param is_add - 1 if adding the route, 0 if deleting @param is_drop - Drop the packet @param is_unreach - Drop the packet and rate limit send ICMP unreachable @@ -390,6 +391,7 @@ autoreply define ip_add_del_route u32 table_id; u32 classify_table_index; u32 next_hop_table_id; + u8 create_vrf_if_needed; u8 is_add; u8 is_drop; u8 is_unreach; @@ -432,6 +434,7 @@ autoreply define ip_mroute_add_del u32 itf_flags; u32 rpf_id; u16 grp_address_length; + u8 create_vrf_if_needed; u8 is_add; u8 is_ipv6; u8 is_local; diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c index 8a3ceb2a4e9..a64c5b7b55b 100644 --- a/src/vnet/ip/ip_api.c +++ b/src/vnet/ip/ip_api.c @@ -989,11 +989,23 @@ add_del_route_check (fib_protocol_t table_proto, { vnet_main_t *vnm = vnet_get_main (); + /* Temporaray whilst I do the CSIT dance */ + u8 create_missing_tables = 1; + *fib_index = fib_table_find (table_proto, ntohl (table_id)); if (~0 == *fib_index) { - /* No such VRF */ - return VNET_API_ERROR_NO_SUCH_FIB; + if (create_missing_tables) + { + *fib_index = fib_table_find_or_create_and_lock (table_proto, + ntohl (table_id), + FIB_SOURCE_API); + } + else + { + /* No such VRF, and we weren't asked to create one */ + return VNET_API_ERROR_NO_SUCH_FIB; + } } if (!is_rpf_id && ~0 != ntohl (next_hop_sw_if_index)) @@ -1022,7 +1034,26 @@ add_del_route_check (fib_protocol_t table_proto, if (~0 == *next_hop_fib_index) { - return VNET_API_ERROR_NO_SUCH_FIB; + if (create_missing_tables) + { + if (is_rpf_id) + *next_hop_fib_index = + mfib_table_find_or_create_and_lock (fib_nh_proto, + ntohl + (next_hop_table_id), + MFIB_SOURCE_API); + else + *next_hop_fib_index = + fib_table_find_or_create_and_lock (fib_nh_proto, + ntohl + (next_hop_table_id), + FIB_SOURCE_API); + } + else + { + /* No such VRF, and we weren't asked to create one */ + return VNET_API_ERROR_NO_SUCH_FIB; + } } } diff --git a/src/vnet/mfib/mfib_table.h b/src/vnet/mfib/mfib_table.h index 4c1077f900d..6a5810291fc 100644 --- a/src/vnet/mfib/mfib_table.h +++ b/src/vnet/mfib/mfib_table.h @@ -347,6 +347,7 @@ extern u32 mfib_table_find_or_create_and_lock_w_name(fib_protocol_t proto, mfib_source_t source, const u8 *name); + /** * @brief * Take a reference counting lock on the table diff --git a/src/vnet/mpls/mpls.api b/src/vnet/mpls/mpls.api index 8cc1ea88a9a..3c817db1306 100644 --- a/src/vnet/mpls/mpls.api +++ b/src/vnet/mpls/mpls.api @@ -22,6 +22,7 @@ vl_api_version 1.0.0 @param mb_mpls_table_id - The MPLS table-id the MPLS entry will be added in @param mb_label - The MPLS label value to bind @param mb_ip_table_id - The IP table-id of the IP prefix to bind to. + @param mb_create_table_if_needed - Create either/both tables if required. @param mb_is_bind - Bind or unbind @param mb_is_ip4 - The prefix to bind to is IPv4 @param mb_address_length - Length of IP prefix @@ -34,6 +35,7 @@ autoreply define mpls_ip_bind_unbind u32 mb_mpls_table_id; u32 mb_label; u32 mb_ip_table_id; + u8 mb_create_table_if_needed; u8 mb_is_bind; u8 mb_is_ip4; u8 mb_address_length; @@ -162,6 +164,8 @@ autoreply define mpls_table_add_del @param mr_table_id - The MPLS table-id the route is added in @param mr_classify_table_index - If this is a classify route, this is the classify table index + @param mr_create_table_if_needed - If the MPLS or IP tables do not exist, + create them @param mr_is_add - Is this a route add or delete @param mr_is_classify - Is this route result a classify @param mr_is_multicast - Is this a multicast route @@ -189,6 +193,7 @@ autoreply define mpls_route_add_del u8 mr_eos; u32 mr_table_id; u32 mr_classify_table_index; + u8 mr_create_table_if_needed; u8 mr_is_add; u8 mr_is_classify; u8 mr_is_multicast; diff --git a/test/test_dvr.py b/test/test_dvr.py index 03c23ac1d03..27522a54eea 100644 --- a/test/test_dvr.py +++ b/test/test_dvr.py @@ -83,6 +83,8 @@ class TestDVR(VppTestCase): L2_VTR_OP.L2_POP_1, 93) + self.logger.error(self.vapi.ppcli("show bridge-domain 1 detail")) + # # Add routes to bridge the traffic via a tagged an nontagged interface # diff --git a/test/test_srv6.py b/test/test_srv6.py index 4c463808151..a31b30eb619 100644 --- a/test/test_srv6.py +++ b/test/test_srv6.py @@ -4,7 +4,7 @@ import unittest from socket import AF_INET6 from framework import VppTestCase, VppTestRunner -from vpp_ip_route import VppIpRoute, VppRoutePath, DpoProto, VppIpTable +from vpp_ip_route import VppIpRoute, VppRoutePath, DpoProto from vpp_srv6 import SRv6LocalSIDBehaviors, VppSRv6LocalSID, VppSRv6Policy, \ SRv6PolicyType, VppSRv6Steering, SRv6PolicySteeringTypes @@ -100,19 +100,6 @@ class TestSRv6(VppTestCase): # create 'count' pg interfaces self.create_pg_interfaces(range(count)) - # create all tables - self.tables = [] - ids = sorted(set(ipv4_table_id)) - for i in range(len(ids)): - if 0 != ids[i]: - self.tables.append(VppIpTable(self, ids[i])) - ids = sorted(set(ipv6_table_id)) - for i in range(len(ids)): - if 0 != ids[i]: - self.tables.append(VppIpTable(self, ids[i], is_ip6=1)) - for t in self.tables: - t.add_vpp_config() - # setup all interfaces for i in range(count): intf = self.pg_interfaces[i] @@ -137,10 +124,8 @@ class TestSRv6(VppTestCase): # AFAIK they cannot be deleted for i in self.pg_interfaces: self.logger.debug("Tear down interface %s" % (i.name)) - i.unconfig() - i.set_table_ip4(0) - i.set_table_ip6(0) i.admin_down() + i.unconfig() def test_SRv6_T_Encaps(self): """ Test SRv6 Transit.Encaps behavior for IPv6. -- 2.16.6