On 5/20/25 1:03 PM, Ales Musil via dev wrote:
> Populate CT fields during CT action with original fields. This
> is needed for tracking actions that use those fields.
>
> Signed-off-by: Ales Musil <[email protected]>
> ---
> v3: Rebase on top of current main.
> Address comments from Dumitru:
> - Populate IPv6 fields too.
> - Add missing dot.
> ---
Hi Ales,
Thanks for v3!
> utilities/ovn-trace.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index a435b0ff1..4411f654d 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -2445,6 +2445,26 @@ execute_dns_lookup(const struct ovnact_result *dl,
> struct flow *uflow,
> "*** dns_lookup action not implemented");
> }
>
> +/* Populate CT fields from the flow corresponding counterpart. */
> +static void
> +populate_ct_fields(struct flow *ct_flow, struct flow *original_flow)
> +{
> + ct_flow->ct_nw_proto = original_flow->nw_proto;
> + /* L3 */
> + if (original_flow->dl_type == htons(ETH_TYPE_IP)) {
> + ct_flow->ct_nw_src = original_flow->nw_src;
> + ct_flow->ct_nw_dst = original_flow->nw_dst;
> + } else if (original_flow->dl_type == htons(ETH_TYPE_IPV6)) {
> + memcpy(&ct_flow->ct_ipv6_src, &original_flow->ipv6_src,
> + sizeof ct_flow->ct_ipv6_src);
> + memcpy(&ct_flow->ct_ipv6_dst, &original_flow->ipv6_dst,
> + sizeof ct_flow->ct_ipv6_dst);
Nit: I'd just use assignment between the ct_flow and original_flow
fields and let the compiler optimize as needed. It also is type-safer.
I'll leave it up to you though.
> + }
> + /* L4 */
> + ct_flow->ct_tp_src = original_flow->tp_src;
> + ct_flow->ct_tp_dst = original_flow->tp_dst;
> +}
> +
> static void
> execute_ct_next(const struct ovnact_ct_next *ct_next,
> const struct ovntrace_datapath *dp, struct flow *uflow,
> @@ -2466,6 +2486,8 @@ execute_ct_next(const struct ovnact_ct_next *ct_next,
> /* Trace the actions in the next table. */
> struct flow ct_flow = *uflow;
> ct_flow.ct_state = state;
> + populate_ct_fields(&ct_flow, uflow);
> +
> trace__(dp, &ct_flow, ct_next->ltable, pipeline, &node->subs);
>
> /* Upon return, we will trace the actions following the ct action in the
> @@ -2530,6 +2552,8 @@ execute_ct_nat(const struct ovnact_ct_nat *ct_nat,
> super, OVNTRACE_NODE_TRANSFORMATION, "%s", ds_cstr(&s));
> ds_destroy(&s);
>
> + populate_ct_fields(&ct_flow, uflow);
> +
> /* Trace the actions in the next table. */
> trace__(dp, &ct_flow, ct_nat->ltable, pipeline, &node->subs);
>
> @@ -2554,6 +2578,8 @@ ct_commit_to_zone__(const struct
> ovnact_ct_commit_to_zone *ct_nat,
> struct ovntrace_node *node = ovntrace_node_append(
> super, OVNTRACE_NODE_TRANSFORMATION, "%s", ds_cstr(action));
>
> + populate_ct_fields(&ct_flow, uflow);
> +
> /* Trace the actions in the next table. */
> trace__(dp, &ct_flow, ct_nat->ltable, pipeline, &node->subs);
>
> @@ -2657,6 +2683,9 @@ execute_ct_lb(const struct ovnact_ct_lb *ct_lb,
> ct_lb->ovnact.type == OVNACT_CT_LB_MARK ? "ct_lb_mark" : "ct_lb",
> ds_cstr_ro(&comment));
> ds_destroy(&comment);
> +
> + populate_ct_fields(&ct_lb_flow, uflow);
> +
> trace__(dp, &ct_lb_flow, ct_lb->ltable, pipeline, &node->subs);
> }
>
> @@ -3771,7 +3800,7 @@ trace(const char *dp_s, const char *flow_s)
>
> return ds_steal_cstr(&output);
> }
> -
> +
Nit: unrelated.
> static void
> ovntrace_exit(struct unixctl_conn *conn, int argc OVS_UNUSED,
> const char *argv[] OVS_UNUSED, void *exiting_)
With that addressed:
Acked-by: Dumitru Ceara <[email protected]>
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
