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)
This commit is contained in:
Benoît Ganne
2019-09-06 13:43:16 +02:00
committed by Andrew Yourtchenko
parent 68ac86e923
commit 5308ce13f6
7 changed files with 50 additions and 20 deletions

View File

@ -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);
}

View File

@ -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);

View File

@ -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:

View File

@ -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;
}

View File

@ -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__ */

View File

@ -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);

View File

@ -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,