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
>> 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
