On 09/10/2023 15:05, Roi Dayan wrote:
> The cited commit fixed missing mirror packets by reset mirror when
> packets are modified but setting geneve options was also treated as
> a modified packet but should be treated as a part of set_tunnel
> which doesn't reset mirror.
>
> Fixes: feed7f677505 ("ofproto-dpif-upcall: Mirror packets that are modified.")
> Signed-off-by: Roi Dayan <r...@nvidia.com>
> Acked-by: Simon Horman <ho...@ovn.org>
> Acked-by: Eelco Chaudron <echau...@redhat.com>
> ---
>
> Notes:
> v2:
> - user correct sha in fixes line.
>
> ofproto/ofproto-dpif-xlate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index be4bd6657688..e243773307b7 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -7097,7 +7097,7 @@ reset_mirror_ctx(struct xlate_ctx *ctx, const struct
> flow *flow,
>
> set_field = ofpact_get_SET_FIELD(a);
> mf = set_field->field;
> - if (mf_are_prereqs_ok(mf, flow, NULL)) {
> + if (mf_are_prereqs_ok(mf, flow, NULL) && !mf_is_tun_metadata(mf)) {
> ctx->mirrors = 0;
> }
> return;
Hi,
I would like to consult another related issue to the original commit.
feed7f677505 ("ofproto-dpif-upcall: Mirror packets that are modified.")
Now with geneve options the redundant mirror is removed but if there will be
a real modification there will still be a mirror output but in an incorrect
place.
For example adding dec_ttl, the action list will be like this:
actions:enp8s0f0_1,set(tunnel(tun_id=0x2a,src=7.7.7.7,dst=7.7.7.8,ttl=64,tp_dst=6081,geneve({class=0xffff,type=0x80,len=4,0x1234}),flags(df|key))),set(ipv4(ttl=63)),genev_sys_6081,enp8s0f0_1
A mirror output enp8s0f0_1 added at the end but the second mirror is with the
tunnel header already.
When not using tunnels the mirror is fine and the action list will look like
this:
actions:port1,dec_ttl,port2,port1
So with tunnel the second mirror shouldn't have been somehow with the dec_ttl
action but without the tunnel header?
Should the actions list somehow be like this
actions:enp8s0f0_1,set(ipv4(ttl=63),enp8s0f0_1,set(tunnel(...),genev_sys_6081
Am I looking at this wrong? What do you think?
Thanks,
Roi
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
