On 9 Oct 2023, at 13:40, Roi Dayan wrote:
> 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.
Thanks!
>>>> 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
