| Message ID | 20250414085122.348614-2-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Avoid non-offloadable ct_state.dnat matches. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Apr 14, 2025 at 10:52 AM Dumitru Ceara <dceara@redhat.com> wrote: > The new action stores the 8 relevant bits of the OVS ct_state field into > a register. This is implemented through an additional OpenFlow table, > OFTABLE_CT_STATE_SAVE, that contains two (static) rules: > - high-priority, set the target register to 0 if ct_state -trk is set > - low-priority, copy the 8 bits ct_state to the target register > otherwise > > From a logical flow perspective, the action is supposed to be used in > the following way: > R = ct_state_save(); > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > V2: > - Addressed Ales' comment: > - properly set the saved ct state register if ct_state includes +trk > - Fixed up output of ovn-trace for ct_state_save() to include the actual > value that gets copied to the destination register. > --- > controller/lflow.c | 1 + > controller/lflow.h | 1 + > controller/physical.c | 23 +++++++++++++++++++++++ > include/ovn/actions.h | 3 +++ > include/ovn/logical-fields.h | 2 ++ > lib/actions.c | 27 +++++++++++++++++++++++++++ > lib/ovn-util.c | 2 +- > ovn-sb.xml | 8 ++++++++ > tests/ovn-macros.at | 1 + > tests/ovn.at | 28 ++++++++++++++++++++++++++-- > tests/test-ovn.c | 1 + > utilities/ovn-trace.c | 21 +++++++++++++++++++++ > 12 files changed, 115 insertions(+), 3 deletions(-) > > diff --git a/controller/lflow.c b/controller/lflow.c > index 196f6a21ac..c24b2e2127 100644 > --- a/controller/lflow.c > +++ b/controller/lflow.c > @@ -893,6 +893,7 @@ add_matches_to_flow_table(const struct > sbrec_logical_flow *lflow, > .ct_ip6_dst_load_table = OFTABLE_CT_ORIG_IP6_DST_LOAD, > .ct_tp_dst_load_table = OFTABLE_CT_ORIG_TP_DST_LOAD, > .flood_remote_table = OFTABLE_FLOOD_REMOTE_CHASSIS, > + .ct_state_save_table = OFTABLE_CT_STATE_SAVE, > .ctrl_meter_id = ctrl_meter_id, > .common_nat_ct_zone = get_common_nat_zone(ldp), > }; > diff --git a/controller/lflow.h b/controller/lflow.h > index fa99173e67..3fd7ca99f5 100644 > --- a/controller/lflow.h > +++ b/controller/lflow.h > @@ -99,6 +99,7 @@ struct uuid; > #define OFTABLE_CT_ORIG_IP6_DST_LOAD 82 > #define OFTABLE_CT_ORIG_TP_DST_LOAD 83 > #define OFTABLE_FLOOD_REMOTE_CHASSIS 84 > +#define OFTABLE_CT_STATE_SAVE 85 > > /* Common defines shared between some controller components. */ > #define CHASSIS_FLOOD_INDEX_START 0x8000 > diff --git a/controller/physical.c b/controller/physical.c > index 9e797b3b3c..b4f91b134d 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -3085,6 +3085,29 @@ physical_run(struct physical_ctx *p_ctx, > ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_IP6_DST_LOAD, 100, 0, > &match, > &ofpacts, hc_uuid); > > + /* Implement the ct_state_save() logical action. */ > + > + /* Add the flow: > + * match = (1), action = (reg4[0..7] = ct_state[0..7]) > + * table = 85, priority = UINT16_MAX - 1 > + */ > + match_init_catchall(&match); > + ofpbuf_clear(&ofpacts); > + put_move(MFF_CT_STATE, 0, MFF_LOG_CT_SAVED_STATE, 0, 8, &ofpacts); > + ofctrl_add_flow(flow_table, OFTABLE_CT_STATE_SAVE, UINT16_MAX - 1, 0, > + &match, &ofpacts, hc_uuid); > + > + /* Add the flow: > + * match = (!ct.trk), action = (reg4[0..7] = 0) > + * table = 85, priority = UINT16_MAX > + */ > + match_init_catchall(&match); > + ofpbuf_clear(&ofpacts); > + match_set_ct_state_masked(&match, 0, OVS_CS_F_TRACKED); > + put_load(0, MFF_LOG_CT_SAVED_STATE, 0, 8, &ofpacts); > + ofctrl_add_flow(flow_table, OFTABLE_CT_STATE_SAVE, UINT16_MAX, 0, > + &match, &ofpacts, hc_uuid); > + > physical_eval_remote_chassis_flows(p_ctx, &ofpacts, flow_table); > > ofpbuf_uninit(&ofpacts); > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 0993ad0e5d..7fd7b26bfe 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -135,6 +135,7 @@ struct collector_set_ids; > OVNACT(CT_ORIG_IP6_DST, ovnact_result) \ > OVNACT(CT_ORIG_TP_DST, ovnact_result) \ > OVNACT(FLOOD_REMOTE, ovnact_null) \ > + OVNACT(CT_STATE_SAVE, ovnact_result) > > /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ > enum OVS_PACKED_ENUM ovnact_type { > @@ -964,6 +965,8 @@ struct ovnact_encode_params { > * to resubmit. */ > uint32_t flood_remote_table; /* OpenFlow table for 'chassis_flood' > * to resubmit. */ > + uint32_t ct_state_save_table; /* OpenFlow table for saving ct_state to > + * register. */ > }; > > void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, > diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h > index 7c6be6ea42..17154d38bd 100644 > --- a/include/ovn/logical-fields.h > +++ b/include/ovn/logical-fields.h > @@ -63,6 +63,8 @@ enum ovn_controller_event { > #define MFF_LOG_CT_ORIG_IP6_DST_ADDR MFF_XXREG1 /* > REG_ORIG_DIP_IPV6 */ > #define MFF_LOG_CT_ORIG_TP_DST_PORT MFF_REG2 /* > REG_ORIG_TP_DPORT > * (bits 0..15). */ > +#define MFF_LOG_CT_SAVED_STATE MFF_REG4 /* REG_CT_STATE > + * (bits 0..8). */ > > /* Logical registers that are needed for backwards > * compatibility with older northd versions. > diff --git a/lib/actions.c b/lib/actions.c > index b81061f5c1..f6f413be27 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -5552,6 +5552,29 @@ encode_FLOOD_REMOTE(const struct ovnact_null *null > OVS_UNUSED, > emit_resubmit(ofpacts, ep->flood_remote_table); > } > > +static void > +encode_CT_STATE_SAVE(const struct ovnact_result *res, > + const struct ovnact_encode_params *ep OVS_UNUSED, > + struct ofpbuf *ofpacts) > +{ > + encode_result_action___(res, ep->ct_state_save_table, > + MFF_LOG_CT_SAVED_STATE, 0, 8, ofpacts); > +} > + > +static void > +parse_CT_STATE_SAVE(struct action_context *ctx, const struct expr_field > *dst, > + struct ovnact_result *res) > +{ > + parse_ovnact_result__(ctx, "ct_state_save", NULL, dst, res, 8); > +} > + > +static void > +format_CT_STATE_SAVE(const struct ovnact_result *res, struct ds *s) > +{ > + expr_field_format(&res->dst, s); > + ds_put_cstr(s, " = ct_state_save();"); > +} > + > /* Parses an assignment or exchange or put_dhcp_opts action. */ > static void > parse_set_action(struct action_context *ctx) > @@ -5658,6 +5681,10 @@ parse_set_action(struct action_context *ctx) > lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { > parse_CT_ORIG_TP_DST(ctx, &lhs, > ovnact_put_CT_ORIG_TP_DST(ctx->ovnacts)); > + } else if (!strcmp(ctx->lexer->token.s, "ct_state_save") && > + lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { > + parse_CT_STATE_SAVE(ctx, &lhs, > + ovnact_put_CT_STATE_SAVE(ctx->ovnacts)); > } else { > parse_assignment_action(ctx, false, &lhs); > } > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index f406114db2..c825e0936f 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -898,7 +898,7 @@ ip_address_and_port_from_lb_key(const char *key, char > **ip_address, > * > * This value is also used to handle some backward compatibility during > * upgrading. It should never decrease or rewind. */ > -#define OVN_INTERNAL_MINOR_VER 7 > +#define OVN_INTERNAL_MINOR_VER 8 > > /* Returns the OVN version. The caller must free the returned value. */ > char * > diff --git a/ovn-sb.xml b/ovn-sb.xml > index 4bcf44d041..9777cc64ad 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2792,6 +2792,14 @@ tcp.flags = RST; > </p> > </dd> > > + <dt><code><var>R</var> = ct_state_save();</code></dt> > + <dd> > + <p> > + This action checks if the packet is tracked and stores the > + conntrack original state in the 8-bit register <var>R</var>. > + </p> > + </dd> > + > <dt><code>sample(probability=<var>packets</var>, ...)</code></dt> > <dd> > <p> > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index fd1a6b197a..0c1c1cd992 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -1494,5 +1494,6 @@ m4_define([OFTABLE_CT_ORIG_NW_DST_LOAD], [81]) > m4_define([OFTABLE_CT_ORIG_IP6_DST_LOAD], [82]) > m4_define([OFTABLE_CT_ORIG_TP_DST_LOAD], [83]) > m4_define([OFTABLE_FLOOD_REMOTE_CHASSIS], [84]) > +m4_define([OFTABLE_CT_STATE_SAVE], [85]) > > m4_define([OFTABLE_SAVE_INPORT_HEX], [m4_eval(OFTABLE_SAVE_INPORT, 16)]) > diff --git a/tests/ovn.at b/tests/ovn.at > index bbaa653a82..938a0d0f0d 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -2287,6 +2287,28 @@ flood_remote; > flood_remote(); > Syntax error at `(' expecting `;'. > > +# ct_state_save() > +reg1[[0..7]] = ct_state_save(); > + encodes as > set_field:0/0xff->reg4,resubmit(,OFTABLE_CT_STATE_SAVE),move:NXM_NX_REG4[[0..7]]->NXM_NX_XXREG0[[64..71]] > + > +reg1[[2..9]] = ct_state_save(); > + encodes as > set_field:0/0xff->reg4,resubmit(,OFTABLE_CT_STATE_SAVE),move:NXM_NX_REG4[[0..7]]->NXM_NX_XXREG0[[66..73]] > + > +reg1[[24..31]] = ct_state_save(); > + encodes as > set_field:0/0xff->reg4,resubmit(,OFTABLE_CT_STATE_SAVE),move:NXM_NX_REG4[[0..7]]->NXM_NX_XXREG0[[88..95]] > + > +xreg1 = ct_state_save(); > + Cannot use 64-bit field xreg1[[0..63]] where 8-bit field is required. > + > +reg1[[0..30]] = ct_state_save(); > + Cannot use 31-bit field reg1[[0..30]] where 8-bit field is required. > + > +ct_state_save; > + Syntax error at `ct_state_save' expecting action. > + > +ct_state_save(); > + Syntax error at `ct_state_save' expecting action. > + > # Miscellaneous negative tests. > ; > Syntax error at `;'. > @@ -37003,8 +37025,10 @@ check_default_flows() { > # Tables 81, 82 and 83 are part of ct_nw_dst(), ct_ip6_dst() and > ct_tp_dst() > # actions respectively and its OK for them to not have default > flows. > # Table 84 is part of flood_remote; action and its OK for > - # it to not have default flows. > - if test ${table} -eq 68 -o ${table} -eq 70 -o ${table} -eq 81 -o > ${table} -eq 82 -o ${table} -eq 83 -o ${table} -eq 84; then > + # it to not have default flows. > + # Table 85 is the implementation of the ct_state_save action and > + # doesn't require a default flow. > + if test ${table} -eq 68 -o ${table} -eq 70 -o ${table} -eq 81 -o > ${table} -eq 82 -o ${table} -eq 83 -o ${table} -eq 84 -o ${table} -eq 85; > then > continue; > fi > AT_CHECK([grep -qe "table=$table.* > priority=0\(,metadata=0x\w*\)\? actions" oflows], [0], [ignore], [ignore], > [echo "Table $table does not contain a default action"]) > diff --git a/tests/test-ovn.c b/tests/test-ovn.c > index de0cb0b444..c4eb2a7dfb 100644 > --- a/tests/test-ovn.c > +++ b/tests/test-ovn.c > @@ -1388,6 +1388,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx > OVS_UNUSED) > .ct_ip6_dst_load_table = OFTABLE_CT_ORIG_IP6_DST_LOAD, > .ct_tp_dst_load_table = OFTABLE_CT_ORIG_TP_DST_LOAD, > .flood_remote_table = OFTABLE_FLOOD_REMOTE_CHASSIS, > + .ct_state_save_table = OFTABLE_CT_STATE_SAVE, > .lflow_uuid.parts = > { 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd}, > .dp_key = 0xabcdef, > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > index b8a9e0f0a6..ce55008a60 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -3121,6 +3121,23 @@ execute_check_out_port_sec(const struct > ovnact_result *dl, > mf_write_subfield_flow(&sf, &sv, uflow); > } > > +static void > +execute_ct_save_state(const struct ovnact_result *dl, struct flow *uflow, > + struct ovs_list *super) > +{ > + struct mf_subfield sf = expr_resolve_field(&dl->dst); > + union mf_subvalue sv = { > + .u8_val = uflow->ct_state & CS_TRACKED ? uflow->ct_state : 0, > + }; > + mf_write_subfield_flow(&sf, &sv, uflow); > + > + struct ds s = DS_EMPTY_INITIALIZER; > + expr_field_format(&dl->dst, &s); > + ovntrace_node_append(super, OVNTRACE_NODE_ACTION, "/* %s = %"PRIu8"; > */", > + ds_cstr(&s), sv.u8_val); > + ds_destroy(&s); > +} > + > static void > trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, > const struct ovntrace_datapath *dp, struct flow *uflow, > @@ -3464,6 +3481,10 @@ trace_actions(const struct ovnact *ovnacts, size_t > ovnacts_len, > case OVNACT_FLOOD_REMOTE: > ovntrace_node_append(super, OVNTRACE_NODE_OUTPUT, > "/* Flood to all remote chassis */"); > + break; > + case OVNACT_CT_STATE_SAVE: > + execute_ct_save_state(ovnact_get_CT_STATE_SAVE(a), uflow, > super); > + break; > } > } > ofpbuf_uninit(&stack); > -- > 2.49.0 > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
diff --git a/controller/lflow.c b/controller/lflow.c index 196f6a21ac..c24b2e2127 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -893,6 +893,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, .ct_ip6_dst_load_table = OFTABLE_CT_ORIG_IP6_DST_LOAD, .ct_tp_dst_load_table = OFTABLE_CT_ORIG_TP_DST_LOAD, .flood_remote_table = OFTABLE_FLOOD_REMOTE_CHASSIS, + .ct_state_save_table = OFTABLE_CT_STATE_SAVE, .ctrl_meter_id = ctrl_meter_id, .common_nat_ct_zone = get_common_nat_zone(ldp), }; diff --git a/controller/lflow.h b/controller/lflow.h index fa99173e67..3fd7ca99f5 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -99,6 +99,7 @@ struct uuid; #define OFTABLE_CT_ORIG_IP6_DST_LOAD 82 #define OFTABLE_CT_ORIG_TP_DST_LOAD 83 #define OFTABLE_FLOOD_REMOTE_CHASSIS 84 +#define OFTABLE_CT_STATE_SAVE 85 /* Common defines shared between some controller components. */ #define CHASSIS_FLOOD_INDEX_START 0x8000 diff --git a/controller/physical.c b/controller/physical.c index 9e797b3b3c..b4f91b134d 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -3085,6 +3085,29 @@ physical_run(struct physical_ctx *p_ctx, ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_IP6_DST_LOAD, 100, 0, &match, &ofpacts, hc_uuid); + /* Implement the ct_state_save() logical action. */ + + /* Add the flow: + * match = (1), action = (reg4[0..7] = ct_state[0..7]) + * table = 85, priority = UINT16_MAX - 1 + */ + match_init_catchall(&match); + ofpbuf_clear(&ofpacts); + put_move(MFF_CT_STATE, 0, MFF_LOG_CT_SAVED_STATE, 0, 8, &ofpacts); + ofctrl_add_flow(flow_table, OFTABLE_CT_STATE_SAVE, UINT16_MAX - 1, 0, + &match, &ofpacts, hc_uuid); + + /* Add the flow: + * match = (!ct.trk), action = (reg4[0..7] = 0) + * table = 85, priority = UINT16_MAX + */ + match_init_catchall(&match); + ofpbuf_clear(&ofpacts); + match_set_ct_state_masked(&match, 0, OVS_CS_F_TRACKED); + put_load(0, MFF_LOG_CT_SAVED_STATE, 0, 8, &ofpacts); + ofctrl_add_flow(flow_table, OFTABLE_CT_STATE_SAVE, UINT16_MAX, 0, + &match, &ofpacts, hc_uuid); + physical_eval_remote_chassis_flows(p_ctx, &ofpacts, flow_table); ofpbuf_uninit(&ofpacts); diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 0993ad0e5d..7fd7b26bfe 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -135,6 +135,7 @@ struct collector_set_ids; OVNACT(CT_ORIG_IP6_DST, ovnact_result) \ OVNACT(CT_ORIG_TP_DST, ovnact_result) \ OVNACT(FLOOD_REMOTE, ovnact_null) \ + OVNACT(CT_STATE_SAVE, ovnact_result) /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ enum OVS_PACKED_ENUM ovnact_type { @@ -964,6 +965,8 @@ struct ovnact_encode_params { * to resubmit. */ uint32_t flood_remote_table; /* OpenFlow table for 'chassis_flood' * to resubmit. */ + uint32_t ct_state_save_table; /* OpenFlow table for saving ct_state to + * register. */ }; void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h index 7c6be6ea42..17154d38bd 100644 --- a/include/ovn/logical-fields.h +++ b/include/ovn/logical-fields.h @@ -63,6 +63,8 @@ enum ovn_controller_event { #define MFF_LOG_CT_ORIG_IP6_DST_ADDR MFF_XXREG1 /* REG_ORIG_DIP_IPV6 */ #define MFF_LOG_CT_ORIG_TP_DST_PORT MFF_REG2 /* REG_ORIG_TP_DPORT * (bits 0..15). */ +#define MFF_LOG_CT_SAVED_STATE MFF_REG4 /* REG_CT_STATE + * (bits 0..8). */ /* Logical registers that are needed for backwards * compatibility with older northd versions. diff --git a/lib/actions.c b/lib/actions.c index b81061f5c1..f6f413be27 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -5552,6 +5552,29 @@ encode_FLOOD_REMOTE(const struct ovnact_null *null OVS_UNUSED, emit_resubmit(ofpacts, ep->flood_remote_table); } +static void +encode_CT_STATE_SAVE(const struct ovnact_result *res, + const struct ovnact_encode_params *ep OVS_UNUSED, + struct ofpbuf *ofpacts) +{ + encode_result_action___(res, ep->ct_state_save_table, + MFF_LOG_CT_SAVED_STATE, 0, 8, ofpacts); +} + +static void +parse_CT_STATE_SAVE(struct action_context *ctx, const struct expr_field *dst, + struct ovnact_result *res) +{ + parse_ovnact_result__(ctx, "ct_state_save", NULL, dst, res, 8); +} + +static void +format_CT_STATE_SAVE(const struct ovnact_result *res, struct ds *s) +{ + expr_field_format(&res->dst, s); + ds_put_cstr(s, " = ct_state_save();"); +} + /* Parses an assignment or exchange or put_dhcp_opts action. */ static void parse_set_action(struct action_context *ctx) @@ -5658,6 +5681,10 @@ parse_set_action(struct action_context *ctx) lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { parse_CT_ORIG_TP_DST(ctx, &lhs, ovnact_put_CT_ORIG_TP_DST(ctx->ovnacts)); + } else if (!strcmp(ctx->lexer->token.s, "ct_state_save") && + lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { + parse_CT_STATE_SAVE(ctx, &lhs, + ovnact_put_CT_STATE_SAVE(ctx->ovnacts)); } else { parse_assignment_action(ctx, false, &lhs); } diff --git a/lib/ovn-util.c b/lib/ovn-util.c index f406114db2..c825e0936f 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -898,7 +898,7 @@ ip_address_and_port_from_lb_key(const char *key, char **ip_address, * * This value is also used to handle some backward compatibility during * upgrading. It should never decrease or rewind. */ -#define OVN_INTERNAL_MINOR_VER 7 +#define OVN_INTERNAL_MINOR_VER 8 /* Returns the OVN version. The caller must free the returned value. */ char * diff --git a/ovn-sb.xml b/ovn-sb.xml index 4bcf44d041..9777cc64ad 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -2792,6 +2792,14 @@ tcp.flags = RST; </p> </dd> + <dt><code><var>R</var> = ct_state_save();</code></dt> + <dd> + <p> + This action checks if the packet is tracked and stores the + conntrack original state in the 8-bit register <var>R</var>. + </p> + </dd> + <dt><code>sample(probability=<var>packets</var>, ...)</code></dt> <dd> <p> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at index fd1a6b197a..0c1c1cd992 100644 --- a/tests/ovn-macros.at +++ b/tests/ovn-macros.at @@ -1494,5 +1494,6 @@ m4_define([OFTABLE_CT_ORIG_NW_DST_LOAD], [81]) m4_define([OFTABLE_CT_ORIG_IP6_DST_LOAD], [82]) m4_define([OFTABLE_CT_ORIG_TP_DST_LOAD], [83]) m4_define([OFTABLE_FLOOD_REMOTE_CHASSIS], [84]) +m4_define([OFTABLE_CT_STATE_SAVE], [85]) m4_define([OFTABLE_SAVE_INPORT_HEX], [m4_eval(OFTABLE_SAVE_INPORT, 16)]) diff --git a/tests/ovn.at b/tests/ovn.at index bbaa653a82..938a0d0f0d 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -2287,6 +2287,28 @@ flood_remote; flood_remote(); Syntax error at `(' expecting `;'. +# ct_state_save() +reg1[[0..7]] = ct_state_save(); + encodes as set_field:0/0xff->reg4,resubmit(,OFTABLE_CT_STATE_SAVE),move:NXM_NX_REG4[[0..7]]->NXM_NX_XXREG0[[64..71]] + +reg1[[2..9]] = ct_state_save(); + encodes as set_field:0/0xff->reg4,resubmit(,OFTABLE_CT_STATE_SAVE),move:NXM_NX_REG4[[0..7]]->NXM_NX_XXREG0[[66..73]] + +reg1[[24..31]] = ct_state_save(); + encodes as set_field:0/0xff->reg4,resubmit(,OFTABLE_CT_STATE_SAVE),move:NXM_NX_REG4[[0..7]]->NXM_NX_XXREG0[[88..95]] + +xreg1 = ct_state_save(); + Cannot use 64-bit field xreg1[[0..63]] where 8-bit field is required. + +reg1[[0..30]] = ct_state_save(); + Cannot use 31-bit field reg1[[0..30]] where 8-bit field is required. + +ct_state_save; + Syntax error at `ct_state_save' expecting action. + +ct_state_save(); + Syntax error at `ct_state_save' expecting action. + # Miscellaneous negative tests. ; Syntax error at `;'. @@ -37003,8 +37025,10 @@ check_default_flows() { # Tables 81, 82 and 83 are part of ct_nw_dst(), ct_ip6_dst() and ct_tp_dst() # actions respectively and its OK for them to not have default flows. # Table 84 is part of flood_remote; action and its OK for - # it to not have default flows. - if test ${table} -eq 68 -o ${table} -eq 70 -o ${table} -eq 81 -o ${table} -eq 82 -o ${table} -eq 83 -o ${table} -eq 84; then + # it to not have default flows. + # Table 85 is the implementation of the ct_state_save action and + # doesn't require a default flow. + if test ${table} -eq 68 -o ${table} -eq 70 -o ${table} -eq 81 -o ${table} -eq 82 -o ${table} -eq 83 -o ${table} -eq 84 -o ${table} -eq 85; then continue; fi AT_CHECK([grep -qe "table=$table.* priority=0\(,metadata=0x\w*\)\? actions" oflows], [0], [ignore], [ignore], [echo "Table $table does not contain a default action"]) diff --git a/tests/test-ovn.c b/tests/test-ovn.c index de0cb0b444..c4eb2a7dfb 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1388,6 +1388,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) .ct_ip6_dst_load_table = OFTABLE_CT_ORIG_IP6_DST_LOAD, .ct_tp_dst_load_table = OFTABLE_CT_ORIG_TP_DST_LOAD, .flood_remote_table = OFTABLE_FLOOD_REMOTE_CHASSIS, + .ct_state_save_table = OFTABLE_CT_STATE_SAVE, .lflow_uuid.parts = { 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd}, .dp_key = 0xabcdef, diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index b8a9e0f0a6..ce55008a60 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -3121,6 +3121,23 @@ execute_check_out_port_sec(const struct ovnact_result *dl, mf_write_subfield_flow(&sf, &sv, uflow); } +static void +execute_ct_save_state(const struct ovnact_result *dl, struct flow *uflow, + struct ovs_list *super) +{ + struct mf_subfield sf = expr_resolve_field(&dl->dst); + union mf_subvalue sv = { + .u8_val = uflow->ct_state & CS_TRACKED ? uflow->ct_state : 0, + }; + mf_write_subfield_flow(&sf, &sv, uflow); + + struct ds s = DS_EMPTY_INITIALIZER; + expr_field_format(&dl->dst, &s); + ovntrace_node_append(super, OVNTRACE_NODE_ACTION, "/* %s = %"PRIu8"; */", + ds_cstr(&s), sv.u8_val); + ds_destroy(&s); +} + static void trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, const struct ovntrace_datapath *dp, struct flow *uflow, @@ -3464,6 +3481,10 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, case OVNACT_FLOOD_REMOTE: ovntrace_node_append(super, OVNTRACE_NODE_OUTPUT, "/* Flood to all remote chassis */"); + break; + case OVNACT_CT_STATE_SAVE: + execute_ct_save_state(ovnact_get_CT_STATE_SAVE(a), uflow, super); + break; } } ofpbuf_uninit(&stack);
The new action stores the 8 relevant bits of the OVS ct_state field into a register. This is implemented through an additional OpenFlow table, OFTABLE_CT_STATE_SAVE, that contains two (static) rules: - high-priority, set the target register to 0 if ct_state -trk is set - low-priority, copy the 8 bits ct_state to the target register otherwise From a logical flow perspective, the action is supposed to be used in the following way: R = ct_state_save(); Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- V2: - Addressed Ales' comment: - properly set the saved ct state register if ct_state includes +trk - Fixed up output of ovn-trace for ct_state_save() to include the actual value that gets copied to the destination register. --- controller/lflow.c | 1 + controller/lflow.h | 1 + controller/physical.c | 23 +++++++++++++++++++++++ include/ovn/actions.h | 3 +++ include/ovn/logical-fields.h | 2 ++ lib/actions.c | 27 +++++++++++++++++++++++++++ lib/ovn-util.c | 2 +- ovn-sb.xml | 8 ++++++++ tests/ovn-macros.at | 1 + tests/ovn.at | 28 ++++++++++++++++++++++++++-- tests/test-ovn.c | 1 + utilities/ovn-trace.c | 21 +++++++++++++++++++++ 12 files changed, 115 insertions(+), 3 deletions(-)