Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Mon, Jul 01, 2024 at 01:33:23PM GMT, Eelco Chaudron wrote: > > > On 28 Jun 2024, at 16:11, Adrián Moreno wrote: > > > On Wed, Jun 26, 2024 at 02:15:02PM GMT, Eelco Chaudron wrote: > >> On 5 Jun 2024, at 22:23, Adrian Moreno wrote: > >> > >>> Test simultaneous IPFIX and local sampling including slow-path. > >> > >> I guess Ilya's comments on this patch summarize most of my comments. I had > >> one small additional question. See below. > >> > >> //Eelco > >> > >>> Signed-off-by: Adrian Moreno > >>> --- > >>> tests/system-common-macros.at | 4 ++ > >>> tests/system-traffic.at | 105 ++ > >>> 2 files changed, 109 insertions(+) > >>> > >>> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > >>> index 2a68cd664..22b8885e4 100644 > >>> --- a/tests/system-common-macros.at > >>> +++ b/tests/system-common-macros.at > >>> @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > >>> # OVS_CHECK_DROP_ACTION() > >>> m4_define([OVS_CHECK_DROP_ACTION], > >>> [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > >>> ovs-vswitchd.log])]) > >>> + > >>> +# OVS_CHECK_EMIT_SAMPLE() > >>> +m4_define([OVS_CHECK_EMIT_SAMPLE], > >>> +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > >>> ovs-vswitchd.log])]) > >>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at > >>> index bd7647cbe..babc56b56 100644 > >>> --- a/tests/system-traffic.at > >>> +++ b/tests/system-traffic.at > >>> @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: > >>> * * *5002 *2000 *b85e *00 > >>> > >>> OVS_TRAFFIC_VSWITCHD_STOP > >>> AT_CLEANUP > >>> + > >>> +AT_SETUP([emit_sample]) > >>> +OVS_TRAFFIC_VSWITCHD_START() > >>> +OVS_CHECK_EMIT_SAMPLE() > >>> + > >>> +ADD_NAMESPACES(at_ns0, at_ns1) > >>> +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > >>> [0], [ignore]) > >>> +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > >>> [0], [ignore]) > >>> + > >>> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > >>> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > >>> + > >>> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > >>> +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > >>> + > >>> + > >>> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > >>> ofproto_dpif_upcall:dbg]) > >>> + > >>> +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > >>> +-- --id=@ipfix create IPFIX > >>> targets=\"127.0.0.1:4739\" \ > >>> +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > >>> ipfix=@ipfix, local_sample_group=10 \ > >>> +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > >>> ipfix=@ipfix, local_sample_group=12], > >>> + [0], [ignore]) > >>> + > >>> +AT_DATA([flows.txt], [dnl > >>> +in_port=ovs-p0,ip > >>> actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > >>> +in_port=ovs-p1,ip > >>> actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > >>> +]) > >>> + > >>> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > >>> + > >>> +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) > >>> + > >>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl > >>> +1 packets transmitted, 1 received, 0% packet loss, time 0ms > >>> +]) > >>> + > >>> +m4_define([SAMPLE1], [group_id=0xa > >>> obs_domain=0x,obs_point=0x > >>> .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) > >>> +m4_define([SAMPLE2], [group_id=0xc > >>> obs_domain=0x,obs_point=0x > >>> .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) > >>> +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) > >>> +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) > >>> + > >>> +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx > >>> pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], [dnl > >> > >> Why are we making tx pkts always 24, and are we waiving any potential tx > >> errors? > >> Is this something you have seen not being consistent, if so any idea why? > >> > > > > Yes. By the time we request the statistics, IPFIX cache should contain > > the flows but they might not have been sent yet. When they are sent they > > will fail (because IPFIX target is localhost and it returns the socket > > returns an error if the listening socket does not exist) so the number > > of errors might vary. > > Your explanation makes sense to me. Maybe just add it as a comment so people > looking at the test know why. > Ack. > > I can look deeper into making it more consistent but it looked like a > > quick workaround. > > > >>> +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids > >>> + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 > >>> ok=0, tx pkts=24 > >>> +
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On 28 Jun 2024, at 16:11, Adrián Moreno wrote: > On Wed, Jun 26, 2024 at 02:15:02PM GMT, Eelco Chaudron wrote: >> On 5 Jun 2024, at 22:23, Adrian Moreno wrote: >> >>> Test simultaneous IPFIX and local sampling including slow-path. >> >> I guess Ilya's comments on this patch summarize most of my comments. I had >> one small additional question. See below. >> >> //Eelco >> >>> Signed-off-by: Adrian Moreno >>> --- >>> tests/system-common-macros.at | 4 ++ >>> tests/system-traffic.at | 105 ++ >>> 2 files changed, 109 insertions(+) >>> >>> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >>> index 2a68cd664..22b8885e4 100644 >>> --- a/tests/system-common-macros.at >>> +++ b/tests/system-common-macros.at >>> @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], >>> # OVS_CHECK_DROP_ACTION() >>> m4_define([OVS_CHECK_DROP_ACTION], >>> [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" >>> ovs-vswitchd.log])]) >>> + >>> +# OVS_CHECK_EMIT_SAMPLE() >>> +m4_define([OVS_CHECK_EMIT_SAMPLE], >>> +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" >>> ovs-vswitchd.log])]) >>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >>> index bd7647cbe..babc56b56 100644 >>> --- a/tests/system-traffic.at >>> +++ b/tests/system-traffic.at >>> @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: >>> * * *5002 *2000 *b85e *00 >>> >>> OVS_TRAFFIC_VSWITCHD_STOP >>> AT_CLEANUP >>> + >>> +AT_SETUP([emit_sample]) >>> +OVS_TRAFFIC_VSWITCHD_START() >>> +OVS_CHECK_EMIT_SAMPLE() >>> + >>> +ADD_NAMESPACES(at_ns0, at_ns1) >>> +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], >>> [ignore]) >>> +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], >>> [ignore]) >>> + >>> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") >>> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") >>> + >>> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) >>> +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) >>> + >>> + >>> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg >>> ofproto_dpif_upcall:dbg]) >>> + >>> +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ >>> +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" >>> \ >>> +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 >>> ipfix=@ipfix, local_sample_group=10 \ >>> +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 >>> ipfix=@ipfix, local_sample_group=12], >>> + [0], [ignore]) >>> + >>> +AT_DATA([flows.txt], [dnl >>> +in_port=ovs-p0,ip >>> actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) >>> +in_port=ovs-p1,ip >>> actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) >>> +]) >>> + >>> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) >>> + >>> +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) >>> + >>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl >>> +1 packets transmitted, 1 received, 0% packet loss, time 0ms >>> +]) >>> + >>> +m4_define([SAMPLE1], [group_id=0xa >>> obs_domain=0x,obs_point=0x >>> .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) >>> +m4_define([SAMPLE2], [group_id=0xc >>> obs_domain=0x,obs_point=0x >>> .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) >>> +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) >>> +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) >>> + >>> +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx >>> pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], [dnl >> >> Why are we making tx pkts always 24, and are we waiving any potential tx >> errors? >> Is this something you have seen not being consistent, if so any idea why? >> > > Yes. By the time we request the statistics, IPFIX cache should contain > the flows but they might not have been sent yet. When they are sent they > will fail (because IPFIX target is localhost and it returns the socket > returns an error if the listening socket does not exist) so the number > of errors might vary. Your explanation makes sense to me. Maybe just add it as a comment so people looking at the test know why. > I can look deeper into making it more consistent but it looked like a > quick workaround. > >>> +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids >>> + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, >>> tx pkts=24 >>> + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 >>> + id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, >>> tx pkts=24 >>> + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 >>> +]) >>> + >>> +AT_CHECK([ovs-appctl lsample/show br0], [0], [dnl >>> +local sample statistics
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Wed, Jun 26, 2024 at 02:15:02PM GMT, Eelco Chaudron wrote: > On 5 Jun 2024, at 22:23, Adrian Moreno wrote: > > > Test simultaneous IPFIX and local sampling including slow-path. > > I guess Ilya's comments on this patch summarize most of my comments. I had > one small additional question. See below. > > //Eelco > > > Signed-off-by: Adrian Moreno > > --- > > tests/system-common-macros.at | 4 ++ > > tests/system-traffic.at | 105 ++ > > 2 files changed, 109 insertions(+) > > > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > > index 2a68cd664..22b8885e4 100644 > > --- a/tests/system-common-macros.at > > +++ b/tests/system-common-macros.at > > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > > # OVS_CHECK_DROP_ACTION() > > m4_define([OVS_CHECK_DROP_ACTION], > > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > > ovs-vswitchd.log])]) > > + > > +# OVS_CHECK_EMIT_SAMPLE() > > +m4_define([OVS_CHECK_EMIT_SAMPLE], > > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > > ovs-vswitchd.log])]) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index bd7647cbe..babc56b56 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: > > * * *5002 *2000 *b85e *00 > > > > OVS_TRAFFIC_VSWITCHD_STOP > > AT_CLEANUP > > + > > +AT_SETUP([emit_sample]) > > +OVS_TRAFFIC_VSWITCHD_START() > > +OVS_CHECK_EMIT_SAMPLE() > > + > > +ADD_NAMESPACES(at_ns0, at_ns1) > > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > > [ignore]) > > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > > [ignore]) > > + > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > > + > > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > > + > > + > > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > > ofproto_dpif_upcall:dbg]) > > + > > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > > +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" > > \ > > +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > > ipfix=@ipfix, local_sample_group=10 \ > > +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > > ipfix=@ipfix, local_sample_group=12], > > + [0], [ignore]) > > + > > +AT_DATA([flows.txt], [dnl > > +in_port=ovs-p0,ip > > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > > +in_port=ovs-p1,ip > > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > > +]) > > + > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > + > > +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) > > + > > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl > > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > > +]) > > + > > +m4_define([SAMPLE1], [group_id=0xa > > obs_domain=0x,obs_point=0x > > .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) > > +m4_define([SAMPLE2], [group_id=0xc > > obs_domain=0x,obs_point=0x > > .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) > > +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) > > +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) > > + > > +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx > > pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], [dnl > > Why are we making tx pkts always 24, and are we waiving any potential tx > errors? > Is this something you have seen not being consistent, if so any idea why? > Yes. By the time we request the statistics, IPFIX cache should contain the flows but they might not have been sent yet. When they are sent they will fail (because IPFIX target is localhost and it returns the socket returns an error if the listening socket does not exist) so the number of errors might vary. I can look deeper into making it more consistent but it looked like a quick workaround. > > +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids > > + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, > > tx pkts=24 > > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > > + id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, > > tx pkts=24 > > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > > +]) > > + > > +AT_CHECK([ovs-appctl lsample/show br0], [0], [dnl > > +local sample statistics for bridge "br0": > > +- Collector Set ID: 1 > > + Local Sample Group: 10 > > + Total number of bytes: 98 > > + Total number of packets: 1 > > + > > +- Collector Set ID: 2 > > + Local S
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On 5 Jun 2024, at 22:23, Adrian Moreno wrote: > Test simultaneous IPFIX and local sampling including slow-path. I guess Ilya's comments on this patch summarize most of my comments. I had one small additional question. See below. //Eelco > Signed-off-by: Adrian Moreno > --- > tests/system-common-macros.at | 4 ++ > tests/system-traffic.at | 105 ++ > 2 files changed, 109 insertions(+) > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 2a68cd664..22b8885e4 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > # OVS_CHECK_DROP_ACTION() > m4_define([OVS_CHECK_DROP_ACTION], > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > ovs-vswitchd.log])]) > + > +# OVS_CHECK_EMIT_SAMPLE() > +m4_define([OVS_CHECK_EMIT_SAMPLE], > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > ovs-vswitchd.log])]) > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index bd7647cbe..babc56b56 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: * > * *5002 *2000 *b85e *00 > > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([emit_sample]) > +OVS_TRAFFIC_VSWITCHD_START() > +OVS_CHECK_EMIT_SAMPLE() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > [ignore]) > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > [ignore]) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > + > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > + > + > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > ofproto_dpif_upcall:dbg]) > + > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" \ > +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > ipfix=@ipfix, local_sample_group=10 \ > +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > ipfix=@ipfix, local_sample_group=12], > + [0], [ignore]) > + > +AT_DATA([flows.txt], [dnl > +in_port=ovs-p0,ip > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > +in_port=ovs-p1,ip > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > +]) > + > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > + > +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) > + > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > +]) > + > +m4_define([SAMPLE1], [group_id=0xa > obs_domain=0x,obs_point=0x > .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) > +m4_define([SAMPLE2], [group_id=0xc > obs_domain=0x,obs_point=0x > .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) > +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) > +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) > + > +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx > pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], [dnl Why are we making tx pkts always 24, and are we waiving any potential tx errors? Is this something you have seen not being consistent, if so any idea why? > +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids > + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > + id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > +]) > + > +AT_CHECK([ovs-appctl lsample/show br0], [0], [dnl > +local sample statistics for bridge "br0": > +- Collector Set ID: 1 > + Local Sample Group: 10 > + Total number of bytes: 98 > + Total number of packets: 1 > + > +- Collector Set ID: 2 > + Local Sample Group: 12 > + Total number of bytes: 98 > + Total number of packets: 1 > +]) > + > +# Disable trunc feature to force traffic to go through slow path. > +AT_CHECK([ovs-appctl dpif/set-dp-features br0 trunc false]) > + > +AT_CHECK([ovs-appctl ofproto/trace br0 > 'in_port=ovs-p0,dl_src=e4:11:22:33:44:55,dl_dst=e4:11:22:33:44:66,dl_type=0x0800,nw_src=10.1.1.1,nw_dst=10.1.1.12'], > [0], [stdout]) > +AT_CHECK([tail -3 stdout], [0], [dnl > +Datapath actions: > emit_sample(group=10,cookie=),userspace(pid=4294967295,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918,output_port=4294967295)),trunc(100),3 > +This flow is h
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Mon, Jun 24, 2024 at 08:26:44PM GMT, Ilya Maximets wrote: > On 6/24/24 20:00, Adrián Moreno wrote: > > On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: > > [...] > >> > >> Why are we adding IPFIX? This should work without IPFIX. > >> > >> Having both together can be a separate test. > >> > >>> + > >>> +AT_DATA([flows.txt], [dnl > >>> +in_port=ovs-p0,ip > >>> actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > >>> +in_port=ovs-p1,ip > >>> actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > >> > >> It should be possible to wrap these lines a little with dnl. > > > > I'm curious about wrapping requirements for flows in tests. > > None of them seem to be wrapped, splitting matches and actions won't > > make lines much shorter and having more exotic wrapped & indented > > action arguments seem strage to read (for eyes made to read flows in > > a single line). What wrapping do you have in mind? > > No hard requirements, but I find lines that long hard to read > since they are going out of the window width for me and getting > wrapped in random places. > > > AT_DATA([flows.txt], [dnl > in_port=ovs-p0,ip dnl > actions=dnl > sample( dnl > probability=65535, dnl > collector_set_id=1,dnl > obs_domain_id=1431655765, dnl > obs_point_id=1717986918), dnl > output(port=ovs-p1,max_len=100) > ... > > I agree that above is not very readable... > > Or > > AT_DATA([flows.txt], [dnl > m4_join([], [ > [in_port=ovs-p0,ip], [ ], > [actions=], > > [sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918)], >[output(port=ovs-p1,max_len=100)] > ] > m4_join([], [ > [in_port=ovs-p1,ip], [ ], > [actions=], > > [sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377)], >[output(port=ovs-p0,max_len=100)] > ] > ) > > Or maybe just shorten the lines with a macro: > > m4_define([SAMPLE_ACTION], > > [sample(probability=65535,collector_set_id=$1,obs_domain_id=$2,obs_point_id=$3)] > ) > > AT_DATA([flows.txt], [dnl > in_port=ovs-p0,ip actions=SAMPLE_ACTION(1, 1431655765, 1717986918), > output(port=ovs-p1,max_len=100) > in_port=ovs-p1,ip actions=SAMPLE_ACTION(2, 2290649224, 2576980377), > output(port=ovs-p0,max_len=100) > ]) > I prefer this one. I'll give it a try. Thanks. Adrián > (Haven't tested any of these) > > Best regards, Ilya Maximets. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Mon, Jun 24, 2024 at 07:57:28PM GMT, Ilya Maximets wrote: > On 6/24/24 19:38, Adrián Moreno wrote: > > On Mon, Jun 24, 2024 at 04:15:50PM GMT, Ilya Maximets wrote: > >> On 6/24/24 15:14, Adrián Moreno wrote: > >>> On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: > On 6/5/24 22:23, Adrian Moreno wrote: > > Test simultaneous IPFIX and local sampling including slow-path. > > > > Signed-off-by: Adrian Moreno > > --- > > tests/system-common-macros.at | 4 ++ > > tests/system-traffic.at | 105 ++ > > 2 files changed, 109 insertions(+) > > > > Not a full review, but see a few comments below. > > > diff --git a/tests/system-common-macros.at > > b/tests/system-common-macros.at > > index 2a68cd664..22b8885e4 100644 > > --- a/tests/system-common-macros.at > > +++ b/tests/system-common-macros.at > > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > > # OVS_CHECK_DROP_ACTION() > > m4_define([OVS_CHECK_DROP_ACTION], > > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > > ovs-vswitchd.log])]) > > + > > +# OVS_CHECK_EMIT_SAMPLE() > > +m4_define([OVS_CHECK_EMIT_SAMPLE], > > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > > ovs-vswitchd.log])]) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index bd7647cbe..babc56b56 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: > > * * *5002 *2000 *b85e *00 > > > > OVS_TRAFFIC_VSWITCHD_STOP > > AT_CLEANUP > > + > > +AT_SETUP([emit_sample]) > > Other tests have a test group name at the beginning. > You're also adding the test into NSH section. Add a new section, e.g., > 'sampling' for this with AT_BANNER. > > >>> > >>> Thanks. Will do. > >>> > > +OVS_TRAFFIC_VSWITCHD_START() > > +OVS_CHECK_EMIT_SAMPLE() > > + > > +ADD_NAMESPACES(at_ns0, at_ns1) > > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > > [0], [ignore]) > > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > > [0], [ignore]) > > This looks strange, don't disable IPv6. Appropriate datapath actions > should not forward IPv6 traffic, if that is a desired behavior. > > >>> > >>> I found tests cleaner if automatic ipv6 traffic is not sent by ports. > >>> But sure, I can deal with that with OpenFlow. > >> > >> Would be good to have two separate tests. One for IPv4 and one for IPv6. > >> > >>> > Though, I guess, it would be nice to test capturing both IPv4 and IPv6. > > > + > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > > + > > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > > + > > + > > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > > ofproto_dpif_upcall:dbg]) > > + > > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > > +-- --id=@ipfix create IPFIX > > targets=\"127.0.0.1:4739\" \ > > +-- create Flow_Sample_Collector_Set id=1 > > bridge=@br0 ipfix=@ipfix, local_sample_group=10 \ > > +-- create Flow_Sample_Collector_Set id=2 > > bridge=@br0 ipfix=@ipfix, local_sample_group=12], > > + [0], [ignore]) > > Why are we adding IPFIX? This should work without IPFIX. > > >>> > >>> It is intentional. I want to test both features work simulteaneously. > >> > >> Tests should be simple. At least, we should have at least one very simple > >> test, so if it fails we know that there is an issue in the basic logic. > >> > >> It's valuable to test a combination of the two, but it should be a separate > >> test. So, 3 tests so far: IPv4, IPv6, psample+ipfix. Maybe the 4th > >> separate > >> test for the slow path. > >> > > > > Ack. > > > >>> > Having both together can be a separate test. > > > + > > +AT_DATA([flows.txt], [dnl > > +in_port=ovs-p0,ip > > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > > +in_port=ovs-p1,ip > > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > > It should be possible to wrap these lines a little with dnl. > > Also, please make IDs non-symmetric, so we can't check that the > byte order is correct. > > >>> > >>> Originally, I thought just concatenating the values in native endianness > >>> as a kind of replacement for
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On 6/24/24 20:00, Adrián Moreno wrote: > On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: > [...] >> >> Why are we adding IPFIX? This should work without IPFIX. >> >> Having both together can be a separate test. >> >>> + >>> +AT_DATA([flows.txt], [dnl >>> +in_port=ovs-p0,ip >>> actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) >>> +in_port=ovs-p1,ip >>> actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) >> >> It should be possible to wrap these lines a little with dnl. > > I'm curious about wrapping requirements for flows in tests. > None of them seem to be wrapped, splitting matches and actions won't > make lines much shorter and having more exotic wrapped & indented > action arguments seem strage to read (for eyes made to read flows in > a single line). What wrapping do you have in mind? No hard requirements, but I find lines that long hard to read since they are going out of the window width for me and getting wrapped in random places. AT_DATA([flows.txt], [dnl in_port=ovs-p0,ip dnl actions=dnl sample( dnl probability=65535, dnl collector_set_id=1,dnl obs_domain_id=1431655765, dnl obs_point_id=1717986918), dnl output(port=ovs-p1,max_len=100) ... I agree that above is not very readable... Or AT_DATA([flows.txt], [dnl m4_join([], [ [in_port=ovs-p0,ip], [ ], [actions=], [sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918)], [output(port=ovs-p1,max_len=100)] ] m4_join([], [ [in_port=ovs-p1,ip], [ ], [actions=], [sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377)], [output(port=ovs-p0,max_len=100)] ] ) Or maybe just shorten the lines with a macro: m4_define([SAMPLE_ACTION], [sample(probability=65535,collector_set_id=$1,obs_domain_id=$2,obs_point_id=$3)] ) AT_DATA([flows.txt], [dnl in_port=ovs-p0,ip actions=SAMPLE_ACTION(1, 1431655765, 1717986918), output(port=ovs-p1,max_len=100) in_port=ovs-p1,ip actions=SAMPLE_ACTION(2, 2290649224, 2576980377), output(port=ovs-p0,max_len=100) ]) (Haven't tested any of these) Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: [...] > > Why are we adding IPFIX? This should work without IPFIX. > > Having both together can be a separate test. > > > + > > +AT_DATA([flows.txt], [dnl > > +in_port=ovs-p0,ip > > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > > +in_port=ovs-p1,ip > > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > > It should be possible to wrap these lines a little with dnl. I'm curious about wrapping requirements for flows in tests. None of them seem to be wrapped, splitting matches and actions won't make lines much shorter and having more exotic wrapped & indented action arguments seem strage to read (for eyes made to read flows in a single line). What wrapping do you have in mind? Thanks. Adrián ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On 6/24/24 19:38, Adrián Moreno wrote: > On Mon, Jun 24, 2024 at 04:15:50PM GMT, Ilya Maximets wrote: >> On 6/24/24 15:14, Adrián Moreno wrote: >>> On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: On 6/5/24 22:23, Adrian Moreno wrote: > Test simultaneous IPFIX and local sampling including slow-path. > > Signed-off-by: Adrian Moreno > --- > tests/system-common-macros.at | 4 ++ > tests/system-traffic.at | 105 ++ > 2 files changed, 109 insertions(+) > Not a full review, but see a few comments below. > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 2a68cd664..22b8885e4 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > # OVS_CHECK_DROP_ACTION() > m4_define([OVS_CHECK_DROP_ACTION], > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > ovs-vswitchd.log])]) > + > +# OVS_CHECK_EMIT_SAMPLE() > +m4_define([OVS_CHECK_EMIT_SAMPLE], > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > ovs-vswitchd.log])]) > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index bd7647cbe..babc56b56 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: > * * *5002 *2000 *b85e *00 > > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([emit_sample]) Other tests have a test group name at the beginning. You're also adding the test into NSH section. Add a new section, e.g., 'sampling' for this with AT_BANNER. >>> >>> Thanks. Will do. >>> > +OVS_TRAFFIC_VSWITCHD_START() > +OVS_CHECK_EMIT_SAMPLE() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > [0], [ignore]) > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > [0], [ignore]) This looks strange, don't disable IPv6. Appropriate datapath actions should not forward IPv6 traffic, if that is a desired behavior. >>> >>> I found tests cleaner if automatic ipv6 traffic is not sent by ports. >>> But sure, I can deal with that with OpenFlow. >> >> Would be good to have two separate tests. One for IPv4 and one for IPv6. >> >>> Though, I guess, it would be nice to test capturing both IPv4 and IPv6. > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > + > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > + > + > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > ofproto_dpif_upcall:dbg]) > + > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > +-- --id=@ipfix create IPFIX > targets=\"127.0.0.1:4739\" \ > +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > ipfix=@ipfix, local_sample_group=10 \ > +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > ipfix=@ipfix, local_sample_group=12], > + [0], [ignore]) Why are we adding IPFIX? This should work without IPFIX. >>> >>> It is intentional. I want to test both features work simulteaneously. >> >> Tests should be simple. At least, we should have at least one very simple >> test, so if it fails we know that there is an issue in the basic logic. >> >> It's valuable to test a combination of the two, but it should be a separate >> test. So, 3 tests so far: IPv4, IPv6, psample+ipfix. Maybe the 4th separate >> test for the slow path. >> > > Ack. > >>> Having both together can be a separate test. > + > +AT_DATA([flows.txt], [dnl > +in_port=ovs-p0,ip > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > +in_port=ovs-p1,ip > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) It should be possible to wrap these lines a little with dnl. Also, please make IDs non-symmetric, so we can't check that the byte order is correct. >>> >>> Originally, I thought just concatenating the values in native endianness >>> as a kind of replacement for the user_action_cookie (hence the symmetric >>> values). But I'm having second thoughts. I plan to change it in the next >>> version and store it in "network" order. I will change the test to >>> verify it accordingly. >> >> I'm not sure I get that, but just in case: OpenFlow messages themselves >> should be encoded w
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Mon, Jun 24, 2024 at 04:15:50PM GMT, Ilya Maximets wrote: > On 6/24/24 15:14, Adrián Moreno wrote: > > On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: > >> On 6/5/24 22:23, Adrian Moreno wrote: > >>> Test simultaneous IPFIX and local sampling including slow-path. > >>> > >>> Signed-off-by: Adrian Moreno > >>> --- > >>> tests/system-common-macros.at | 4 ++ > >>> tests/system-traffic.at | 105 ++ > >>> 2 files changed, 109 insertions(+) > >>> > >> > >> Not a full review, but see a few comments below. > >> > >>> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > >>> index 2a68cd664..22b8885e4 100644 > >>> --- a/tests/system-common-macros.at > >>> +++ b/tests/system-common-macros.at > >>> @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > >>> # OVS_CHECK_DROP_ACTION() > >>> m4_define([OVS_CHECK_DROP_ACTION], > >>> [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > >>> ovs-vswitchd.log])]) > >>> + > >>> +# OVS_CHECK_EMIT_SAMPLE() > >>> +m4_define([OVS_CHECK_EMIT_SAMPLE], > >>> +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > >>> ovs-vswitchd.log])]) > >>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at > >>> index bd7647cbe..babc56b56 100644 > >>> --- a/tests/system-traffic.at > >>> +++ b/tests/system-traffic.at > >>> @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: > >>> * * *5002 *2000 *b85e *00 > >>> > >>> OVS_TRAFFIC_VSWITCHD_STOP > >>> AT_CLEANUP > >>> + > >>> +AT_SETUP([emit_sample]) > >> > >> Other tests have a test group name at the beginning. > >> You're also adding the test into NSH section. Add a new section, e.g., > >> 'sampling' for this with AT_BANNER. > >> > > > > Thanks. Will do. > > > >>> +OVS_TRAFFIC_VSWITCHD_START() > >>> +OVS_CHECK_EMIT_SAMPLE() > >>> + > >>> +ADD_NAMESPACES(at_ns0, at_ns1) > >>> +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > >>> [0], [ignore]) > >>> +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], > >>> [0], [ignore]) > >> > >> This looks strange, don't disable IPv6. Appropriate datapath actions > >> should not forward IPv6 traffic, if that is a desired behavior. > >> > > > > I found tests cleaner if automatic ipv6 traffic is not sent by ports. > > But sure, I can deal with that with OpenFlow. > > Would be good to have two separate tests. One for IPv4 and one for IPv6. > > > > >> Though, I guess, it would be nice to test capturing both IPv4 and IPv6. > >> > >>> + > >>> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > >>> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > >>> + > >>> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > >>> +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > >>> + > >>> + > >>> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > >>> ofproto_dpif_upcall:dbg]) > >>> + > >>> +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > >>> +-- --id=@ipfix create IPFIX > >>> targets=\"127.0.0.1:4739\" \ > >>> +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > >>> ipfix=@ipfix, local_sample_group=10 \ > >>> +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > >>> ipfix=@ipfix, local_sample_group=12], > >>> + [0], [ignore]) > >> > >> Why are we adding IPFIX? This should work without IPFIX. > >> > > > > It is intentional. I want to test both features work simulteaneously. > > Tests should be simple. At least, we should have at least one very simple > test, so if it fails we know that there is an issue in the basic logic. > > It's valuable to test a combination of the two, but it should be a separate > test. So, 3 tests so far: IPv4, IPv6, psample+ipfix. Maybe the 4th separate > test for the slow path. > Ack. > > > >> Having both together can be a separate test. > >> > >>> + > >>> +AT_DATA([flows.txt], [dnl > >>> +in_port=ovs-p0,ip > >>> actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > >>> +in_port=ovs-p1,ip > >>> actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > >> > >> It should be possible to wrap these lines a little with dnl. > >> > >> Also, please make IDs non-symmetric, so we can't check that the > >> byte order is correct. > >> > > > > Originally, I thought just concatenating the values in native endianness > > as a kind of replacement for the user_action_cookie (hence the symmetric > > values). But I'm having second thoughts. I plan to change it in the next > > version and store it in "network" order. I will change the test to > > verify it accordingly. > > I'm not sure I get that, but just in case: OpenFlow messages themselves > should be encoded with a network byte order. When printed to the user, > they sh
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On 6/24/24 15:14, Adrián Moreno wrote: > On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: >> On 6/5/24 22:23, Adrian Moreno wrote: >>> Test simultaneous IPFIX and local sampling including slow-path. >>> >>> Signed-off-by: Adrian Moreno >>> --- >>> tests/system-common-macros.at | 4 ++ >>> tests/system-traffic.at | 105 ++ >>> 2 files changed, 109 insertions(+) >>> >> >> Not a full review, but see a few comments below. >> >>> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >>> index 2a68cd664..22b8885e4 100644 >>> --- a/tests/system-common-macros.at >>> +++ b/tests/system-common-macros.at >>> @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], >>> # OVS_CHECK_DROP_ACTION() >>> m4_define([OVS_CHECK_DROP_ACTION], >>> [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" >>> ovs-vswitchd.log])]) >>> + >>> +# OVS_CHECK_EMIT_SAMPLE() >>> +m4_define([OVS_CHECK_EMIT_SAMPLE], >>> +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" >>> ovs-vswitchd.log])]) >>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >>> index bd7647cbe..babc56b56 100644 >>> --- a/tests/system-traffic.at >>> +++ b/tests/system-traffic.at >>> @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: >>> * * *5002 *2000 *b85e *00 >>> >>> OVS_TRAFFIC_VSWITCHD_STOP >>> AT_CLEANUP >>> + >>> +AT_SETUP([emit_sample]) >> >> Other tests have a test group name at the beginning. >> You're also adding the test into NSH section. Add a new section, e.g., >> 'sampling' for this with AT_BANNER. >> > > Thanks. Will do. > >>> +OVS_TRAFFIC_VSWITCHD_START() >>> +OVS_CHECK_EMIT_SAMPLE() >>> + >>> +ADD_NAMESPACES(at_ns0, at_ns1) >>> +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], >>> [ignore]) >>> +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], >>> [ignore]) >> >> This looks strange, don't disable IPv6. Appropriate datapath actions >> should not forward IPv6 traffic, if that is a desired behavior. >> > > I found tests cleaner if automatic ipv6 traffic is not sent by ports. > But sure, I can deal with that with OpenFlow. Would be good to have two separate tests. One for IPv4 and one for IPv6. > >> Though, I guess, it would be nice to test capturing both IPv4 and IPv6. >> >>> + >>> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") >>> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") >>> + >>> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) >>> +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) >>> + >>> + >>> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg >>> ofproto_dpif_upcall:dbg]) >>> + >>> +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ >>> +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" >>> \ >>> +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 >>> ipfix=@ipfix, local_sample_group=10 \ >>> +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 >>> ipfix=@ipfix, local_sample_group=12], >>> + [0], [ignore]) >> >> Why are we adding IPFIX? This should work without IPFIX. >> > > It is intentional. I want to test both features work simulteaneously. Tests should be simple. At least, we should have at least one very simple test, so if it fails we know that there is an issue in the basic logic. It's valuable to test a combination of the two, but it should be a separate test. So, 3 tests so far: IPv4, IPv6, psample+ipfix. Maybe the 4th separate test for the slow path. > >> Having both together can be a separate test. >> >>> + >>> +AT_DATA([flows.txt], [dnl >>> +in_port=ovs-p0,ip >>> actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) >>> +in_port=ovs-p1,ip >>> actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) >> >> It should be possible to wrap these lines a little with dnl. >> >> Also, please make IDs non-symmetric, so we can't check that the >> byte order is correct. >> > > Originally, I thought just concatenating the values in native endianness > as a kind of replacement for the user_action_cookie (hence the symmetric > values). But I'm having second thoughts. I plan to change it in the next > version and store it in "network" order. I will change the test to > verify it accordingly. I'm not sure I get that, but just in case: OpenFlow messages themselves should be encoded with a network byte order. When printed to the user, they should be printed in a host byte order. Netlink messages typically have values in the host byte order, but there are some weird netlink messages that expect network byte order. In any case, the psample test utility should print out values in the host byte order. > >>> +]) >>> + >>> +AT_CHECK([ovs-ofctl
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Mon, Jun 24, 2024 at 02:03:00PM GMT, Ilya Maximets wrote: > On 6/5/24 22:23, Adrian Moreno wrote: > > Test simultaneous IPFIX and local sampling including slow-path. > > > > Signed-off-by: Adrian Moreno > > --- > > tests/system-common-macros.at | 4 ++ > > tests/system-traffic.at | 105 ++ > > 2 files changed, 109 insertions(+) > > > > Not a full review, but see a few comments below. > > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > > index 2a68cd664..22b8885e4 100644 > > --- a/tests/system-common-macros.at > > +++ b/tests/system-common-macros.at > > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > > # OVS_CHECK_DROP_ACTION() > > m4_define([OVS_CHECK_DROP_ACTION], > > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > > ovs-vswitchd.log])]) > > + > > +# OVS_CHECK_EMIT_SAMPLE() > > +m4_define([OVS_CHECK_EMIT_SAMPLE], > > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > > ovs-vswitchd.log])]) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index bd7647cbe..babc56b56 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: > > * * *5002 *2000 *b85e *00 > > > > OVS_TRAFFIC_VSWITCHD_STOP > > AT_CLEANUP > > + > > +AT_SETUP([emit_sample]) > > Other tests have a test group name at the beginning. > You're also adding the test into NSH section. Add a new section, e.g., > 'sampling' for this with AT_BANNER. > Thanks. Will do. > > +OVS_TRAFFIC_VSWITCHD_START() > > +OVS_CHECK_EMIT_SAMPLE() > > + > > +ADD_NAMESPACES(at_ns0, at_ns1) > > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > > [ignore]) > > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > > [ignore]) > > This looks strange, don't disable IPv6. Appropriate datapath actions > should not forward IPv6 traffic, if that is a desired behavior. > I found tests cleaner if automatic ipv6 traffic is not sent by ports. But sure, I can deal with that with OpenFlow. > Though, I guess, it would be nice to test capturing both IPv4 and IPv6. > > > + > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > > + > > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > > + > > + > > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > > ofproto_dpif_upcall:dbg]) > > + > > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > > +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" > > \ > > +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > > ipfix=@ipfix, local_sample_group=10 \ > > +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > > ipfix=@ipfix, local_sample_group=12], > > + [0], [ignore]) > > Why are we adding IPFIX? This should work without IPFIX. > It is intentional. I want to test both features work simulteaneously. > Having both together can be a separate test. > > > + > > +AT_DATA([flows.txt], [dnl > > +in_port=ovs-p0,ip > > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > > +in_port=ovs-p1,ip > > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > > It should be possible to wrap these lines a little with dnl. > > Also, please make IDs non-symmetric, so we can't check that the > byte order is correct. > Originally, I thought just concatenating the values in native endianness as a kind of replacement for the user_action_cookie (hence the symmetric values). But I'm having second thoughts. I plan to change it in the next version and store it in "network" order. I will change the test to verify it accordingly. > > +]) > > + > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > + > > +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) > > Should we wait for the test app to start listening? i.e. there might be > a race between the listener and the ping below. > I guess so. > > + > > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl > > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > > +]) > > We may need to add -W to the ping. > Ack. > > + > > +m4_define([SAMPLE1], [group_id=0xa > > obs_domain=0x,obs_point=0x > > .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) > > +m4_define([SAMPLE2], [group_id=0xc > > obs_domain=0x,obs_point=0x > > .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) > > May use m4_join to wrap these lines. > > > +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) > > +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) > > 'grep -q' to avoid the outpu
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On Mon, Jun 24, 2024 at 01:23:55PM GMT, Eelco Chaudron wrote: > > > On 5 Jun 2024, at 22:23, Adrian Moreno wrote: > > > Test simultaneous IPFIX and local sampling including slow-path. > > > > Signed-off-by: Adrian Moreno > > --- > > tests/system-common-macros.at | 4 ++ > > tests/system-traffic.at | 105 ++ > > 2 files changed, 109 insertions(+) > > > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > > index 2a68cd664..22b8885e4 100644 > > --- a/tests/system-common-macros.at > > +++ b/tests/system-common-macros.at > > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > > # OVS_CHECK_DROP_ACTION() > > m4_define([OVS_CHECK_DROP_ACTION], > > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > > ovs-vswitchd.log])]) > > + > > +# OVS_CHECK_EMIT_SAMPLE() > > +m4_define([OVS_CHECK_EMIT_SAMPLE], > > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > > ovs-vswitchd.log])]) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index bd7647cbe..babc56b56 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: > > * * *5002 *2000 *b85e *00 > > > > OVS_TRAFFIC_VSWITCHD_STOP > > AT_CLEANUP > > + > > +AT_SETUP([emit_sample]) > > +OVS_TRAFFIC_VSWITCHD_START() > > +OVS_CHECK_EMIT_SAMPLE() > > + > > +ADD_NAMESPACES(at_ns0, at_ns1) > > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > > [ignore]) > > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > > [ignore]) > > + > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > > + > > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > > + > > + > > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > > ofproto_dpif_upcall:dbg]) > > + > > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > > +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" > > \ > > +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > > ipfix=@ipfix, local_sample_group=10 \ > > +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > > ipfix=@ipfix, local_sample_group=12], > > + [0], [ignore]) > > + > > +AT_DATA([flows.txt], [dnl > > +in_port=ovs-p0,ip > > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > > +in_port=ovs-p1,ip > > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > > +]) > > + > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > + > > +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) > > + > > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl > > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > > +]) > > + > > +m4_define([SAMPLE1], [group_id=0xa > > obs_domain=0x,obs_point=0x > > .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) > > +m4_define([SAMPLE2], [group_id=0xc > > obs_domain=0x,obs_point=0x > > .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) > > +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) > > +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) > > + > > +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx > > pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], [dnl > > +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids > > + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, > > tx pkts=24 > > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > > + id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, > > tx pkts=24 > > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > > +]) > > This is not a review, just a heads up so you are aware that this test fails > sometimes: > > +++ > /home/echaudron/Documents/review/ovs_adrian_psample/tests/system-kmod-testsuite.dir/at-groups/178/stdout > 2024-06-24 13:05:35.05700 +0200 > @@ -1,6 +1,6 @@ > NXST_IPFIX_FLOW reply (xid=0x2): 2 ids > - id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > - pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 >id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 >pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > > The order does not seem to be fixed all the time (same for the one below). > Ugh, I assumed the output would be ordered. I'll deal with it manually in the test. > //Eelco > > > + > > +AT_CHECK([ovs-
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On 6/5/24 22:23, Adrian Moreno wrote: > Test simultaneous IPFIX and local sampling including slow-path. > > Signed-off-by: Adrian Moreno > --- > tests/system-common-macros.at | 4 ++ > tests/system-traffic.at | 105 ++ > 2 files changed, 109 insertions(+) > Not a full review, but see a few comments below. > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 2a68cd664..22b8885e4 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > # OVS_CHECK_DROP_ACTION() > m4_define([OVS_CHECK_DROP_ACTION], > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > ovs-vswitchd.log])]) > + > +# OVS_CHECK_EMIT_SAMPLE() > +m4_define([OVS_CHECK_EMIT_SAMPLE], > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > ovs-vswitchd.log])]) > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index bd7647cbe..babc56b56 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: * > * *5002 *2000 *b85e *00 > > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([emit_sample]) Other tests have a test group name at the beginning. You're also adding the test into NSH section. Add a new section, e.g., 'sampling' for this with AT_BANNER. > +OVS_TRAFFIC_VSWITCHD_START() > +OVS_CHECK_EMIT_SAMPLE() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > [ignore]) > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > [ignore]) This looks strange, don't disable IPv6. Appropriate datapath actions should not forward IPv6 traffic, if that is a desired behavior. Though, I guess, it would be nice to test capturing both IPv4 and IPv6. > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > + > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > + > + > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > ofproto_dpif_upcall:dbg]) > + > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" \ > +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > ipfix=@ipfix, local_sample_group=10 \ > +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > ipfix=@ipfix, local_sample_group=12], > + [0], [ignore]) Why are we adding IPFIX? This should work without IPFIX. Having both together can be a separate test. > + > +AT_DATA([flows.txt], [dnl > +in_port=ovs-p0,ip > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > +in_port=ovs-p1,ip > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) It should be possible to wrap these lines a little with dnl. Also, please make IDs non-symmetric, so we can't check that the byte order is correct. > +]) > + > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > + > +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) Should we wait for the test app to start listening? i.e. there might be a race between the listener and the ping below. > + > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > +]) We may need to add -W to the ping. > + > +m4_define([SAMPLE1], [group_id=0xa > obs_domain=0x,obs_point=0x > .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) > +m4_define([SAMPLE2], [group_id=0xc > obs_domain=0x,obs_point=0x > .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) May use m4_join to wrap these lines. > +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) > +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) 'grep -q' to avoid the output redirection. We may need to wait for them also, i.e. OVS_WAIT_UNTIL. > + > +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx > pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], Wrap the line with '\'. [dnl > +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids > + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > + id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > +]) > + > +AT_CHECK([ovs-appctl lsample/show br0], [0], [dnl > +local sample statistics for bridge "br0": > +- Collector Set ID: 1 > + Local Sample Group: 10 > + Total number of bytes: 98 > + Total number of packets: 1 > + > +- Colle
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
On 5 Jun 2024, at 22:23, Adrian Moreno wrote: > Test simultaneous IPFIX and local sampling including slow-path. > > Signed-off-by: Adrian Moreno > --- > tests/system-common-macros.at | 4 ++ > tests/system-traffic.at | 105 ++ > 2 files changed, 109 insertions(+) > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 2a68cd664..22b8885e4 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], > # OVS_CHECK_DROP_ACTION() > m4_define([OVS_CHECK_DROP_ACTION], > [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" > ovs-vswitchd.log])]) > + > +# OVS_CHECK_EMIT_SAMPLE() > +m4_define([OVS_CHECK_EMIT_SAMPLE], > +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" > ovs-vswitchd.log])]) > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index bd7647cbe..babc56b56 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: * > * *5002 *2000 *b85e *00 > > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([emit_sample]) > +OVS_TRAFFIC_VSWITCHD_START() > +OVS_CHECK_EMIT_SAMPLE() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > [ignore]) > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], > [ignore]) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") > + > +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) > +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) > + > + > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg > ofproto_dpif_upcall:dbg]) > + > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ > +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" \ > +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 > ipfix=@ipfix, local_sample_group=10 \ > +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 > ipfix=@ipfix, local_sample_group=12], > + [0], [ignore]) > + > +AT_DATA([flows.txt], [dnl > +in_port=ovs-p0,ip > actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) > +in_port=ovs-p1,ip > actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) > +]) > + > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > + > +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) > + > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > +]) > + > +m4_define([SAMPLE1], [group_id=0xa > obs_domain=0x,obs_point=0x > .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) > +m4_define([SAMPLE2], [group_id=0xc > obs_domain=0x,obs_point=0x > .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) > +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) > +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) > + > +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx > pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], [dnl > +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids > + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > + id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx > pkts=24 > + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 > +]) This is not a review, just a heads up so you are aware that this test fails sometimes: +++ /home/echaudron/Documents/review/ovs_adrian_psample/tests/system-kmod-testsuite.dir/at-groups/178/stdout 2024-06-24 13:05:35.05700 +0200 @@ -1,6 +1,6 @@ NXST_IPFIX_FLOW reply (xid=0x2): 2 ids - id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx pkts=24 - pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx pkts=24 pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx pkts=24 + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 The order does not seem to be fixed all the time (same for the one below). //Eelco > + > +AT_CHECK([ovs-appctl lsample/show br0], [0], [dnl > +local sample statistics for bridge "br0": > +- Collector Set ID: 1 > + Local Sample Group: 10 > + Total number of bytes: 98 > + Total number of packets: 1 > + > +- Collector Set ID: 2 > + Local Sample Group: 12 > + Total number of bytes: 98 > + Total number of packets: 1 > +]) > + > +# Disable trunc feature to force traffic to go through slo
Re: [ovs-dev] [RFC v2 9/9] tests: Test local sampling.
Bleep bloop. Greetings Adrian Moreno, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. Patch skipped due to previous failure. Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC v2 9/9] tests: Test local sampling.
Test simultaneous IPFIX and local sampling including slow-path. Signed-off-by: Adrian Moreno --- tests/system-common-macros.at | 4 ++ tests/system-traffic.at | 105 ++ 2 files changed, 109 insertions(+) diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 2a68cd664..22b8885e4 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -378,3 +378,7 @@ m4_define([OVS_CHECK_GITHUB_ACTION], # OVS_CHECK_DROP_ACTION() m4_define([OVS_CHECK_DROP_ACTION], [AT_SKIP_IF([! grep -q "Datapath supports explicit drop action" ovs-vswitchd.log])]) + +# OVS_CHECK_EMIT_SAMPLE() +m4_define([OVS_CHECK_EMIT_SAMPLE], +[AT_SKIP_IF([! grep -q "Datapath supports emit_sample" ovs-vswitchd.log])]) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index bd7647cbe..babc56b56 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -8977,3 +8977,108 @@ OVS_WAIT_UNTIL([cat p2.pcap | grep -E "0x0050: * * *5002 *2000 *b85e *00 OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([emit_sample]) +OVS_TRAFFIC_VSWITCHD_START() +OVS_CHECK_EMIT_SAMPLE() + +ADD_NAMESPACES(at_ns0, at_ns1) +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], [ignore]) +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], [ignore]) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66") + +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e4:11:22:33:44:66]) +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e4:11:22:33:44:55]) + + +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg ofproto_dpif_upcall:dbg]) + +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \ +-- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" \ +-- create Flow_Sample_Collector_Set id=1 bridge=@br0 ipfix=@ipfix, local_sample_group=10 \ +-- create Flow_Sample_Collector_Set id=2 bridge=@br0 ipfix=@ipfix, local_sample_group=12], + [0], [ignore]) + +AT_DATA([flows.txt], [dnl +in_port=ovs-p0,ip actions=sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918),output(port=ovs-p1,max_len=100) +in_port=ovs-p1,ip actions=sample(probability=65535,collector_set_id=2,obs_domain_id=2290649224,obs_point_id=2576980377),output(port=ovs-p0,max_len=100) +]) + +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl +1 packets transmitted, 1 received, 0% packet loss, time 0ms +]) + +m4_define([SAMPLE1], [group_id=0xa obs_domain=0x,obs_point=0x .*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2]) +m4_define([SAMPLE2], [group_id=0xc obs_domain=0x,obs_point=0x .*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1]) +AT_CHECK([grep -E 'SAMPLE1' psample.out >/dev/null]) +AT_CHECK([grep -E 'SAMPLE2' psample.out >/dev/null]) + +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0], [dnl +NXST_IPFIX_FLOW reply (xid=0x2): 2 ids + id 1: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx pkts=24 + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 + id 2: flows=1, current flows=0, sampled pkts=1, ipv4 ok=1, ipv6 ok=0, tx pkts=24 + pkts errs=0, ipv4 errs=0, ipv6 errs=0, tx errs=0 +]) + +AT_CHECK([ovs-appctl lsample/show br0], [0], [dnl +local sample statistics for bridge "br0": +- Collector Set ID: 1 + Local Sample Group: 10 + Total number of bytes: 98 + Total number of packets: 1 + +- Collector Set ID: 2 + Local Sample Group: 12 + Total number of bytes: 98 + Total number of packets: 1 +]) + +# Disable trunc feature to force traffic to go through slow path. +AT_CHECK([ovs-appctl dpif/set-dp-features br0 trunc false]) + +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=ovs-p0,dl_src=e4:11:22:33:44:55,dl_dst=e4:11:22:33:44:66,dl_type=0x0800,nw_src=10.1.1.1,nw_dst=10.1.1.12'], [0], [stdout]) +AT_CHECK([tail -3 stdout], [0], [dnl +Datapath actions: emit_sample(group=10,cookie=),userspace(pid=4294967295,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=1431655765,obs_point_id=1717986918,output_port=4294967295)),trunc(100),3 +This flow is handled by the userspace slow path because it: + - Uses action(s) not supported by datapath. +]) + +OVS_DAEMONIZE([ovstest test-psample > psample_slow.out], [psample_slow.pid]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl +1 packets transmitted, 1 received, 0% packet loss, time 0ms +]) + +AT_CHECK([grep -E 'SAMPLE1' psample_slow.out >/dev/null]) +AT_CHECK([grep -E 'SAMPLE2' psample_slow.out >/dev/null]) + +AT_CHECK([ovs-ofctl dump-ipfix-flow br0 | sed 's/tx pkts=[[0-9]]*/tx pkts=24/' | sed 's/tx errs=[[0-9]]*/tx errs=0/'], [0],
