On 09/10/2023 12:56, Eelco Chaudron wrote:
>
>
> On 8 Oct 2023, at 9:08, Roi Dayan wrote:
>
>> On 05/10/2023 15:36, Eelco Chaudron wrote:
>>>
>>>
>>> On 4 Oct 2023, at 12:09, Roi Dayan via dev wrote:
>>>
>>>> Test geneve options mirror flow doesn't add redundant mirror.
>>>>
>>>> Signed-off-by: Roi Dayan <[email protected]>
>>>
>>> In general this patch looks good, some small nit below.
>>>
>>> //Eelco
>>>
>>>> ---
>>>>
>>>> Notes:
>>>> v2:
>>>> - add dot in title.
>>>>
>>>> tests/tunnel.at | 29 +++++++++++++++++++++++++++++
>>>> 1 file changed, 29 insertions(+)
>>>>
>>>> diff --git a/tests/tunnel.at b/tests/tunnel.at
>>>> index ddeb66bc9fb7..c0b932110446 100644
>>>> --- a/tests/tunnel.at
>>>> +++ b/tests/tunnel.at
>>>> @@ -1279,3 +1279,32 @@ AT_CHECK([tail -1 stdout], [0],
>>>>
>>>> OVS_VSWITCHD_STOP
>>>> AT_CLEANUP
>>>> +
>>>> +AT_SETUP([tunnel - Geneve metadata mirror])
>>>> +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=geneve \
>>>> + options:remote_ip=1.1.1.1 ofport_request=1 \
>>>> + -- add-port br0 p2 -- set Interface p2 type=dummy \
>>>> + ofport_request=2 ofport_request=2])
>>>> +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP
>>>> +add_of_ports br0 90
>>>> +ovs-vsctl \
>>>> + set Bridge br0 mirrors=@m --\
>>>> + --id=@p90 get Port p90 --\
>>>> + --id=@m create Mirror name=mymirror select_all=true
>>>> output_port=@p90
>>>
>>> Can wer put this command inside an AT_CHECK()? Something like this (not
>>> tested):
>>>
>>
>> Is it needed? test will fail anyway if this set fails.
>
> Yes it will fail, but with the explicit check we directly know why it fails.
> I made the same request to a patch from mkp if I remember correcrtly.
>
>> I took this from other case in this file. it looks like
>> All cases in this file don't use AT_CHECK() for this when set the mirror.
>> So I suggest maybe it's not needed but if it is and you want it, can I
>> submit it in a later commit for all cases not related to this series?
>
> I would prefer to fix yours in this patch, we can do a follow up patch to fix
> all other instances.
>
> //Eelco
>
ok. i'll do the change.
>>> AT_CHECK([ovs-vsctl \
>>> set Bridge br0 mirrors=@m --\
>>> --id=@p90 get Port p90 --\
>>> --id=@m create Mirror name=mymirror select_all=true
>>> output_port=@p90
>>> ], [], [ignore])
>>>> +
>>>> +AT_CHECK([ovs-ofctl add-tlv-map br0
>>>> "{class=0xffff,type=0,len=4}->tun_metadata0,{class=0xffff,type=1,len=8}->tun_metadata1"])
>>>> +
>>>> +AT_DATA([flows.txt], [dnl
>>>> +in_port=2,actions=set_field:0xa->tun_metadata0,set_field:0x1234567890abcdef->tun_metadata1,1
>>>> +tun_metadata0=0xb/0xf,actions=2
>>>> +])
>>>> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
>>>> +
>>>> +flow="in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)"
>>>> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow"], [0], [stdout])
>>>> +AT_CHECK_UNQUOTED([tail -1 stdout], [0],
>>>> + [Datapath actions:
>>>> 90,set(tunnel(dst=1.1.1.1,ttl=64,tp_dst=6081,geneve({class=0xffff,type=0,len=4,0xa}{class=0xffff,type=0x1,len=8,0x1234567890abcdef}),flags(df))),6081
>>>> +])
>>>> +
>>>> +OVS_VSWITCHD_STOP
>>>> +AT_CLEANUP
>>>> --
>>>> 2.38.0
>>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> [email protected]
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
