Skip to content

Commit f9ffb0e

Browse files
sombrafamalmusil
authored andcommitted
controller: ACL correctly handles fragmented traffic.
At the current state, OVN can not handle fragmented traffic for ACLs in the userspace datapath (DPDK). Just like in the case of LB (commit 20a96b9), the kernel DP will try to reassemble the fragments during CT lookup, however userspace won't reassemble them. This patch allows OVN to handle fragmented traffic by defining a translation table on southbound that leverages OpenFlow connection tracking capabilities. When a stateful flow is created on NB, we add a hint in the flow. This hint will be read in SB and if the connection tracking is set to be used, SB will use the alternative translation table that will use the connection tracking information. This approach should not change the current behavior and it's only enabled if acl_ct_translation is set: ovn-nbctl set NB_Global . options:acl_ct_translation=true Reported-at: https://issues.redhat.com/browse/FDP-1992 Signed-off-by: Erlon R. Cruz <erlon@canonical.com> Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ales Musil <amusil@redhat.com>
1 parent b4a0942 commit f9ffb0e

13 files changed

Lines changed: 698 additions & 43 deletions

File tree

NEWS

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ Post v25.09.0
104104
- Add support for special port_security prefix "VRRPv3". This prefix allows
105105
CMS to allow all required traffic for a VRRPv3 virtual router behind LSP.
106106
See ovn-nb(5) man page for more details.
107+
- Fixed support for fragmented traffic in the userspace datapath. Added the
108+
"acl_ct_translation" NB_Global option to enable connection tracking
109+
based L4 field translation for stateful ACLs. When enabled allows proper
110+
handling of IP fragmentation in userspace datapaths. This option may break
111+
hardware offloading and is disabled by default.
107112

108113
OVN v25.09.0 - xxx xx xxxx
109114
--------------------------

controller/lflow.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,19 @@ COVERAGE_DEFINE(consider_logical_flow);
5151
/* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
5252
static struct shash symtab;
5353

54+
/* Alternative symbol table for ACL CT translation.
55+
* This symbol table maps L4 port fields (tcp/udp/sctp) to their connection
56+
* tracking equivalents (ct_tp_src/ct_tp_dst with ct_proto predicates).
57+
* This allows matching on all IP fragments (not just the first fragment)
58+
* so that all fragments can be matched based on the connection tracking state.
59+
*/
60+
static struct shash acl_ct_symtab;
61+
5462
void
5563
lflow_init(void)
5664
{
5765
ovn_init_symtab(&symtab);
66+
ovn_init_acl_ct_symtab(&acl_ct_symtab);
5867
}
5968

6069
struct lookup_port_aux {
@@ -1001,7 +1010,15 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow,
10011010
lflow->match);
10021011
return NULL;
10031012
}
1004-
struct expr *e = expr_parse_string(lex_str_get(&match_s), &symtab,
1013+
1014+
/* Check if this logical flow requires ACL CT translation.
1015+
* If the tags contains "acl_ct_translation"="true", we use the
1016+
* alternative symbol table that maps L4 fields (tcp/udp/sctp ports)
1017+
* to their CT equivalents. */
1018+
bool ct_trans = smap_get_bool(&lflow->tags, "acl_ct_translation", false);
1019+
struct shash *symtab_to_use = ct_trans ? &acl_ct_symtab : &symtab;
1020+
1021+
struct expr *e = expr_parse_string(lex_str_get(&match_s), symtab_to_use,
10051022
addr_sets, port_groups, &addr_sets_ref,
10061023
&port_groups_ref,
10071024
ldp->datapath->tunnel_key,
@@ -1033,7 +1050,7 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow,
10331050
e = expr_combine(EXPR_T_AND, e, *prereqs);
10341051
*prereqs = NULL;
10351052
}
1036-
e = expr_annotate(e, &symtab, &error);
1053+
e = expr_annotate(e, symtab_to_use, &error);
10371054
}
10381055
if (error) {
10391056
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
@@ -2062,6 +2079,8 @@ lflow_destroy(void)
20622079
{
20632080
expr_symtab_destroy(&symtab);
20642081
shash_destroy(&symtab);
2082+
expr_symtab_destroy(&acl_ct_symtab);
2083+
shash_destroy(&acl_ct_symtab);
20652084
}
20662085

20672086
bool

include/ovn/logical-fields.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,8 @@ const char *event_to_string(enum ovn_controller_event event);
278278
int string_to_event(const char *s);
279279
const struct ovn_field *ovn_field_from_name(const char *name);
280280

281+
void ovn_init_acl_ct_symtab(struct shash *symtab);
282+
281283
/* OVN CT label values
282284
* ===================
283285
* These are specific ct.label bit values OVN uses to track different types

lib/logical-fields.c

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,3 +451,70 @@ ovn_field_from_name(const char *name)
451451

452452
return shash_find_data(&ovnfield_by_name, name);
453453
}
454+
455+
/* Removes the symbol with 'name' from 'symtab', freeing its memory. */
456+
static void
457+
expr_symtab_remove(struct shash *symtab, const char *name)
458+
{
459+
struct expr_symbol *symbol = shash_find_and_delete(symtab, name);
460+
if (symbol) {
461+
free(symbol->name);
462+
free(symbol->prereqs);
463+
free(symbol->predicate);
464+
free(symbol);
465+
}
466+
}
467+
468+
/* Initialize a symbol table for ACL CT translation.
469+
* This creates an alternative symbol table that maps L4 port fields
470+
* (tcp/udp/sctp) to their connection tracking equivalents. This allows
471+
* matching on all IP fragments (not just the first) by using CT state
472+
* which is available for all fragments. */
473+
void
474+
ovn_init_acl_ct_symtab(struct shash *acl_symtab)
475+
{
476+
/* Initialize with the standard symbol table. */
477+
ovn_init_symtab(acl_symtab);
478+
479+
/* Remove the original tcp/udp/sctp symbols that we will override. */
480+
expr_symtab_remove(acl_symtab, "tcp.src");
481+
expr_symtab_remove(acl_symtab, "tcp.dst");
482+
expr_symtab_remove(acl_symtab, "tcp");
483+
expr_symtab_remove(acl_symtab, "udp.src");
484+
expr_symtab_remove(acl_symtab, "udp.dst");
485+
expr_symtab_remove(acl_symtab, "udp");
486+
expr_symtab_remove(acl_symtab, "sctp.src");
487+
expr_symtab_remove(acl_symtab, "sctp.dst");
488+
expr_symtab_remove(acl_symtab, "sctp");
489+
490+
/* Add ct_proto field - CT original direction protocol. Used in the
491+
* tcp/udp/sctp predicate expansions below. */
492+
expr_symtab_add_field(acl_symtab, "ct_proto", MFF_CT_NW_PROTO,
493+
"ct.trk", false);
494+
495+
/* Override TCP protocol and port fields to use CT equivalents.
496+
* When "tcp" is used as a predicate, it expands to "ct_proto == 6"
497+
* instead of "ip.proto == 6". */
498+
expr_symtab_add_predicate(acl_symtab, "tcp",
499+
"ct.trk && !ct.inv && ct_proto == 6");
500+
expr_symtab_add_field(acl_symtab, "tcp.src", MFF_CT_TP_SRC,
501+
"tcp", false);
502+
expr_symtab_add_field(acl_symtab, "tcp.dst", MFF_CT_TP_DST,
503+
"tcp", false);
504+
505+
/* Override UDP protocol and port fields */
506+
expr_symtab_add_predicate(acl_symtab, "udp",
507+
"ct.trk && !ct.inv && ct_proto == 17");
508+
expr_symtab_add_field(acl_symtab, "udp.src", MFF_CT_TP_SRC,
509+
"udp", false);
510+
expr_symtab_add_field(acl_symtab, "udp.dst", MFF_CT_TP_DST,
511+
"udp", false);
512+
513+
/* Override SCTP protocol and port fields */
514+
expr_symtab_add_predicate(acl_symtab, "sctp",
515+
"ct.trk && !ct.inv && ct_proto == 132");
516+
expr_symtab_add_field(acl_symtab, "sctp.src", MFF_CT_TP_SRC,
517+
"sctp", false);
518+
expr_symtab_add_field(acl_symtab, "sctp.dst", MFF_CT_TP_DST,
519+
"sctp", false);
520+
}

lib/ovn-util.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -698,19 +698,23 @@ ovn_pipeline_from_name(const char *pipeline)
698698
uint32_t
699699
sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf)
700700
{
701+
bool acl_ct_translation = smap_get_bool(&lf->tags,
702+
"acl_ct_translation", false);
701703
return ovn_logical_flow_hash(lf->table_id,
702704
ovn_pipeline_from_name(lf->pipeline),
703705
lf->priority, lf->match,
704-
lf->actions);
706+
lf->actions, acl_ct_translation);
705707
}
706708

707709
uint32_t
708710
ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline,
709711
uint16_t priority,
710-
const char *match, const char *actions)
712+
const char *match, const char *actions,
713+
bool acl_ct_translation)
711714
{
712715
size_t hash = hash_2words((table_id << 16) | priority, pipeline);
713716
hash = hash_string(match, hash);
717+
hash = hash_boolean(acl_ct_translation, hash);
714718
return hash_string(actions, hash);
715719
}
716720

lib/ovn-util.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ enum ovn_pipeline {
155155
uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *);
156156
uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline,
157157
uint16_t priority,
158-
const char *match, const char *actions);
158+
const char *match, const char *actions,
159+
bool acl_ct_translation);
159160
void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
160161
const char *argv[] OVS_UNUSED, void *idl_);
161162

northd/en-global-config.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,11 @@ check_nb_options_out_of_sync(
717717
return true;
718718
}
719719

720+
if (config_out_of_sync(&nb->options, &config_data->nb_options,
721+
"acl_ct_translation", false)) {
722+
return true;
723+
}
724+
720725
return false;
721726
}
722727

0 commit comments

Comments
 (0)