| Message ID | 20240719183126.187651-1-mkp@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] tunnel, tests: Add test for mirroring over tunnels. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | fail | test: fail |
On 7/19/24 20:31, Mike Pattrick wrote: > Add a new test for mirroring packets over a tunnel, which isn't covered > by other existing tests. Thanks, Mike. I think it's a good to have a test covering this functionality. See some comments below. Best regards, Ilya Maximets. > > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- > v2: fixed test by pinning port > --- > tests/tunnel.at | 66 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/tests/tunnel.at b/tests/tunnel.at > index 31e935901..5a9d7663e 100644 > --- a/tests/tunnel.at > +++ b/tests/tunnel.at You're testing the generation of tnl_push actions, so this test should likely live in the tunnel-push-pop.at instead. Most tests in tunnel.at cover kernel-style tunneling. Should we have a test for that as well? > @@ -1362,3 +1362,69 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0], > > OVS_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([tunnel - Mirror over tunnels]) > +OVS_VSWITCHD_START([add-br br-ext -- set bridge br-ext datapath_type=dummy \ > + -- set bridge br-ext other-config:hwaddr=aa:55:aa:55:00:00 \ > + -- add-port br0 t1 -- set Interface t1 type=geneve \ > + options:remote_ip=1.1.1.1 \ > + -- add-port br0 t2 -- set Interface t2 type=erspan \ > + options:remote_ip=1.1.1.2 \ > + options:key=flow options:erspan_ver=1 options:erspan_idx=flow \ > + -- add-port br0 p0 -- set Interface p0 type=dummy \ > + -- add-port br0 p1 -- set Interface p1 type=dummy \ > + -- add-port br-ext p-ext -- set Interface p-ext type=dummy \ > + options:pcap=ext.pcap]) Please, re-format this so it's more clear which options belong to which port, i.e. indent the 'options' setting a bit. You may also avoid a huge initial indentation by using dnl at the top. > + > +dnl Configure mirroring over the UDP and ERSPAN tunnels. > +AT_CHECK([ovs-vsctl \ > + set Bridge br0 mirrors=@m1,@m2 --\ > + --id=@t1 get Port t1 --\ > + --id=@t2 get Port t2 --\ > + --id=@m1 create Mirror name=vxlan select_all=true output_port=@t1 --\ > + --id=@m2 create Mirror name=erspan select_all=true output_port=@t2], [0], [stdout]) Indentation here is a little strange looking. Please, shift the lines beyond the start of ovs-vsctl word. Or put a dnl on the first line, so ovs-vsctl can be placed closer to the beginning of the line. Also, please, move [0], [stdout] to a new line at the ovs-vsctl level. And, please, add spaces before line breaks. > + > +AT_CHECK([ovs-ofctl add-flow br-ext actions=normal]) > +AT_CHECK([ovs-ofctl add-flow br0 actions=normal]) > + > +dnl Make sure ephemeral ports stay static across tests. > +AT_CHECK([ovs-appctl tnl/egress_port_range 35190 35190], [0], [OK > +]) > + > +dnl Setup an IP address for the local side of the tunnel. > +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br-ext 1.1.1.3/24], [0], [OK > +]) > + > +dnl Send two arp replies to populate arp table with tunnel remote endpoints. > +AT_CHECK([ovs-appctl netdev-dummy/receive p-ext dnl > + 'eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),dnl > + arp(sip=1.1.1.1,tip=1.1.1.3,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)' > +]) > +AT_CHECK([ovs-appctl netdev-dummy/receive p-ext dnl > + 'eth(src=f8:bc:12:44:34:b3,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),dnl > + arp(sip=1.1.1.2,tip=1.1.1.3,op=2,sha=f8:bc:12:44:34:b3,tha=00:00:00:00:00:00)' > +]) > + > +flow="in_port(p1),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)" Use m4_define for this instead, e.g. (not tested): m4_define([FLOW], [m4_join([,], [in_port(p1)], [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)])]) > + > +dnl Verify packet is mirrored to both tunnels. Tunnel actions may happen in any order. Double spaces between sentences. And the line is a little too long. > +AT_CHECK([ovs-appctl ofproto/trace --names ovs-dummy "$flow"], [0], [stdout]) > +AT_CHECK([grep -q 'clone(tnl_push(tnl_port(erspan_sys),header(size=50,type=107,eth(dst=f8:bc:12:44:34:b3,src=aa:55:aa:55:00:00,dl_type=0x0800),dnl > +ipv4(src=1.1.1.3,dst=1.1.1.2,proto=47,tos=0,ttl=64,frag=0x4000),erspan(ver=1,sid=0x0,idx=0x0)),out_port(br-ext)),p-ext)' stdout]) > +AT_CHECK([grep -q 'clone(tnl_push(tnl_port(genev_sys_6081),header(size=50,type=5,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),dnl > +ipv4(src=1.1.1.3,dst=1.1.1.1,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=6081,csum=0x0),geneve(vni=0x0)),out_port(br-ext)),p-ext)' stdout]) We can define these as well in a bit more readable form, e.g.: m4_define([ERSPAN_ACT], [m4_join([,], [clone(tnl_push(tnl_port(erspan_sys)], [header(size=50,type=107], [eth(dst=f8:bc:12:44:34:b3,src=aa:55:aa:55:00:00,dl_type=0x0800)], [ipv4(src=1.1.1.3,dst=1.1.1.2,proto=47,tos=0,ttl=64,frag=0x4000)], [erspan(ver=1,sid=0x0,idx=0x0))], [out_port(br-ext))], [p-ext)])]) m4_define([GENEVE_ACT, [m4_join([,], [clone(tnl_push(tnl_port(genev_sys_6081)], [header(size=50,type=5], [eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800)], [ipv4(src=1.1.1.3,dst=1.1.1.1,proto=17,tos=0,ttl=64,frag=0x4000)], [udp(src=0,dst=6081,csum=0x0)], [geneve(vni=0x0))], [out_port(br-ext))], [p-ext)])]) And then use "grep -q 'ERSPAN_ACT' stdout". > + > +dnl Send a packet and then verify the contents. As with the previous test, tunneled packets can have any order. > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'eth(src=aa:55:aa:55:00:00,dst=f8:bc:12:ff:ff:ff),eth_type(0x0800),ipv4(src=1.1.3.92,dst=1.1.3.88,proto=1,tos=0,ttl=64,frag=no),icmp(type=0,code=0)']) Do we need a different packet here? Can the same packet be used? Also, we may use packet-compose to create this packet, then use the resulted hex for netdev-dummy/receive and then we can re-use the same hex to match for the parts of the resulted encapsulated packet. > +AT_CHECK([ovs-pcap ext.pcap > ext.pcap.txt 2>&1]) > +AT_CHECK([cat ext.pcap.txt | tail -n 2 | sort], [0], [dnl We may need to wait for this condition, it may be possible that pcap is not fully written yet at the time of checking. > +[f8bc124434b3aa55aa55000008004500008e00004000402f363b0101010301010102100088be000000001000000000000000f8bc12ffffffaa55aa55000008004500005c00000000]dnl > +[400171ec0101035c0101035800001bfc00000000000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f30313233]dnl > +[3435363738393a3b3c3d3e3f] > +[f8bc124434b6aa55aa55000008004500008e000040004011365a0101010301010101897617c1007a00000000655800000000f8bc12ffffffaa55aa55000008004500005c00000000]dnl > +[400171ec0101035c0101035800001bfc00000000000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f30313233]dnl > +[3435363738393a3b3c3d3e3f] > +]) Wrapping lines at 150 characters is a little strange, not sure what's the benefit of this. If wrapping, then better wrapping much earlier. But also, please, break down the headers into smaller chunks in separate variables, so they are not just huge hex blobs. Maybe smaller hex blobs. If we turn off the df_default for the these tunnels, then the encapsulated packets can even be almost composed with compose-packet. We can do something like: packet=$(ovs-ofctl compose-packet --bare 'FLOW') m4_define([ENCAP_PKT], [m4_join([,], [eth_dst=f8:bc:12:44:34:b3,eth_src=aa:55:aa:55:00:00,eth_type=0x0800)], [nw_src=1.1.1.3,nw_dst=1.1.1.2,nw_proto=47,nw_ttl=64,nw_frag=no)]]) gre_hdr=100088be00000000 # Type: ERSPAN (0x88be) erspan_hdr=1000000000000000 # Version: Type II (1) erspan_pkt=$(ovs-ofctl compose-packet --bare \ 'ENCAP_PKT' "${gre_hdr}${erspan_hdr}${packet}") And the same can be done for geneve with an extra UDP header in the m4_join and a geneve header as a small hex blob. And then we should be able to use AT_CHECK_UNQUOTED to match the pcap with resulted packets. What do you think? We may also not check the actual hex in this test. We have other tests that check the actual encapsulation via pcap. It may be enough to just check that the datapath actions are correct, i.e. contain clones and correct headers. Best regards, Ilya Maximets.
diff --git a/tests/tunnel.at b/tests/tunnel.at index 31e935901..5a9d7663e 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -1362,3 +1362,69 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0], OVS_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([tunnel - Mirror over tunnels]) +OVS_VSWITCHD_START([add-br br-ext -- set bridge br-ext datapath_type=dummy \ + -- set bridge br-ext other-config:hwaddr=aa:55:aa:55:00:00 \ + -- add-port br0 t1 -- set Interface t1 type=geneve \ + options:remote_ip=1.1.1.1 \ + -- add-port br0 t2 -- set Interface t2 type=erspan \ + options:remote_ip=1.1.1.2 \ + options:key=flow options:erspan_ver=1 options:erspan_idx=flow \ + -- add-port br0 p0 -- set Interface p0 type=dummy \ + -- add-port br0 p1 -- set Interface p1 type=dummy \ + -- add-port br-ext p-ext -- set Interface p-ext type=dummy \ + options:pcap=ext.pcap]) + +dnl Configure mirroring over the UDP and ERSPAN tunnels. +AT_CHECK([ovs-vsctl \ + set Bridge br0 mirrors=@m1,@m2 --\ + --id=@t1 get Port t1 --\ + --id=@t2 get Port t2 --\ + --id=@m1 create Mirror name=vxlan select_all=true output_port=@t1 --\ + --id=@m2 create Mirror name=erspan select_all=true output_port=@t2], [0], [stdout]) + +AT_CHECK([ovs-ofctl add-flow br-ext actions=normal]) +AT_CHECK([ovs-ofctl add-flow br0 actions=normal]) + +dnl Make sure ephemeral ports stay static across tests. +AT_CHECK([ovs-appctl tnl/egress_port_range 35190 35190], [0], [OK +]) + +dnl Setup an IP address for the local side of the tunnel. +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br-ext 1.1.1.3/24], [0], [OK +]) + +dnl Send two arp replies to populate arp table with tunnel remote endpoints. +AT_CHECK([ovs-appctl netdev-dummy/receive p-ext dnl + 'eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),dnl + arp(sip=1.1.1.1,tip=1.1.1.3,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)' +]) +AT_CHECK([ovs-appctl netdev-dummy/receive p-ext dnl + 'eth(src=f8:bc:12:44:34:b3,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),dnl + arp(sip=1.1.1.2,tip=1.1.1.3,op=2,sha=f8:bc:12:44:34:b3,tha=00:00:00:00:00:00)' +]) + +flow="in_port(p1),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)" + +dnl Verify packet is mirrored to both tunnels. Tunnel actions may happen in any order. +AT_CHECK([ovs-appctl ofproto/trace --names ovs-dummy "$flow"], [0], [stdout]) +AT_CHECK([grep -q 'clone(tnl_push(tnl_port(erspan_sys),header(size=50,type=107,eth(dst=f8:bc:12:44:34:b3,src=aa:55:aa:55:00:00,dl_type=0x0800),dnl +ipv4(src=1.1.1.3,dst=1.1.1.2,proto=47,tos=0,ttl=64,frag=0x4000),erspan(ver=1,sid=0x0,idx=0x0)),out_port(br-ext)),p-ext)' stdout]) +AT_CHECK([grep -q 'clone(tnl_push(tnl_port(genev_sys_6081),header(size=50,type=5,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),dnl +ipv4(src=1.1.1.3,dst=1.1.1.1,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=6081,csum=0x0),geneve(vni=0x0)),out_port(br-ext)),p-ext)' stdout]) + +dnl Send a packet and then verify the contents. As with the previous test, tunneled packets can have any order. +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'eth(src=aa:55:aa:55:00:00,dst=f8:bc:12:ff:ff:ff),eth_type(0x0800),ipv4(src=1.1.3.92,dst=1.1.3.88,proto=1,tos=0,ttl=64,frag=no),icmp(type=0,code=0)']) +AT_CHECK([ovs-pcap ext.pcap > ext.pcap.txt 2>&1]) +AT_CHECK([cat ext.pcap.txt | tail -n 2 | sort], [0], [dnl +[f8bc124434b3aa55aa55000008004500008e00004000402f363b0101010301010102100088be000000001000000000000000f8bc12ffffffaa55aa55000008004500005c00000000]dnl +[400171ec0101035c0101035800001bfc00000000000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f30313233]dnl +[3435363738393a3b3c3d3e3f] +[f8bc124434b6aa55aa55000008004500008e000040004011365a0101010301010101897617c1007a00000000655800000000f8bc12ffffffaa55aa55000008004500005c00000000]dnl +[400171ec0101035c0101035800001bfc00000000000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f30313233]dnl +[3435363738393a3b3c3d3e3f] +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP
Add a new test for mirroring packets over a tunnel, which isn't covered by other existing tests. Signed-off-by: Mike Pattrick <mkp@redhat.com> --- v2: fixed test by pinning port --- tests/tunnel.at | 66 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)