Great thanks for the help, guys! I just saw that it was merged. On Wed, Feb 18, 2026 at 8:09 AM Ales Musil <[email protected]> wrote:
> > > On Wed, Feb 18, 2026 at 8:46 AM Dumitru Ceara <[email protected]> wrote: > >> On 2/17/26 4:12 PM, Erlon R. Cruz wrote: >> > 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 >> > >> > Signed-off-by: Erlon R. Cruz <[email protected]> >> > --- >> > v2: Rebased on current upstream main, removed external python >> > code traffic generators, added scenario tests for: dhcp, negative >> > udp and ovn rule generation, documentation and many code clean ups. >> > DHCP traffic is being dropped for some reason. Still need to figure >> > that out for v3. >> > v3: Rebased on current upstream main, clean ups. Fix DHCP/broadcast >> > bug >> > v4: Rebase, code cleanup >> > v5: Rebase, code cleanup, make ovn_lflow_find acl_ct_translation aware >> > --- >> >> Hi Erlon, >> >> This version looks good to me. I only have a few very minor comments. >> I know Ales wanted to review the patch too and, if he doesn't have any >> major comments either, I hope he can fix up the small issues I found >> below. >> >> With that: >> Acked-by: Dumitru Ceara <[email protected]> >> >> Thanks, >> Dumitru >> > > Thank you Erlon and Dumitru, > I have just one extra nit below. > > >> >> > NEWS | 5 + >> > controller/lflow.c | 23 +- >> > include/ovn/logical-fields.h | 2 + >> > lib/logical-fields.c | 67 ++++++ >> > lib/ovn-util.c | 10 +- >> > lib/ovn-util.h | 3 +- >> > northd/en-global-config.c | 5 + >> > northd/lflow-mgr.c | 76 ++++-- >> > northd/lflow-mgr.h | 4 +- >> > northd/northd.c | 48 +++- >> > ovn-nb.xml | 23 ++ >> > tests/ovn.at | 75 ++++++ >> > tests/system-ovn.at | 440 +++++++++++++++++++++++++++++++++-- >> > 13 files changed, 718 insertions(+), 63 deletions(-) >> > >> > diff --git a/NEWS b/NEWS >> > index c566ebfc8..6eb77b0e0 100644 >> > --- a/NEWS >> > +++ b/NEWS >> > @@ -104,6 +104,11 @@ Post v25.09.0 >> > - Add support for special port_security prefix "VRRPv3". This >> prefix allows >> > CMS to allow all required traffic for a VRRPv3 virtual router >> behind LSP. >> > See ovn-nb(5) man page for more details. >> > + - Fixed support for fragmented traffic in the userspace datapath. >> Added the >> > + "acl_ct_translation" NB_Global option to enable connection >> tracking >> > + based L4 field translation for stateful ACLs. When enabled allows >> proper >> > + handling of IP fragmentation in userspace datapaths. This option >> breaks >> >> Nit: I'd rephrase this as "this option may break..." >> >> > + hardware offloading and is disabled by default. >> > >> > OVN v25.09.0 - xxx xx xxxx >> > -------------------------- >> > diff --git a/controller/lflow.c b/controller/lflow.c >> > index a12627b0a..35ed6d30b 100644 >> > --- a/controller/lflow.c >> > +++ b/controller/lflow.c >> > @@ -51,10 +51,19 @@ COVERAGE_DEFINE(consider_logical_flow); >> > /* Contains "struct expr_symbol"s for fields supported by OVN lflows. >> */ >> > static struct shash symtab; >> > >> > +/* Alternative symbol table for ACL CT translation. >> > + * This symbol table maps L4 port fields (tcp/udp/sctp) to their >> connection >> > + * tracking equivalents (ct_tp_src/ct_tp_dst with ct_proto predicates). >> > + * This allows matching on all IP fragments (not just the first >> fragment) >> > + * so that all fragments can be matched based on the connection >> tracking state. >> > + */ >> > +static struct shash acl_ct_symtab; >> > + >> > void >> > lflow_init(void) >> > { >> > ovn_init_symtab(&symtab); >> > + ovn_init_acl_ct_symtab(&acl_ct_symtab); >> > } >> > >> > struct lookup_port_aux { >> > @@ -1001,7 +1010,15 @@ convert_match_to_expr(const struct >> sbrec_logical_flow *lflow, >> > lflow->match); >> > return NULL; >> > } >> > - struct expr *e = expr_parse_string(lex_str_get(&match_s), &symtab, >> > + >> > + /* Check if this logical flow requires ACL CT translation. >> > + * If the tags contains "acl_ct_translation"="true", we use the >> > + * alternative symbol table that maps L4 fields (tcp/udp/sctp >> ports) >> > + * to their CT equivalents. */ >> > + bool ct_trans = smap_get_bool(&lflow->tags, "acl_ct_translation", >> false); >> > + struct shash *symtab_to_use = ct_trans ? &acl_ct_symtab : &symtab; >> > + >> > + struct expr *e = expr_parse_string(lex_str_get(&match_s), >> symtab_to_use, >> > addr_sets, port_groups, >> &addr_sets_ref, >> > &port_groups_ref, >> > ldp->datapath->tunnel_key, >> > @@ -1033,7 +1050,7 @@ convert_match_to_expr(const struct >> sbrec_logical_flow *lflow, >> > e = expr_combine(EXPR_T_AND, e, *prereqs); >> > *prereqs = NULL; >> > } >> > - e = expr_annotate(e, &symtab, &error); >> > + e = expr_annotate(e, symtab_to_use, &error); >> > } >> > if (error) { >> > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); >> > @@ -2062,6 +2079,8 @@ lflow_destroy(void) >> > { >> > expr_symtab_destroy(&symtab); >> > shash_destroy(&symtab); >> > + expr_symtab_destroy(&acl_ct_symtab); >> > + shash_destroy(&acl_ct_symtab); >> > } >> > >> > bool >> > diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h >> > index a22e4cfb7..3c0fb22e7 100644 >> > --- a/include/ovn/logical-fields.h >> > +++ b/include/ovn/logical-fields.h >> > @@ -278,6 +278,8 @@ const char *event_to_string(enum >> ovn_controller_event event); >> > int string_to_event(const char *s); >> > const struct ovn_field *ovn_field_from_name(const char *name); >> > >> > +void ovn_init_acl_ct_symtab(struct shash *symtab); >> > + >> > /* OVN CT label values >> > * =================== >> > * These are specific ct.label bit values OVN uses to track different >> types >> > diff --git a/lib/logical-fields.c b/lib/logical-fields.c >> > index 601208cb9..9b04762a1 100644 >> > --- a/lib/logical-fields.c >> > +++ b/lib/logical-fields.c >> > @@ -451,3 +451,70 @@ ovn_field_from_name(const char *name) >> > >> > return shash_find_data(&ovnfield_by_name, name); >> > } >> > + >> > +/* Removes the symbol with 'name' from 'symtab', freeing its memory. */ >> > +static void >> > +expr_symtab_remove(struct shash *symtab, const char *name) >> > +{ >> > + struct expr_symbol *symbol = shash_find_and_delete(symtab, name); >> > + if (symbol) { >> > + free(symbol->name); >> > + free(symbol->prereqs); >> > + free(symbol->predicate); >> > + free(symbol); >> > + } >> > +} >> > + >> > +/* Initialize a symbol table for ACL CT translation. >> > + * This creates an alternative symbol table that maps L4 port fields >> > + * (tcp/udp/sctp) to their connection tracking equivalents. This allows >> > + * matching on all IP fragments (not just the first) by using CT state >> > + * which is available for all fragments. */ >> > +void >> > +ovn_init_acl_ct_symtab(struct shash *acl_symtab) >> > +{ >> > + /* Initialize with the standard symbol table. */ >> > + ovn_init_symtab(acl_symtab); >> > + >> > + /* Remove the original tcp/udp/sctp symbols that we will override. >> */ >> > + expr_symtab_remove(acl_symtab, "tcp.src"); >> > + expr_symtab_remove(acl_symtab, "tcp.dst"); >> > + expr_symtab_remove(acl_symtab, "tcp"); >> > + expr_symtab_remove(acl_symtab, "udp.src"); >> > + expr_symtab_remove(acl_symtab, "udp.dst"); >> > + expr_symtab_remove(acl_symtab, "udp"); >> > + expr_symtab_remove(acl_symtab, "sctp.src"); >> > + expr_symtab_remove(acl_symtab, "sctp.dst"); >> > + expr_symtab_remove(acl_symtab, "sctp"); >> > + >> > + /* Add ct_proto field - CT original direction protocol. Used in the >> > + * tcp/udp/sctp predicate expansions below. */ >> > + expr_symtab_add_field(acl_symtab, "ct_proto", MFF_CT_NW_PROTO, >> > + "ct.trk", false); >> > + >> > + /* Override TCP protocol and port fields to use CT equivalents. >> > + * When "tcp" is used as a predicate, it expands to "ct_proto == 6" >> > + * instead of "ip.proto == 6". */ >> > + expr_symtab_add_predicate(acl_symtab, "tcp", >> > + "ct.trk && !ct.inv && ct_proto == 6"); >> > + expr_symtab_add_field(acl_symtab, "tcp.src", MFF_CT_TP_SRC, >> > + "tcp", false); >> > + expr_symtab_add_field(acl_symtab, "tcp.dst", MFF_CT_TP_DST, >> > + "tcp", false); >> > + >> > + /* Override UDP protocol and port fields */ >> > + expr_symtab_add_predicate(acl_symtab, "udp", >> > + "ct.trk && !ct.inv && ct_proto == 17"); >> > + expr_symtab_add_field(acl_symtab, "udp.src", MFF_CT_TP_SRC, >> > + "udp", false); >> > + expr_symtab_add_field(acl_symtab, "udp.dst", MFF_CT_TP_DST, >> > + "udp", false); >> > + >> > + /* Override SCTP protocol and port fields */ >> > + expr_symtab_add_predicate(acl_symtab, "sctp", >> > + "ct.trk && !ct.inv && ct_proto == 132"); >> > + expr_symtab_add_field(acl_symtab, "sctp.src", MFF_CT_TP_SRC, >> > + "sctp", false); >> > + expr_symtab_add_field(acl_symtab, "sctp.dst", MFF_CT_TP_DST, >> > + "sctp", false); >> > +} >> > diff --git a/lib/ovn-util.c b/lib/ovn-util.c >> > index f34c37a79..63a68df68 100644 >> > --- a/lib/ovn-util.c >> > +++ b/lib/ovn-util.c >> > @@ -698,20 +698,24 @@ ovn_pipeline_from_name(const char *pipeline) >> > uint32_t >> > sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) >> > { >> > + bool acl_ct_translation = smap_get_bool(&lf->tags, >> > + "acl_ct_translation", >> false); >> > return ovn_logical_flow_hash(lf->table_id, >> > ovn_pipeline_from_name(lf->pipeline), >> > lf->priority, lf->match, >> > - lf->actions); >> > + lf->actions, acl_ct_translation); >> > } >> > >> > uint32_t >> > ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline, >> > uint16_t priority, >> > - const char *match, const char *actions) >> > + const char *match, const char *actions, >> > + bool acl_ct_translation) >> > { >> > size_t hash = hash_2words((table_id << 16) | priority, pipeline); >> > hash = hash_string(match, hash); >> > - return hash_string(actions, hash); >> > + hash = hash_string(actions, hash); >> > + return hash_boolean(acl_ct_translation, hash); >> > > nit: hash_boolean(...) doesn't call hash_finish, so we need > to swap the order with hash_string(...). > > >> > } >> > >> > >> > diff --git a/lib/ovn-util.h b/lib/ovn-util.h >> > index 0efeb217c..b882b8533 100644 >> > --- a/lib/ovn-util.h >> > +++ b/lib/ovn-util.h >> > @@ -155,7 +155,8 @@ enum ovn_pipeline { >> > uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *); >> > uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline >> pipeline, >> > uint16_t priority, >> > - const char *match, const char *actions); >> > + const char *match, const char *actions, >> > + bool acl_ct_translation); >> > void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED, >> > const char *argv[] OVS_UNUSED, void *idl_); >> > >> > diff --git a/northd/en-global-config.c b/northd/en-global-config.c >> > index 2556b2888..a6f37b0c3 100644 >> > --- a/northd/en-global-config.c >> > +++ b/northd/en-global-config.c >> > @@ -717,6 +717,11 @@ check_nb_options_out_of_sync( >> > return true; >> > } >> > >> > + if (config_out_of_sync(&nb->options, &config_data->nb_options, >> > + "acl_ct_translation", false)) { >> > + return true; >> > + } >> > + >> > return false; >> > } >> > >> > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c >> > index f21024903..a9811601f 100644 >> > --- a/northd/lflow-mgr.c >> > +++ b/northd/lflow-mgr.c >> > @@ -40,12 +40,14 @@ static void ovn_lflow_init(struct ovn_lflow *, >> > char *actions, char *io_port, >> > char *ctrl_meter, char *stage_hint, >> > const char *where, const char *flow_desc, >> > - struct uuid sbuuid); >> > + struct uuid sbuuid, bool >> acl_ct_translation); >> > static struct ovn_lflow *ovn_lflow_find(const struct hmap *lflows, >> > const struct ovn_stage *stage, >> > uint16_t priority, const char >> *match, >> > const char *actions, >> > - const char *ctrl_meter, >> uint32_t hash); >> > + const char *ctrl_meter, >> > + bool acl_ct_translation, >> > + uint32_t hash); >> > static void ovn_lflow_destroy(struct lflow_table *lflow_table, >> > struct ovn_lflow *lflow); >> > static char *ovn_lflow_hint(const struct ovsdb_idl_row *row); >> > @@ -56,7 +58,8 @@ static struct ovn_lflow *do_ovn_lflow_add( >> > const char *actions, const char *io_port, >> > const char *ctrl_meter, >> > const struct ovsdb_idl_row *stage_hint, >> > - const char *where, const char *flow_desc); >> > + const char *where, const char *flow_desc, >> > + bool acl_ct_translation); >> > >> > >> > static struct ovs_mutex *lflow_hash_lock(const struct hmap >> *lflow_table, >> > @@ -184,6 +187,7 @@ struct ovn_lflow { >> > struct ovn_dp_group *dpg; /* Link to unique Sb datapath group. >> */ >> > const char *where; >> > const char *flow_desc; >> > + bool acl_ct_translation; /* Use CT-based L4 field translation. >> */ >> > >> > struct uuid sb_uuid; /* SB DB row uuid, specified by >> northd. */ >> > struct ovs_list referenced_by; /* List of struct lflow_ref_node. >> */ >> > @@ -362,10 +366,12 @@ lflow_table_sync_to_sb(struct lflow_table >> *lflow_table, >> > struct ovn_stage stage = ovn_stage_build(dp_type, pipeline, >> > sbflow->table_id); >> > >> > + bool acl_ct_translation = smap_get_bool(&sbflow->tags, >> > + "acl_ct_translation", >> false); >> > lflow = ovn_lflow_find( >> > lflows, &stage, >> > sbflow->priority, sbflow->match, sbflow->actions, >> > - sbflow->controller_meter, sbflow->hash); >> > + sbflow->controller_meter, acl_ct_translation, >> sbflow->hash); >> > if (lflow) { >> > const struct ovn_synced_datapaths *datapaths; >> > struct hmap *dp_groups; >> > @@ -727,14 +733,15 @@ lflow_ref_sync_lflows(struct lflow_ref *lflow_ref, >> > */ >> > static void >> > lflow_table_add_lflow__(struct lflow_table *lflow_table, >> > - const struct ovn_synced_datapath *sdp, >> > - const unsigned long *dp_bitmap, size_t >> dp_bitmap_len, >> > - const struct ovn_stage *stage, uint16_t >> priority, >> > - const char *match, const char *actions, >> > - const char *io_port, const char *ctrl_meter, >> > - const struct ovsdb_idl_row *stage_hint, >> > - const char *where, const char *flow_desc, >> > - struct lflow_ref *lflow_ref) >> > + const struct ovn_synced_datapath *sdp, >> > + const unsigned long *dp_bitmap, size_t >> dp_bitmap_len, >> > + const struct ovn_stage *stage, uint16_t >> priority, >> > + const char *match, const char *actions, >> > + const char *io_port, const char *ctrl_meter, >> > + const struct ovsdb_idl_row *stage_hint, >> > + const char *where, const char *flow_desc, >> > + bool acl_ct_translation, >> >> Nit: to be more aligned with the order of arguments of other functions, >> I'd move 'acl_ct_translation' just after 'ctrl_meter'. >> >> > + struct lflow_ref *lflow_ref) >> > OVS_EXCLUDED(fake_hash_mutex) >> > { >> > struct ovs_mutex *hash_lock; >> > @@ -747,7 +754,7 @@ lflow_table_add_lflow__(struct lflow_table >> *lflow_table, >> > hash = ovn_logical_flow_hash(ovn_stage_get_table(stage), >> > ovn_stage_get_pipeline(stage), >> > priority, match, >> > - actions); >> > + actions, acl_ct_translation); >> > >> > hash_lock = lflow_hash_lock(&lflow_table->entries, hash); >> > struct ovn_lflow *lflow = >> > @@ -755,7 +762,8 @@ lflow_table_add_lflow__(struct lflow_table >> *lflow_table, >> > sdp ? sparse_array_len(&sdp->dps->dps_array) >> > : dp_bitmap_len, >> > hash, stage, priority, match, actions, >> > - io_port, ctrl_meter, stage_hint, where, >> flow_desc); >> > + io_port, ctrl_meter, stage_hint, where, >> flow_desc, >> > + acl_ct_translation); >> > >> > if (lflow_ref) { >> > struct lflow_ref_node *lrn = >> > @@ -815,7 +823,8 @@ lflow_table_add_lflow(struct lflow_table_add_args >> *args) >> > args->dp_bitmap_len, args->stage, >> args->priority, >> > args->match, args->actions, args->io_port, >> > args->ctrl_meter, args->stage_hint, >> args->where, >> > - args->flow_desc, args->lflow_ref); >> > + args->flow_desc, args->acl_ct_translation, >> > + args->lflow_ref); >> > } >> > >> > struct ovn_dp_group * >> > @@ -933,7 +942,8 @@ ovn_lflow_init(struct ovn_lflow *lflow, >> > size_t dp_bitmap_len, const struct ovn_stage *stage, >> > uint16_t priority, char *match, char *actions, char >> *io_port, >> > char *ctrl_meter, char *stage_hint, const char *where, >> > - const char *flow_desc, struct uuid sbuuid) >> > + const char *flow_desc, struct uuid sbuuid, >> > + bool acl_ct_translation) >> >> Nit: to be more aligned with the order of arguments of other functions, >> I'd move 'acl_ct_translation' just after 'stage_hint'. >> >> > { >> > dynamic_bitmap_alloc(&lflow->dpg_bitmap, dp_bitmap_len); >> > lflow->dp = dp; >> > @@ -949,6 +959,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, >> > lflow->where = where; >> > lflow->sb_uuid = sbuuid; >> > lflow->sync_state = LFLOW_TO_SYNC; >> > + lflow->acl_ct_translation = acl_ct_translation; >> > hmap_init(&lflow->dp_refcnts_map); >> > ovs_list_init(&lflow->referenced_by); >> > } >> > @@ -981,25 +992,28 @@ lflow_hash_unlock(struct ovs_mutex *hash_lock) >> > static bool >> > ovn_lflow_equal(const struct ovn_lflow *a, const struct ovn_stage >> *stage, >> > uint16_t priority, const char *match, >> > - const char *actions, const char *ctrl_meter) >> > + const char *actions, const char *ctrl_meter, >> > + bool acl_ct_translation) >> > { >> > return (ovn_stage_equal(a->stage, stage) >> > && a->priority == priority >> > && !strcmp(a->match, match) >> > && !strcmp(a->actions, actions) >> > - && nullable_string_is_equal(a->ctrl_meter, ctrl_meter)); >> > + && nullable_string_is_equal(a->ctrl_meter, ctrl_meter) >> > + && a->acl_ct_translation == acl_ct_translation); >> > } >> > >> > static struct ovn_lflow * >> > ovn_lflow_find(const struct hmap *lflows, >> > const struct ovn_stage *stage, uint16_t priority, >> > const char *match, const char *actions, >> > - const char *ctrl_meter, uint32_t hash) >> > + const char *ctrl_meter, bool acl_ct_translation, >> > + uint32_t hash) >> > { >> > struct ovn_lflow *lflow; >> > HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, hash, lflows) { >> > if (ovn_lflow_equal(lflow, stage, priority, match, actions, >> > - ctrl_meter)) { >> > + ctrl_meter, acl_ct_translation)) { >> > return lflow; >> > } >> > } >> > @@ -1039,7 +1053,8 @@ do_ovn_lflow_add(struct lflow_table *lflow_table, >> size_t dp_bitmap_len, >> > uint16_t priority, const char *match, const char >> *actions, >> > const char *io_port, const char *ctrl_meter, >> > const struct ovsdb_idl_row *stage_hint, >> > - const char *where, const char *flow_desc) >> > + const char *where, const char *flow_desc, >> > + bool acl_ct_translation) >> > OVS_REQUIRES(fake_hash_mutex) >> > { >> > struct ovn_lflow *old_lflow; >> > @@ -1049,7 +1064,8 @@ do_ovn_lflow_add(struct lflow_table *lflow_table, >> size_t dp_bitmap_len, >> > ovs_assert(dp_bitmap_len); >> > >> > old_lflow = ovn_lflow_find(&lflow_table->entries, stage, >> > - priority, match, actions, ctrl_meter, >> hash); >> > + priority, match, actions, ctrl_meter, >> > + acl_ct_translation, hash); >> > if (old_lflow) { >> > dynamic_bitmap_realloc(&old_lflow->dpg_bitmap, dp_bitmap_len); >> > if (old_lflow->sync_state != LFLOW_STALE) { >> > @@ -1068,7 +1084,7 @@ do_ovn_lflow_add(struct lflow_table *lflow_table, >> size_t dp_bitmap_len, >> > io_port ? xstrdup(io_port) : NULL, >> > nullable_xstrdup(ctrl_meter), >> > ovn_lflow_hint(stage_hint), where, >> > - flow_desc, sbuuid); >> > + flow_desc, sbuuid, acl_ct_translation); >> > >> > if (parallelization_state != STATE_USE_PARALLELIZATION) { >> > hmap_insert(&lflow_table->entries, &lflow->hmap_node, hash); >> > @@ -1122,12 +1138,22 @@ sync_lflow_to_sb(struct ovn_lflow *lflow, >> > sbrec_logical_flow_set_match(sbflow, lflow->match); >> > sbrec_logical_flow_set_actions(sbflow, lflow->actions); >> > sbrec_logical_flow_set_flow_desc(sbflow, lflow->flow_desc); >> > + >> > + struct smap tags = SMAP_INITIALIZER(&tags); >> > if (lflow->io_port) { >> > - struct smap tags = SMAP_INITIALIZER(&tags); >> > smap_add(&tags, "in_out_port", lflow->io_port); >> > + } >> > + >> > + if (lflow->acl_ct_translation) { >> > + smap_add(&tags, "acl_ct_translation", "true"); >> > + } >> > + >> > + if (!smap_is_empty(&tags)) { >> > sbrec_logical_flow_set_tags(sbflow, &tags); >> > - smap_destroy(&tags); >> > } >> > + >> > + smap_destroy(&tags); >> > + >> > sbrec_logical_flow_set_controller_meter(sbflow, >> lflow->ctrl_meter); >> > >> > /* Trim the source locator lflow->where, which looks something >> like >> > diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h >> > index 4a1655c35..84d0b3e67 100644 >> > --- a/northd/lflow-mgr.h >> > +++ b/northd/lflow-mgr.h >> > @@ -24,6 +24,7 @@ >> > struct ovsdb_idl_txn; >> > struct ovn_datapath; >> > struct ovsdb_idl_row; >> > +struct ovn_lflow; >> > >> > /* lflow map which stores the logical flows. */ >> > struct lflow_table { >> > @@ -87,12 +88,13 @@ struct lflow_table_add_args { >> > const char *flow_desc; >> > struct lflow_ref *lflow_ref; >> > const char *where; >> > + bool acl_ct_translation; >> > }; >> > >> > void lflow_table_add_lflow(struct lflow_table_add_args *args); >> > >> > - >> > #define WITH_HINT(HINT) .stage_hint = HINT >> > +#define WITH_CT_TRANSLATION(CT_TRANS) .acl_ct_translation = (CT_TRANS) >> > /* The IN_OUT_PORT argument tells the lport name that appears in the >> MATCH, >> > * which helps ovn-controller to bypass lflows parsing when the lport >> is >> > * not local to the chassis. The critiera of the lport to be added >> using this >> > diff --git a/northd/northd.c b/northd/northd.c >> > index 7fe36b850..937383e4b 100644 >> > --- a/northd/northd.c >> > +++ b/northd/northd.c >> > @@ -89,6 +89,12 @@ static bool use_common_zone = false; >> > * Otherwise, it will avoid using it. The default is true. */ >> > static bool use_ct_inv_match = true; >> > >> > +/* If this option is 'true' northd will flag the related ACL flows to >> use >> > + * connection tracking fields to properly handle IP fragments. By >> default this >> > + * option is set to 'false'. >> > + */ >> > +static bool acl_ct_translation = false; >> > + >> > /* If this option is 'true' northd will implicitly add a >> lowest-priority >> > * drop rule in the ACL stage of logical switches that have at least >> one >> > * ACL. >> > @@ -6306,11 +6312,19 @@ build_ls_stateful_rec_pre_acls( >> > "(udp && udp.src == 546 && udp.dst == 547)", >> "next;", >> > lflow_ref); >> > >> > - /* Do not send multicast packets to conntrack. */ >> > - ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, >> "eth.mcast", >> > - "next;", lflow_ref); >> > - ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, >> "eth.mcast", >> > - "next;", lflow_ref); >> > + /* Do not send multicast packets to conntrack unless ACL CT >> > + * translation is enabled. When translation is active, L4 port >> > + * fields are rewritten to their CT equivalents (e.g. udp.dst >> -> >> > + * ct_udp.dst), which requires ct.trk to be set. Skipping CT >> > + * for multicast would leave ct.trk unset and cause all >> > + * CT-translated ACL matches to fail for multicast traffic >> > + * (including DHCP). */ >> > + if (!acl_ct_translation) { >> > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, >> "eth.mcast", >> > + "next;", lflow_ref); >> > + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, >> "eth.mcast", >> > + "next;", lflow_ref); >> > + } >> > >> > /* Ingress and Egress Pre-ACL Table (Priority 100). >> > * >> > @@ -7286,6 +7300,11 @@ consider_acl(struct lflow_table *lflows, const >> struct ovn_datapath *od, >> > match_tier_len = match->length; >> > } >> > >> > + /* Check if this ACL needs CT translation for fragment handling. >> > + * All stateful ACLs are marked when the option is enabled; the >> actual >> > + * translation only affects L4 port fields in ovn-controller. */ >> > + bool needs_ct_trans = has_stateful && acl_ct_translation; >> > + >> > if (!has_stateful >> > || !strcmp(acl->action, "pass") >> > || !strcmp(acl->action, "allow-stateless")) { >> > @@ -7303,6 +7322,7 @@ consider_acl(struct lflow_table *lflows, const >> struct ovn_datapath *od, >> > >> > ds_put_cstr(actions, "next;"); >> > ds_put_format(match, "(%s)", acl->match); >> > + /* Stateless ACLs don't need CT translation. */ >> > ovn_lflow_add(lflows, od, stage, priority, ds_cstr(match), >> > ds_cstr(actions), lflow_ref, >> WITH_HINT(&acl->header_)); >> > return; >> > @@ -7372,7 +7392,9 @@ consider_acl(struct lflow_table *lflows, const >> struct ovn_datapath *od, >> > } >> > ds_put_cstr(actions, "next;"); >> > ovn_lflow_add(lflows, od, stage, priority, ds_cstr(match), >> > - ds_cstr(actions), lflow_ref, >> WITH_HINT(&acl->header_)); >> > + ds_cstr(actions), lflow_ref, >> > + WITH_HINT(&acl->header_), >> > + WITH_CT_TRANSLATION(needs_ct_trans)); >> > >> > /* Match on traffic in the request direction for an established >> > * connection tracking entry that has not been marked for >> > @@ -7402,7 +7424,9 @@ consider_acl(struct lflow_table *lflows, const >> struct ovn_datapath *od, >> > } >> > ds_put_cstr(actions, "next;"); >> > ovn_lflow_add(lflows, od, stage, priority, ds_cstr(match), >> > - ds_cstr(actions), lflow_ref, >> WITH_HINT(&acl->header_)); >> > + ds_cstr(actions), lflow_ref, >> > + WITH_HINT(&acl->header_), >> > + WITH_CT_TRANSLATION(needs_ct_trans)); >> > } else if (!strcmp(acl->action, "drop") >> > || !strcmp(acl->action, "reject")) { >> > if (acl->network_function_group) { >> > @@ -7428,7 +7452,9 @@ consider_acl(struct lflow_table *lflows, const >> struct ovn_datapath *od, >> > obs_stage); >> > ds_put_cstr(actions, "next;"); >> > ovn_lflow_add(lflows, od, stage, priority, ds_cstr(match), >> > - ds_cstr(actions), lflow_ref, >> WITH_HINT(&acl->header_)); >> > + ds_cstr(actions), lflow_ref, >> > + WITH_HINT(&acl->header_), >> > + WITH_CT_TRANSLATION(needs_ct_trans)); >> > /* For an existing connection without ct_mark.blocked set, >> we've >> > * encountered a policy change. ACLs previously allowed >> > * this connection and we committed the connection tracking >> > @@ -7454,7 +7480,9 @@ consider_acl(struct lflow_table *lflows, const >> struct ovn_datapath *od, >> > "ct_commit { ct_mark.blocked = 1; " >> > "ct_label.obs_point_id = %"PRIu32"; }; next;", >> obs_pid); >> > ovn_lflow_add(lflows, od, stage, priority, ds_cstr(match), >> > - ds_cstr(actions), lflow_ref, >> WITH_HINT(&acl->header_)); >> > + ds_cstr(actions), lflow_ref, >> > + WITH_HINT(&acl->header_), >> > + WITH_CT_TRANSLATION(needs_ct_trans)); >> > } >> > } >> > >> > @@ -20758,6 +20786,8 @@ ovnnb_db_run(struct northd_input *input_data, >> > >> > use_ct_inv_match = smap_get_bool(input_data->nb_options, >> > "use_ct_inv_match", true); >> > + acl_ct_translation = smap_get_bool(input_data->nb_options, >> > + "acl_ct_translation", false); >> > >> > /* deprecated, use --event instead */ >> > controller_event_en = smap_get_bool(input_data->nb_options, >> > diff --git a/ovn-nb.xml b/ovn-nb.xml >> > index ae37d03d1..9bb49c2ea 100644 >> > --- a/ovn-nb.xml >> > +++ b/ovn-nb.xml >> > @@ -327,6 +327,29 @@ >> > </p> >> > </column> >> > >> > + <column name="options" key="acl_ct_translation"> >> > + <p> >> > + If set to <code>true</code>, <code>ovn-northd</code> will >> enable >> > + connection tracking based L4 field translation for stateful >> ACLs. >> > + When enabled, ACL matches on L4 port fields (tcp/udp/sctp) >> use >> > + connection tracking state instead of packet headers. This >> allows >> > + proper handling of IP fragmentation in userspace datapaths >> (e.g., >> > + DPDK) where fragments are not automatically reassembled >> during >> > + connection tracking lookup. >> > + </p> >> > + <p> >> > + <em>Important:</em> Enabling this option breaks hardware >> offloading >> >> Nit: s/breaks/may break/ >> >> > + of flows. It also disables the multicast conntrack bypass, >> which >> > + means multicast traffic (including DHCP) will go through >> connection >> > + tracking. This may have a performance impact on >> multicast-heavy >> > + workloads. Only enable this option if you need to handle >> fragmented >> > + traffic with stateful ACLs in userspace datapaths. >> > + </p> >> > + <p> >> > + The default value is <code>false</code>. >> > + </p> >> > + </column> >> > + >> > <column name="options" key="default_acl_drop"> >> > <p> >> > If set to <code>true</code>., <code>ovn-northd</code> will >> > diff --git a/tests/ovn.at b/tests/ovn.at >> > index 3a3b05ab7..ff3dbd6bc 100644 >> > --- a/tests/ovn.at >> > +++ b/tests/ovn.at >> > @@ -45700,3 +45700,78 @@ AT_CHECK([grep -q 'name="eth-unknown"' >> hv1/ovn-controller.log], [1]) >> > OVN_CLEANUP([hv1]) >> > AT_CLEANUP >> > ]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ACL CT translation - Rules]) >> > +AT_KEYWORDS([acl_ct_translation_rules]) >> > +ovn_start >> > + >> > +check ovn-nbctl ls-add ls1 >> > + >> > +check ovn-nbctl lsp-add ls1 lp1 \ >> > + -- lsp-set-addresses lp1 "f0:00:00:00:00:01 10.0.0.1" >> > +check ovn-nbctl lsp-add ls1 lp2 \ >> > + -- lsp-set-addresses lp2 "f0:00:00:00:00:02 10.0.0.2" >> > + >> > +net_add n1 >> > +sim_add hv1 >> > + >> > +as hv1 >> > +check ovs-vsctl add-br br-phys >> > +ovn_attach n1 br-phys 192.168.0.1 >> > +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ >> > + set interface hv1-vif1 external-ids:iface-id=lp1 >> > +check ovs-vsctl -- add-port br-int hv1-vif2 -- \ >> > + set interface hv1-vif2 external-ids:iface-id=lp2 >> > + >> > +# Get the OF table numbers for ACL evaluation stages. >> > +acl_in_eval=$(ovn-debug lflow-stage-to-oftable ls_in_acl_eval) >> > +acl_out_eval=$(ovn-debug lflow-stage-to-oftable ls_out_acl_eval) >> > + >> > +# Create port group and add ACLs with TCP/UDP matches. >> > +lp1_uuid=$(fetch_column nb:logical_switch_port _uuid name=lp1) >> > +lp2_uuid=$(fetch_column nb:logical_switch_port _uuid name=lp2) >> > +check ovn-nbctl pg-add pg1 $lp1_uuid $lp2_uuid >> > + >> > +check ovn-nbctl acl-add pg1 from-lport 1002 \ >> > + "inport == @pg1 && ip4 && tcp && tcp.dst == 80" allow-related >> > +check ovn-nbctl acl-add pg1 from-lport 1002 \ >> > + "inport == @pg1 && ip4 && udp && udp.dst == 53" allow-related >> > +check ovn-nbctl acl-add pg1 to-lport 1002 \ >> > + "outport == @pg1 && ip4 && tcp && tcp.src == 80" allow-related >> > +check ovn-nbctl acl-add pg1 to-lport 1002 \ >> > + "outport == @pg1 && ip4 && udp && udp.src == 53" allow-related >> > +check ovn-nbctl --wait=hv --log acl-add pg1 to-lport 100 "outport == >> @pg1" drop >> > + >> > +# Verify lflows do NOT have acl_ct_translation tag when option is >> disabled (default). >> > +check_row_count Logical_Flow 0 'tags:acl_ct_translation="true"' >> > + >> > +# Verify OpenFlows use packet header fields (tcp.dst, udp.dst) not CT >> fields. >> > +# When CT translation is OFF, we should see tp_dst matches in ACL >> tables. >> > +as hv1 >> > +AT_CHECK([ovs-ofctl dump-flows br-int table=$acl_in_eval | grep -q >> "tp_dst=80"], [0]) >> > + >> > +# Now enable ACL CT translation. >> > +check ovn-nbctl --wait=hv set NB_Global . >> options:acl_ct_translation=true >> > + >> > +# Verify lflows now have acl_ct_translation tag in the tags column. >> > +wait_row_count Logical_Flow 10 'tags:acl_ct_translation="true"' >> > + >> > +# Verify OpenFlows now use CT fields (ct_tp_dst) instead of packet >> headers. >> > +# When CT translation is ON, we should see ct_tp_dst matches in ACL >> tables. >> > +as hv1 >> > +AT_CHECK([ovs-ofctl dump-flows br-int table=$acl_in_eval | grep -q >> "ct_tp_dst=80"], [0]) >> > + >> > +# Now disable ACL CT translation again. >> > +check ovn-nbctl --wait=hv set NB_Global . >> options:acl_ct_translation=false >> > + >> > +# Verify lflows no longer have acl_ct_translation tag. >> > +wait_row_count Logical_Flow 0 'tags:acl_ct_translation="true"' >> > + >> > +# Verify OpenFlows reverted to packet header fields in ACL tables. >> > +as hv1 >> > +AT_CHECK([ovs-ofctl dump-flows br-int table=$acl_in_eval | grep -q >> "tp_dst=80"], [0]) >> > + >> > +OVN_CLEANUP([hv1]) >> > +AT_CLEANUP >> > +]) >> > diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> > index a478dd709..f2c8cdb65 100644 >> > --- a/tests/system-ovn.at >> > +++ b/tests/system-ovn.at >> > @@ -18,7 +18,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -190,7 +190,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -363,7 +363,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -468,7 +468,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -573,7 +573,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -788,7 +788,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -1007,7 +1007,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -1314,7 +1314,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -1588,7 +1588,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -2044,7 +2044,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -2145,7 +2145,7 @@ ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > >> > -# Start ovn-controller >> > +# Start ovn-controller. >> > start_daemon ovn-controller >> > >> > # Logical network: >> > @@ -7296,7 +7296,7 @@ ADD_BR([br-int]) >> > ADD_BR([br-ext]) >> > >> > check ovs-ofctl add-flow br-ext action=normal >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -7838,7 +7838,7 @@ ADD_BR([br-int]) >> > ADD_BR([br-ext]) >> > >> > check ovs-ofctl add-flow br-ext action=normal >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -10463,7 +10463,7 @@ ovn_start >> > OVS_TRAFFIC_VSWITCHD_START() >> > ADD_BR([br-int]) >> > >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -11718,7 +11718,7 @@ ovn_start >> > OVS_TRAFFIC_VSWITCHD_START() >> > ADD_BR([br-int]) >> > >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -12570,7 +12570,7 @@ ADD_BR([br-int]) >> > ADD_BR([br-ext]) >> > >> > check ovs-ofctl add-flow br-ext action=normal >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -12793,7 +12793,7 @@ ADD_BR([br-int]) >> > ADD_BR([br-ext]) >> > >> > check ovs-ofctl add-flow br-ext action=normal >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -12919,7 +12919,7 @@ ADD_BR([br-int]) >> > ADD_BR([br-ext]) >> > >> > check ovs-ofctl add-flow br-ext action=normal >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -13946,7 +13946,7 @@ ADD_BR([br-int]) >> > ADD_BR([br-ext]) >> > >> > check ovs-ofctl add-flow br-ext action=normal >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -14085,7 +14085,7 @@ ADD_BR([br-int]) >> > ADD_BR([br-ext]) >> > >> > check ovs-ofctl add-flow br-ext action=normal >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -14986,7 +14986,7 @@ ovn_start >> > OVS_TRAFFIC_VSWITCHD_START() >> > ADD_BR([br-int]) >> > >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -17750,7 +17750,7 @@ ovn_start >> > OVS_TRAFFIC_VSWITCHD_START() >> > ADD_BR([br-int]) >> > >> > -# Set external-ids in br-int needed for ovn-controller >> > +# Set external-ids in br-int needed for ovn-controller. >> > check ovs-vsctl \ >> > -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > @@ -20997,3 +20997,399 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query >> port patch-.*/d >> > /connection dropped.*/d"]) >> > AT_CLEANUP >> > ]) >> >> While I appreciate the comment fixes above, they should be in a separate >> patch as they're not related to the ACL patch and would make backporting >> it even harder than it might be. >> >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ACL CT translation - UDP fragmentation]) >> > +AT_KEYWORDS([acl_ct_translation_udp_fragmentation]) >> > +AT_SKIP_IF([test $HAVE_TCPDUMP = no]) >> > + >> > +CHECK_CONNTRACK() >> > + >> > +ovn_start >> > +OVS_TRAFFIC_VSWITCHD_START() >> > +ADD_BR([br-int]) >> > + >> > +# Set external-ids in br-int needed for ovn-controller >> > +check ovs-vsctl \ >> > + -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > + -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > + -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > + >> > +# Start ovn-controller. >> > +start_daemon ovn-controller >> > + >> > +# Set the minimal fragment size for userspace DP. >> > +ovs-appctl dpctl/ipf-set-min-frag v4 500 >> > + >> > +check ovn-nbctl ls-add internal >> > +check ovn-nbctl lsp-add internal client \ >> > + -- lsp-set-addresses client "f0:00:00:01:02:03 172.16.1.3" >> > +check ovn-nbctl lsp-add internal server \ >> > + -- lsp-set-addresses server "f0:00:0f:01:02:03 172.16.1.2" >> > + >> > +ADD_NAMESPACES(client) >> > +ADD_VETH(client, client, br-int, "172.16.1.3/24", >> "f0:00:00:01:02:03", \ >> > + "172.16.1.1") >> > +NS_EXEC([client], [ip l set dev client mtu 900]) >> > + >> > +ADD_NAMESPACES(server) >> > +ADD_VETH(server, server, br-int, "172.16.1.2/24", >> "f0:00:0f:01:02:03", \ >> > + "172.16.1.1") >> > +NS_EXEC([server], [ip l set dev server mtu 900]) >> > + >> > +# Create data file for traffic testing (8000 bytes will be fragmented >> with MTU 900). >> > +printf %8000s > datafile >> > + >> > +# Create port group with both client and server. >> > +client_uuid=$(fetch_column nb:logical_switch_port _uuid name=client) >> > +server_uuid=$(fetch_column nb:logical_switch_port _uuid name=server) >> > +check ovn-nbctl pg-add internal_vms $client_uuid $server_uuid >> > + >> > +# ACL rules - allow outgoing traffic. >> > +check ovn-nbctl acl-add internal_vms from-lport 1002 "inport == >> @internal_vms && ip4 && udp" allow-related >> > +check ovn-nbctl acl-add internal_vms from-lport 1002 "inport == >> @internal_vms && ip4" allow-related >> > + >> > +# ACL rules - allow incoming UDP on specific ports. >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && ip4 && udp && udp.dst == 5060" allow-related >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && ip4 && udp && udp.dst == 5061" allow-related >> > + >> > +# Allow ARP. >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && arp" allow-related >> > + >> > +# Drop rule. >> > +check ovn-nbctl --log --severity=info acl-add internal_vms to-lport >> 100 "outport == @internal_vms" drop >> > + >> > +# Enable ACL CT translation for fragmentation handling. >> > +check ovn-nbctl --wait=hv set NB_Global . >> options:acl_ct_translation=true >> > + >> > +check ovn-nbctl --wait=hv sync >> > +check ovs-appctl dpctl/flush-conntrack >> > + >> > +# Start tcpdump to capture IP fragments on both sides. >> > +NETNS_START_TCPDUMP([server], [-U -i server -Q in -nn ip and >> '(ip[[6:2]] & 0x3fff != 0)'], [tcpdump-udp-server]) >> > +NETNS_START_TCPDUMP([client], [-U -i client -Q in -nn ip and >> '(ip[[6:2]] & 0x3fff != 0)'], [tcpdump-udp-client]) >> > + >> > +# Start UDP listeners on both sides. >> > +NETNS_DAEMONIZE([server], [nc -l -u 172.16.1.2 5060 > >> udp_server.rcvd], [server.pid]) >> > +NETNS_DAEMONIZE([client], [nc -l -u 172.16.1.3 5061 > >> udp_client.rcvd], [client.pid]) >> > + >> > +# Client sends to server (will be fragmented due to MTU 900). >> > +NS_CHECK_EXEC([client], [cat datafile | nc -w 1 -u 172.16.1.2 5060], >> [0], [ignore], [ignore]) >> > + >> > +# Server sends to client (will be fragmented due to MTU 900). >> > +NS_CHECK_EXEC([server], [cat datafile | nc -w 1 -u 172.16.1.3 5061], >> [0], [ignore], [ignore]) >> > + >> > +OVS_WAIT_UNTIL([test -s udp_server.rcvd]) >> > +OVS_WAIT_UNTIL([test -s udp_client.rcvd]) >> > + >> > +# Verify both sides received data. >> > +check cmp datafile udp_server.rcvd >> > +check cmp datafile udp_client.rcvd >> > + >> > +# Verify IP fragments were received on both sides (at least 5 >> fragments confirms fragmentation). >> > +OVS_WAIT_UNTIL([test $(cat tcpdump-udp-server.tcpdump | wc -l) -ge 5]) >> > +OVS_WAIT_UNTIL([test $(cat tcpdump-udp-client.tcpdump | wc -l) -ge 5]) >> > + >> > +OVN_CLEANUP_CONTROLLER([hv1]) >> > +OVN_CLEANUP_NORTHD >> > + >> > +as >> > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> > +/connection dropped.*/d >> > +/WARN|netdev@ovs-netdev: execute.*/d >> > +/dpif|WARN|system@ovs-system: execute.*/d >> > +"]) >> > +AT_CLEANUP >> > +]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ACL CT translation - TCP traffic]) >> > +AT_KEYWORDS([acl_ct_translation_tcp_fragmentation]) >> > + >> > +CHECK_CONNTRACK() >> > + >> > +ovn_start >> > +OVS_TRAFFIC_VSWITCHD_START() >> > +ADD_BR([br-int]) >> > + >> > +# Set external-ids in br-int needed for ovn-controller >> > +check ovs-vsctl \ >> > + -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > + -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > + -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > + >> > +# Start ovn-controller >> > +start_daemon ovn-controller >> > + >> > +check ovn-nbctl ls-add internal >> > +check ovn-nbctl lsp-add internal client \ >> > + -- lsp-set-addresses client "f0:00:00:01:02:03 172.16.1.3" >> > +check ovn-nbctl lsp-add internal server \ >> > + -- lsp-set-addresses server "f0:00:0f:01:02:03 172.16.1.2" >> > + >> > +ADD_NAMESPACES(client) >> > +ADD_VETH(client, client, br-int, "172.16.1.3/24", >> "f0:00:00:01:02:03", \ >> > + "172.16.1.1") >> > + >> > +ADD_NAMESPACES(server) >> > +ADD_VETH(server, server, br-int, "172.16.1.2/24", >> "f0:00:0f:01:02:03", \ >> > + "172.16.1.1") >> > + >> > +# Create data file for traffic testing. >> > +printf %8000s > datafile >> > + >> > +# Create port group with both client and server. >> > +client_uuid=$(fetch_column nb:logical_switch_port _uuid name=client) >> > +server_uuid=$(fetch_column nb:logical_switch_port _uuid name=server) >> > +check ovn-nbctl pg-add internal_vms $client_uuid $server_uuid >> > + >> > +# ACL rules - allow outgoing traffic. >> > +check ovn-nbctl acl-add internal_vms from-lport 1002 "inport == >> @internal_vms && ip4 && tcp" allow-related >> > +check ovn-nbctl acl-add internal_vms from-lport 1002 "inport == >> @internal_vms && ip4" allow-related >> > + >> > +# ACL rules - allow incoming TCP on specific ports. >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && ip4 && tcp && tcp.dst == 8080" allow-related >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && ip4 && tcp && tcp.dst == 8081" allow-related >> > + >> > +# Allow ARP. >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && arp" allow-related >> > + >> > +# Drop rule. >> > +check ovn-nbctl --log --severity=info acl-add internal_vms to-lport >> 100 "outport == @internal_vms" drop >> > + >> > +# Enable ACL CT translation. >> > +check ovn-nbctl --wait=hv set NB_Global . >> options:acl_ct_translation=true >> > + >> > +check ovn-nbctl --wait=hv sync >> > +check ovs-appctl dpctl/flush-conntrack >> > + >> > +# Test client -> server direction. >> > +NETNS_DAEMONIZE([server], [nc -l 172.16.1.2 8080 > tcp_server.rcvd], >> [server.pid]) >> > +NS_CHECK_EXEC([client], [nc -w 1 172.16.1.2 8080 < datafile], [0], >> [ignore], [ignore]) >> > + >> > +OVS_WAIT_UNTIL([test -s tcp_server.rcvd]) >> > +check cmp datafile tcp_server.rcvd >> > + >> > +# Clean up first direction. >> > +kill $(cat server.pid) 2>/dev/null || true >> > + >> > +# Test server -> client direction. >> > +NETNS_DAEMONIZE([client], [nc -l 172.16.1.3 8081 > tcp_client.rcvd], >> [client.pid]) >> > +NS_CHECK_EXEC([server], [nc -w 1 172.16.1.3 8081 < datafile], [0], >> [ignore], [ignore]) >> > + >> > +OVS_WAIT_UNTIL([test -s tcp_client.rcvd]) >> > +check cmp datafile tcp_client.rcvd >> > + >> > +OVN_CLEANUP_CONTROLLER([hv1]) >> > +OVN_CLEANUP_NORTHD >> > + >> > +as >> > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> > +/connection dropped.*/d >> > +"]) >> > +AT_CLEANUP >> > +]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ACL CT translation - negative test]) >> > +AT_KEYWORDS([acl_ct_translation_udp_fragmentation_negative]) >> > +AT_SKIP_IF([test $HAVE_TCPDUMP = no]) >> > + >> > +CHECK_CONNTRACK() >> > + >> > +ovn_start >> > +OVS_TRAFFIC_VSWITCHD_START() >> > +ADD_BR([br-int]) >> > + >> > +# Set external-ids in br-int needed for ovn-controller >> > +check ovs-vsctl \ >> > + -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > + -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > + -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > + >> > +# Start ovn-controller >> > +start_daemon ovn-controller >> > + >> > +check ovn-nbctl ls-add internal >> > +check ovn-nbctl lsp-add internal client \ >> > + -- lsp-set-addresses client "f0:00:00:01:02:03 172.16.1.3" >> > +check ovn-nbctl lsp-add internal server \ >> > + -- lsp-set-addresses server "f0:00:0f:01:02:03 172.16.1.2" >> > + >> > +ADD_NAMESPACES(client) >> > +ADD_VETH(client, client, br-int, "172.16.1.3/24", >> "f0:00:00:01:02:03", \ >> > + "172.16.1.1") >> > + >> > +ADD_NAMESPACES(server) >> > +ADD_VETH(server, server, br-int, "172.16.1.2/24", >> "f0:00:0f:01:02:03", \ >> > + "172.16.1.1") >> > + >> > +# Create port group with both client and server. >> > +client_uuid=$(fetch_column nb:logical_switch_port _uuid name=client) >> > +server_uuid=$(fetch_column nb:logical_switch_port _uuid name=server) >> > +check ovn-nbctl pg-add internal_vms $client_uuid $server_uuid >> > + >> > +# ACL rules - allow outgoing traffic. >> > +check ovn-nbctl acl-add internal_vms from-lport 1002 "inport == >> @internal_vms && ip4 && udp" allow-related >> > +check ovn-nbctl acl-add internal_vms from-lport 1002 "inport == >> @internal_vms && ip4" allow-related >> > + >> > +# ACL rules - allow incoming UDP ONLY on port 5060 (NOT 4000). >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && ip4 && udp && udp.dst == 5060" allow-related >> > + >> > +# Allow ARP. >> > +check ovn-nbctl acl-add internal_vms to-lport 1002 "outport == >> @internal_vms && arp" allow-related >> > + >> > +# Drop rule - this should drop traffic on port 4000. >> > +check ovn-nbctl --log --severity=info acl-add internal_vms to-lport >> 100 "outport == @internal_vms" drop >> > + >> > +# Enable ACL CT translation. >> > +check ovn-nbctl --wait=hv set NB_Global . >> options:acl_ct_translation=true >> > + >> > +check ovn-nbctl --wait=hv sync >> > +check ovs-appctl dpctl/flush-conntrack >> > + >> > +# Start tcpdump to capture any incoming packets on server. >> > +NETNS_START_TCPDUMP([server], [-U -i server -Q in -nn udp port 4000], >> [tcpdump-drop-server]) >> > + >> > +# Start UDP listener on server (on port 4000 which is NOT allowed by >> ACLs). >> > +NETNS_DAEMONIZE([server], [nc -l -u 172.16.1.2 4000 > udp_drop.rcvd], >> [drop_server.pid]) >> > + >> > +# Client sends to server on disallowed port. >> > +NS_CHECK_EXEC([client], [echo "test" | nc -w 1 -u 172.16.1.2 4000], >> [0], [ignore], [ignore]) >> > + >> > +# Wait a bit for any packets to arrive. >> > +sleep 2 >> > + >> > +# Verify server did NOT receive any data (file should be empty or not >> exist). >> > +AT_CHECK([test ! -s udp_drop.rcvd], [0]) >> > + >> > +# Verify tcpdump captured no packets (traffic was dropped by ACL). >> > +AT_CHECK([test $(cat tcpdump-drop-server.tcpdump | wc -l) -eq 0], [0]) >> > + >> > +OVN_CLEANUP_CONTROLLER([hv1]) >> > +OVN_CLEANUP_NORTHD >> > + >> > +as >> > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> > +/connection dropped.*/d >> > +"]) >> > +AT_CLEANUP >> > +]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ACL CT translation with DHCP traffic]) >> > +AT_KEYWORDS([acl_ct_translation_dhcp]) >> > +AT_SKIP_IF([test $HAVE_DHCPD = no]) >> > +AT_SKIP_IF([test $HAVE_DHCLIENT = no]) >> > + >> > +CHECK_CONNTRACK() >> > + >> > +ovn_start >> > +OVS_TRAFFIC_VSWITCHD_START() >> > +ADD_BR([br-int]) >> > + >> > +# Set external-ids in br-int needed for ovn-controller >> > +check ovs-vsctl \ >> > + -- set Open_vSwitch . external-ids:system-id=hv1 \ >> > + -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ >> > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> > + -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> > + >> > +# Start ovn-controller >> > +start_daemon ovn-controller >> > + >> > +# Create logical switch. >> > +check ovn-nbctl ls-add ls1 >> > + >> > +# Create DHCP server port. >> > +check ovn-nbctl lsp-add ls1 dhcp-server \ >> > + -- lsp-set-addresses dhcp-server "00:00:00:00:02:00 192.168.1.2" >> > + >> > +# Create DHCP client port. >> > +check ovn-nbctl lsp-add ls1 dhcp-client \ >> > + -- lsp-set-addresses dhcp-client "00:00:00:00:02:01" >> > + >> > +# Add OVS ports. >> > +ADD_NAMESPACES(server, client) >> > +ADD_VETH(server, server, br-int, "192.168.1.2/24", >> "00:00:00:00:02:00") >> > +ADD_VETH(client, client, br-int, "0.0.0.0/0", "00:00:00:00:02:01") >> > + >> > +# Bind logical ports to OVS ports. >> > +check ovs-vsctl set Interface ovs-server >> external_ids:iface-id=dhcp-server >> > +check ovs-vsctl set Interface ovs-client >> external_ids:iface-id=dhcp-client >> > + >> > +# Create port group with both ports. >> > +server_uuid=$(fetch_column nb:logical_switch_port _uuid >> name=dhcp-server) >> > +client_uuid=$(fetch_column nb:logical_switch_port _uuid >> name=dhcp-client) >> > +check ovn-nbctl pg-add dhcp_vms $server_uuid $client_uuid >> > + >> > +# Add ACL rules - allow outgoing traffic from both ports. >> > +check ovn-nbctl acl-add dhcp_vms from-lport 1002 "inport == @dhcp_vms >> && ip4" allow-related >> > +check ovn-nbctl acl-add dhcp_vms from-lport 1002 "inport == @dhcp_vms >> && udp" allow-related >> > + >> > +# Add ACL rules for DHCP traffic (ports 67 and 68). >> > +check ovn-nbctl acl-add dhcp_vms to-lport 1002 "outport == @dhcp_vms >> && ip4 && udp && udp.dst == 67" allow-related >> > +check ovn-nbctl acl-add dhcp_vms to-lport 1002 "outport == @dhcp_vms >> && ip4 && udp && udp.dst == 68" allow-related >> > + >> > +# Allow ARP and broadcast. >> > +check ovn-nbctl acl-add dhcp_vms to-lport 1002 "outport == @dhcp_vms >> && arp" allow-related >> > + >> > +# Add drop rule. >> > +check ovn-nbctl --log --severity=info acl-add dhcp_vms to-lport 100 >> "outport == @dhcp_vms" drop >> > + >> > +# Enable ACL CT translation. >> > +check ovn-nbctl --wait=hv set NB_Global . >> options:acl_ct_translation=true >> > + >> > +# Wait for flows to be installed. >> > +check ovn-nbctl --wait=hv sync >> > + >> > +# Setup DHCP server configuration. >> > +DHCP_TEST_DIR="$ovs_base/dhcp-test" >> > +mkdir -p $DHCP_TEST_DIR >> > + >> > +cat > $DHCP_TEST_DIR/dhcpd.conf <<EOF >> > +subnet 192.168.1.0 netmask 255.255.255.0 { >> > + range 192.168.1.100 192.168.1.100; >> > + option routers 192.168.1.2; >> > + option broadcast-address 192.168.1.255; >> > + default-lease-time 60; >> > + max-lease-time 120; >> > +} >> > +EOF >> > + >> > +touch $DHCP_TEST_DIR/dhcpd.leases >> > +chown root:dhcpd $DHCP_TEST_DIR $DHCP_TEST_DIR/dhcpd.leases >> > +chmod 775 $DHCP_TEST_DIR >> > +chmod 664 $DHCP_TEST_DIR/dhcpd.leases >> > + >> > +# Start dhcpd as DHCP server in the server namespace. >> > +NETNS_DAEMONIZE([server], [dhcpd -4 -f -cf $DHCP_TEST_DIR/dhcpd.conf >> server > $DHCP_TEST_DIR/dhcpd.log 2>&1], [dhcpd.pid]) >> > + >> > +# Give dhcpd time to start. >> > +sleep 1 >> > + >> > +# Request IP via DHCP using dhclient. >> > +NS_CHECK_EXEC([client], [dhclient -1 -v -lf >> $DHCP_TEST_DIR/dhclient.lease -pf $DHCP_TEST_DIR/dhclient.pid client], [0], >> [ignore], [ignore]) >> > +# Register cleanup handler to kill dhclient when test exits. >> > +on_exit 'kill $(cat $DHCP_TEST_DIR/dhclient.pid) 2>/dev/null || true' >> > + >> > +# Verify client got an IP address from DHCP. >> > +NS_CHECK_EXEC([client], [ip addr show client | grep -q >> "192.168.1.100"], [0]) >> > + >> > +OVN_CLEANUP_CONTROLLER([hv1]) >> > + >> > +OVN_CLEANUP_NORTHD >> > + >> > +as >> > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> > +/connection dropped.*/d >> > +"]) >> > +AT_CLEANUP >> > +]) >> >> > I addressed the nits, added "Reported-at: > https://issues.redhat.com/browse/FDP-1992"; for our internal tracking and > merged this into main. > > Regards, > Ales > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
