summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBenoît Ganne <bganne@cisco.com>2019-09-06 13:43:16 +0200
committerAndrew Yourtchenko <ayourtch@gmail.com>2019-10-03 08:41:20 +0000
commit5308ce13f6b070cb1e4558cb70b330ef548544cf (patch)
treec8d9f9baab257d95cb73229c3ee8eae03c9cceb4
parent68ac86e923ce55bcc0ea82c4b5a0e0ef83b56c23 (diff)
gbp: fix contract rule handling
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 <bganne@cisco.com> (cherry picked from commit 44ca60ecdba866160bebbc6c1eb983674819d429)
-rw-r--r--src/plugins/gbp/gbp_api.c2
-rw-r--r--src/plugins/gbp/gbp_contract.c10
-rw-r--r--src/plugins/gbp/gbp_contract.h18
-rw-r--r--src/plugins/gbp/gbp_policy.c8
-rw-r--r--src/plugins/gbp/gbp_policy.h10
-rw-r--r--src/plugins/gbp/gbp_policy_dpo.c11
-rw-r--r--src/plugins/gbp/gbp_policy_node.c11
7 files changed, 50 insertions, 20 deletions
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,