summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew Yourtchenko <ayourtch@gmail.com>2018-06-18 12:15:09 +0200
committerFlorin Coras <florin.coras@gmail.com>2018-06-21 07:37:56 +0000
commitb53f4d0e184f8c8e117cbaa79ca7f6b9e88e11ba (patch)
treee8fa5cd1a7ac7cf1b59d00c0408306c34dd71671
parenta6726b59f1488f1510b143e7533f0062e19786f6 (diff)
acl-plugin: fallback to linear ACL search for fragments
Trying to accomodate fragments as first class citizens has shown to be more trouble than it's worth. So fallback to linear ACL search in case it is a fragment packet. Delete the corresponding code from the hash matching. Change-Id: Ic9ecc7c800d575615addb33dcaa89621462e9c7b Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
-rw-r--r--src/plugins/acl/hash_lookup.c57
-rw-r--r--src/plugins/acl/public_inlines.h15
2 files changed, 34 insertions, 38 deletions
diff --git a/src/plugins/acl/hash_lookup.c b/src/plugins/acl/hash_lookup.c
index 7e76794969d..935ef2680ab 100644
--- a/src/plugins/acl/hash_lookup.c
+++ b/src/plugins/acl/hash_lookup.c
@@ -571,7 +571,7 @@ make_port_mask(u16 *portmask, u16 port_first, u16 port_last)
}
static void
-make_mask_and_match_from_rule(fa_5tuple_t *mask, acl_rule_t *r, hash_ace_info_t *hi, int match_nonfirst_fragment)
+make_mask_and_match_from_rule(fa_5tuple_t *mask, acl_rule_t *r, hash_ace_info_t *hi)
{
memset(mask, 0, sizeof(*mask));
memset(&hi->match, 0, sizeof(hi->match));
@@ -600,30 +600,25 @@ make_mask_and_match_from_rule(fa_5tuple_t *mask, acl_rule_t *r, hash_ace_info_t
if (r->proto != 0) {
mask->l4.proto = ~0; /* L4 proto needs to be matched */
hi->match.l4.proto = r->proto;
- if (match_nonfirst_fragment) {
- /* match the non-first fragments only */
- mask->pkt.is_nonfirst_fragment = 1;
- hi->match.pkt.is_nonfirst_fragment = 1;
- } else {
- /* Calculate the src/dst port masks and make the src/dst port matches accordingly */
- hi->src_portrange_not_powerof2 = make_port_mask(&mask->l4.port[0], r->src_port_or_type_first, r->src_port_or_type_last);
- hi->match.l4.port[0] = r->src_port_or_type_first & mask->l4.port[0];
- hi->dst_portrange_not_powerof2 = make_port_mask(&mask->l4.port[1], r->dst_port_or_code_first, r->dst_port_or_code_last);
- hi->match.l4.port[1] = r->dst_port_or_code_first & mask->l4.port[1];
- /* L4 info must be valid in order to match */
- mask->pkt.l4_valid = 1;
- hi->match.pkt.l4_valid = 1;
- /* And we must set the mask to check that it is an initial fragment */
- mask->pkt.is_nonfirst_fragment = 1;
- hi->match.pkt.is_nonfirst_fragment = 0;
- if ((r->proto == IPPROTO_TCP) && (r->tcp_flags_mask != 0)) {
- /* if we want to match on TCP flags, they must be masked off as well */
- mask->pkt.tcp_flags = r->tcp_flags_mask;
- hi->match.pkt.tcp_flags = r->tcp_flags_value;
- /* and the flags need to be present within the packet being matched */
- mask->pkt.tcp_flags_valid = 1;
- hi->match.pkt.tcp_flags_valid = 1;
- }
+
+ /* Calculate the src/dst port masks and make the src/dst port matches accordingly */
+ hi->src_portrange_not_powerof2 = make_port_mask(&mask->l4.port[0], r->src_port_or_type_first, r->src_port_or_type_last);
+ hi->match.l4.port[0] = r->src_port_or_type_first & mask->l4.port[0];
+ hi->dst_portrange_not_powerof2 = make_port_mask(&mask->l4.port[1], r->dst_port_or_code_first, r->dst_port_or_code_last);
+ hi->match.l4.port[1] = r->dst_port_or_code_first & mask->l4.port[1];
+ /* L4 info must be valid in order to match */
+ mask->pkt.l4_valid = 1;
+ hi->match.pkt.l4_valid = 1;
+ /* And we must set the mask to check that it is an initial fragment */
+ mask->pkt.is_nonfirst_fragment = 1;
+ hi->match.pkt.is_nonfirst_fragment = 0;
+ if ((r->proto == IPPROTO_TCP) && (r->tcp_flags_mask != 0)) {
+ /* if we want to match on TCP flags, they must be masked off as well */
+ mask->pkt.tcp_flags = r->tcp_flags_mask;
+ hi->match.pkt.tcp_flags = r->tcp_flags_value;
+ /* and the flags need to be present within the packet being matched */
+ mask->pkt.tcp_flags_valid = 1;
+ hi->match.pkt.tcp_flags_valid = 1;
}
}
/* Sanitize the mask and the match */
@@ -711,7 +706,7 @@ void hash_acl_add(acl_main_t *am, int acl_index)
ace_info.acl_index = acl_index;
ace_info.ace_index = i;
- make_mask_and_match_from_rule(&mask, &a->rules[i], &ace_info, 0);
+ make_mask_and_match_from_rule(&mask, &a->rules[i], &ace_info);
ace_info.mask_type_index = assign_mask_type_index(am, &mask);
/* assign the mask type index for matching itself */
ace_info.match.pkt.mask_type_index_lsb = ace_info.mask_type_index;
@@ -719,16 +714,6 @@ void hash_acl_add(acl_main_t *am, int acl_index)
/* Ensure a given index is set in the mask type index bitmap for this ACL */
ha->mask_type_index_bitmap = clib_bitmap_set(ha->mask_type_index_bitmap, ace_info.mask_type_index, 1);
vec_add1(ha->rules, ace_info);
- if (am->l4_match_nonfirst_fragment) {
- /* add the second rule which matches the noninitial fragments with the respective mask */
- make_mask_and_match_from_rule(&mask, &a->rules[i], &ace_info, 1);
- ace_info.mask_type_index = assign_mask_type_index(am, &mask);
- ace_info.match.pkt.mask_type_index_lsb = ace_info.mask_type_index;
- DBG("ACE: %d (non-initial frags) mask_type_index: %d", i, ace_info.mask_type_index);
- /* Ensure a given index is set in the mask type index bitmap for this ACL */
- ha->mask_type_index_bitmap = clib_bitmap_set(ha->mask_type_index_bitmap, ace_info.mask_type_index, 1);
- vec_add1(ha->rules, ace_info);
- }
}
/*
* if an ACL is applied somewhere, fill the corresponding lookup data structures.
diff --git a/src/plugins/acl/public_inlines.h b/src/plugins/acl/public_inlines.h
index f7d7abbec0a..f5ce0da6da4 100644
--- a/src/plugins/acl/public_inlines.h
+++ b/src/plugins/acl/public_inlines.h
@@ -611,9 +611,20 @@ acl_plugin_match_5tuple_inline (void *p_acl_main, u32 lc_index,
acl_main_t *am = p_acl_main;
fa_5tuple_t * pkt_5tuple_internal = (fa_5tuple_t *)pkt_5tuple;
pkt_5tuple_internal->pkt.lc_index = lc_index;
- if (am->use_hash_acl_matching) {
- return hash_multi_acl_match_5tuple(p_acl_main, lc_index, pkt_5tuple_internal, is_ip6, r_action,
+ if (PREDICT_TRUE(am->use_hash_acl_matching)) {
+ if (PREDICT_FALSE(pkt_5tuple_internal->pkt.is_nonfirst_fragment)) {
+ /*
+ * tuplemerge does not take fragments into account,
+ * and in general making fragments first class citizens has
+ * proved more overhead than it's worth - so just fall back to linear
+ * matching in that case.
+ */
+ return linear_multi_acl_match_5tuple(p_acl_main, lc_index, pkt_5tuple_internal, is_ip6, r_action,
r_acl_pos_p, r_acl_match_p, r_rule_match_p, trace_bitmap);
+ } else {
+ return hash_multi_acl_match_5tuple(p_acl_main, lc_index, pkt_5tuple_internal, is_ip6, r_action,
+ r_acl_pos_p, r_acl_match_p, r_rule_match_p, trace_bitmap);
+ }
} else {
return linear_multi_acl_match_5tuple(p_acl_main, lc_index, pkt_5tuple_internal, is_ip6, r_action,
r_acl_pos_p, r_acl_match_p, r_rule_match_p, trace_bitmap);