On 7/20/25 11:24 AM, Reuven Plevinsky via dev wrote:
> For example, consider the following OpenFlow rule:
> - IPv6 -> IPv4 tunnel translation:
> add-flow br-int 'in_port=vxlan_br-int_0, \
> actions=set_field:8.8.8.7->tun_src, \
> set_field:8.8.8.8->tun_dst,set_field:43->tun_id,geneve_br-int_1'
>
> As any set_field action, flow wildcards are set to the
> tun_src/dst. The offload layer fails if not all matches are
> handled ("consumed"). Clear the irrelevant wildcards.
>
> Signed-off-by: Reuven Plevinsky <rplevin...@nvidia.com>
> ---
> ofproto/ofproto-dpif-xlate.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
Hi, Reuven. Thanks for the patch!
Though I'm not sure we can actually clear the bits of the old tunnel from
the match. What happens if we actually have a match on the original
tunnel metadata before sending to another tunnel? It seems like the new
code will clear the bits and we'll lose the information generating a
wrong datapath match.
Please, also provide a unit test that is showcasing the issue. Maybe
that will clear some of the questions I have.
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 2c8197fb7307..7e986d0d4d0f 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -8078,6 +8078,19 @@ xlate_wc_init(struct xlate_ctx *ctx)
> tnl_wc_init(&ctx->xin->flow, ctx->wc);
> }
>
> +static void
> +clear_tunnel_wc(const struct flow_tnl *spec, struct flow_tnl *mask)
> +{
> + if (!spec->ip_src || !spec->ip_dst) {
The '||' seems strange here, I'd expect an '&&' instead.
> + mask->ip_src = 0;
> + mask->ip_dst = 0;
> + }
> + if (ipv6_is_zero(&spec->ipv6_src) || ipv6_is_zero(&spec->ipv6_dst)) {
Same for the '||', and also should use ipv6_addr_is_set() instead.
> + mask->ipv6_src = in6addr_any;
> + mask->ipv6_dst = in6addr_any;
> + }
> +}
> +
> static void
> xlate_wc_finish(struct xlate_ctx *ctx)
> {
> @@ -8152,6 +8165,9 @@ xlate_wc_finish(struct xlate_ctx *ctx)
> if (!flow_tnl_dst_is_set(&ctx->xin->upcall_flow->tunnel)) {
> memset(&ctx->wc->masks.tunnel, 0, sizeof ctx->wc->masks.tunnel);
> }
> + /* Both IPv4/IPv6 tunnel wc may be set because of set_fields action.
> + * Clear the irrelevant one. */
> + clear_tunnel_wc(&ctx->xin->upcall_flow->tunnel, &ctx->wc->masks.tunnel);
> }
>
> /* This will tweak the odp actions generated. For now, it will:
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
