From 44ca60ecdba866160bebbc6c1eb983674819d429 Mon Sep 17 00:00:00 2001 From: Benoît Ganne Date: Fri, 6 Sep 2019 13:43:16 +0200 Subject: gbp: fix contract rule handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak when removing old GBP contract rules and make sure a GBP contract rule exists when matching the corresponding ACL rule. Type: fix Fixes: 13a08cc098 Change-Id: Iba67d573e69280ad998488a7a3d3462341c68ea4 Signed-off-by: Benoît Ganne --- src/plugins/gbp/gbp_api.c | 2 ++ src/plugins/gbp/gbp_contract.c | 10 +++++++++- src/plugins/gbp/gbp_contract.h | 18 ++++++++++++++---- src/plugins/gbp/gbp_policy.c | 8 +++++--- src/plugins/gbp/gbp_policy.h | 10 +++++++--- src/plugins/gbp/gbp_policy_dpo.c | 11 +++++++---- src/plugins/gbp/gbp_policy_node.c | 11 ++++++----- 7 files changed, 50 insertions(+), 20 deletions(-) (limited to 'src/plugins/gbp') diff --git a/src/plugins/gbp/gbp_api.c b/src/plugins/gbp/gbp_api.c index 09455471c5e..b347c1ddea4 100644 --- a/src/plugins/gbp/gbp_api.c +++ b/src/plugins/gbp/gbp_api.c @@ -940,6 +940,8 @@ gbp_contract_rules_decode (u8 n_rules, if (0 != rv) { + index_t *gui; + vec_foreach (gui, guis) gbp_rule_free (*gui); vec_free (guis); return (rv); } diff --git a/src/plugins/gbp/gbp_contract.c b/src/plugins/gbp/gbp_contract.c index 452c5a5c359..e12b33145ce 100644 --- a/src/plugins/gbp/gbp_contract.c +++ b/src/plugins/gbp/gbp_contract.c @@ -73,6 +73,12 @@ gbp_rule_alloc (gbp_rule_action_t action, return (gu - gbp_rule_pool); } +void +gbp_rule_free (index_t gui) +{ + pool_put_index (gbp_rule_pool, gui); +} + index_t gbp_next_hop_alloc (const ip46_address_t * ip, index_t grd, const mac_address_t * mac, index_t gbd) @@ -139,6 +145,8 @@ gbp_contract_rules_free (index_t * rules) adj_unlock (gnh->gnh_ai[fproto]); } } + + gbp_rule_free (*gui); } vec_free (rules); } @@ -159,7 +167,7 @@ format_gbp_next_hop (u8 * s, va_list * args) return (s); } -static u8 * +u8 * format_gbp_rule_action (u8 * s, va_list * args) { gbp_rule_action_t action = va_arg (*args, gbp_rule_action_t); diff --git a/src/plugins/gbp/gbp_contract.h b/src/plugins/gbp/gbp_contract.h index 139de1c372b..509e785dd9e 100644 --- a/src/plugins/gbp/gbp_contract.h +++ b/src/plugins/gbp/gbp_contract.h @@ -28,7 +28,8 @@ _(DROP_CONTRACT, "drop-contract") \ _(DROP_ETHER_TYPE, "drop-ether-type") \ _(DROP_NO_CONTRACT, "drop-no-contract") \ - _(DROP_NO_DCLASS, "drop-no-dclass") + _(DROP_NO_DCLASS, "drop-no-dclass") \ + _(DROP_NO_RULE, "drop-no-rule") typedef enum { @@ -173,6 +174,7 @@ extern int gbp_contract_delete (gbp_scope_t scope, sclass_t sclass, extern index_t gbp_rule_alloc (gbp_rule_action_t action, gbp_hash_mode_t hash_mode, index_t * nhs); +extern void gbp_rule_free (index_t gui); extern index_t gbp_next_hop_alloc (const ip46_address_t * ip, index_t grd, const mac_address_t * mac, index_t gbd); @@ -180,6 +182,7 @@ extern index_t gbp_next_hop_alloc (const ip46_address_t * ip, typedef int (*gbp_contract_cb_t) (gbp_contract_t * gbpe, void *ctx); extern void gbp_contract_walk (gbp_contract_cb_t bgpe, void *ctx); +extern u8 *format_gbp_rule_action (u8 * s, va_list * args); extern u8 *format_gbp_contract (u8 * s, va_list * args); /** @@ -230,13 +233,14 @@ static_always_inline gbp_rule_action_t gbp_contract_apply (vlib_main_t * vm, gbp_main_t * gm, gbp_contract_key_t * key, vlib_buffer_t * b, gbp_rule_t ** rule, u32 * intra, u32 * sclass1, + u32 * acl_match, u32 * rule_match, gbp_contract_error_t * err, gbp_contract_apply_type_t type) { fa_5tuple_opaque_t fa_5tuple; const gbp_contract_t *contract; index_t contract_index; - u32 acl_pos, acl_match, rule_match, trace_bitmap; + u32 acl_pos, trace_bitmap; u16 etype; u8 ip6, action; @@ -315,12 +319,18 @@ gbp_contract_apply (vlib_main_t * vm, gbp_main_t * gm, &fa_5tuple); acl_plugin_match_5tuple_inline (gm->acl_plugin.p_acl_main, contract->gc_lc_index, &fa_5tuple, ip6, - &action, &acl_pos, &acl_match, &rule_match, + &action, &acl_pos, acl_match, rule_match, &trace_bitmap); if (action <= 0) goto contract_deny; - *rule = gbp_rule_get (contract->gc_rules[rule_match]); + if (PREDICT_FALSE (*rule_match >= vec_len (contract->gc_rules))) + { + *err = GBP_CONTRACT_ERROR_DROP_NO_RULE; + goto contract_deny; + } + + *rule = gbp_rule_get (contract->gc_rules[*rule_match]); switch ((*rule)->gu_action) { case GBP_RULE_PERMIT: diff --git a/src/plugins/gbp/gbp_policy.c b/src/plugins/gbp/gbp_policy.c index 21e4bdba064..127c6d3f059 100644 --- a/src/plugins/gbp/gbp_policy.c +++ b/src/plugins/gbp/gbp_policy.c @@ -28,9 +28,11 @@ format_gbp_policy_trace (u8 * s, va_list * args) gbp_policy_trace_t *t = va_arg (*args, gbp_policy_trace_t *); s = - format (s, "scope:%d sclass:%d, dclass:%d, allowed:%d flags:%U", t->scope, - t->sclass, t->dclass, t->allowed, format_vxlan_gbp_header_gpflags, - t->flags); + format (s, + "scope:%d sclass:%d, dclass:%d, action:%U flags:%U acl: %d rule: %d", + t->scope, t->sclass, t->dclass, format_gbp_rule_action, t->action, + format_vxlan_gbp_header_gpflags, t->flags, t->acl_match, + t->rule_match); return s; } diff --git a/src/plugins/gbp/gbp_policy.h b/src/plugins/gbp/gbp_policy.h index 36bb4933c11..6f87f2ec7c4 100644 --- a/src/plugins/gbp/gbp_policy.h +++ b/src/plugins/gbp/gbp_policy.h @@ -27,15 +27,17 @@ typedef struct gbp_policy_trace_t_ gbp_scope_t scope; sclass_t sclass; sclass_t dclass; - u32 allowed; + gbp_rule_action_t action; u32 flags; + u32 acl_match; + u32 rule_match; } gbp_policy_trace_t; /* packet trace format function */ u8 * format_gbp_policy_trace (u8 * s, va_list * args); static_always_inline void -gbp_policy_trace(vlib_main_t * vm, vlib_node_runtime_t * node, vlib_buffer_t *b, const gbp_contract_key_t *key, u8 allowed) +gbp_policy_trace(vlib_main_t * vm, vlib_node_runtime_t * node, vlib_buffer_t *b, const gbp_contract_key_t *key, gbp_rule_action_t action, u32 acl_match, u32 rule_match) { gbp_policy_trace_t *t; @@ -46,8 +48,10 @@ gbp_policy_trace(vlib_main_t * vm, vlib_node_runtime_t * node, vlib_buffer_t *b, t->sclass = key->gck_src; t->dclass = key->gck_dst; t->scope = key->gck_scope; - t->allowed = allowed; + t->action = action; t->flags = vnet_buffer2 (b)->gbp.flags; + t->acl_match = acl_match; + t->rule_match = rule_match; } #endif /* __GBP_POLICY_H__ */ diff --git a/src/plugins/gbp/gbp_policy_dpo.c b/src/plugins/gbp/gbp_policy_dpo.c index dec30e46336..9f26b9c67ab 100644 --- a/src/plugins/gbp/gbp_policy_dpo.c +++ b/src/plugins/gbp/gbp_policy_dpo.c @@ -268,13 +268,14 @@ gbp_policy_dpo_inline (vlib_main_t * vm, while (n_left_from > 0 && n_left_to_next > 0) { + gbp_rule_action_t action0 = GBP_RULE_DENY; + u32 acl_match = ~0, rule_match = ~0; const gbp_policy_dpo_t *gpd0; - gbp_rule_action_t action0; gbp_contract_error_t err0; - u32 bi0, next0; gbp_contract_key_t key0; vlib_buffer_t *b0; gbp_rule_t *rule0; + u32 bi0, next0; bi0 = from[0]; to_next[0] = bi0; @@ -325,7 +326,8 @@ gbp_policy_dpo_inline (vlib_main_t * vm, action0 = gbp_contract_apply (vm, gm, &key0, b0, &rule0, &n_allow_intra, - &n_allow_sclass_1, &err0, + &n_allow_sclass_1, &acl_match, &rule_match, + &err0, is_ip6 ? GBP_CONTRACT_APPLY_IP6 : GBP_CONTRACT_APPLY_IP4); switch (action0) @@ -345,7 +347,8 @@ gbp_policy_dpo_inline (vlib_main_t * vm, } trace: - gbp_policy_trace (vm, node, b0, &key0, (next0 != GBP_POLICY_DROP)); + gbp_policy_trace (vm, node, b0, &key0, action0, acl_match, + rule_match); vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next, n_left_to_next, bi0, next0); diff --git a/src/plugins/gbp/gbp_policy_node.c b/src/plugins/gbp/gbp_policy_node.c index 7bbcffa5b47..8c6ef5c2b94 100644 --- a/src/plugins/gbp/gbp_policy_node.c +++ b/src/plugins/gbp/gbp_policy_node.c @@ -116,10 +116,11 @@ gbp_policy_inline (vlib_main_t * vm, while (n_left_from > 0 && n_left_to_next > 0) { + gbp_rule_action_t action0 = GBP_RULE_DENY; const ethernet_header_t *h0; const gbp_endpoint_t *ge0; - gbp_rule_action_t action0; gbp_contract_error_t err0; + u32 acl_match = ~0, rule_match = ~0; gbp_policy_next_t next0; gbp_contract_key_t key0; u32 bi0, sw_if_index0; @@ -220,8 +221,8 @@ gbp_policy_inline (vlib_main_t * vm, action0 = gbp_contract_apply (vm, gm, &key0, b0, &rule0, &n_allow_intra, - &n_allow_sclass_1, &err0, - GBP_CONTRACT_APPLY_L2); + &n_allow_sclass_1, &acl_match, &rule_match, + &err0, GBP_CONTRACT_APPLY_L2); switch (action0) { case GBP_RULE_PERMIT: @@ -239,8 +240,8 @@ gbp_policy_inline (vlib_main_t * vm, } trace: - gbp_policy_trace (vm, node, b0, &key0, - (next0 != GBP_POLICY_NEXT_DROP)); + gbp_policy_trace (vm, node, b0, &key0, action0, acl_match, + rule_match); /* verify speculative enqueue, maybe switch current next frame */ vlib_validate_buffer_enqueue_x1 (vm, node, next_index, -- cgit 1.2.3-korg