Re: [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test

[Aaron Conole <aconole@redhat.com>]

Adrián Moreno <amorenoz@redhat.com> writes:

> On Mon, Jun 17, 2024 at 07:18:05AM GMT, Adrián Moreno wrote:
>> On Fri, Jun 14, 2024 at 01:07:33PM GMT, Aaron Conole wrote:
>> > Adrian Moreno <amorenoz@redhat.com> writes:
>> >
>> > > Add a test to verify sampling packets via psample works.
>> > >
>> > > In order to do that, create a subcommand in ovs-dpctl.py to listen to
>> > > on the psample multicast group and print samples.
>> > >
>> > > In order to also test simultaneous sFlow and psample actions and
>> > > packet truncation, add missing parsing support for "userspace" and
>> > > "trunc" actions.
>> >
>> > Maybe split that into a separate patch.  This has a bugfix and 3
>> > features being pushed in.  I know it's already getting long as a series,
>> > so maybe it's okay to fold the userspace attribute bugfix with the parse
>> > support (since it wasn't really usable before).
>> >
>>
>> OK. Sounds reasonable.
>>
>> > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>> > > ---
>> > >  .../selftests/net/openvswitch/openvswitch.sh  |  99 +++++++++++++++-
>> > >  .../selftests/net/openvswitch/ovs-dpctl.py    | 112 +++++++++++++++++-
>> > >  2 files changed, 204 insertions(+), 7 deletions(-)
>> > >
>> > > diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> > > index 5cae53543849..f6e0ae3f6424 100755
>> > > --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> > > +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> > > @@ -20,7 +20,8 @@ tests="
>> > >  	nat_related_v4				ip4-nat-related: ICMP related matches work with SNAT
>> > >  	netlink_checks				ovsnl: validate netlink attrs and settings
>> > >  	upcall_interfaces			ovs: test the upcall interfaces
>> > > -	drop_reason				drop: test drop reasons are emitted"
>> > > +	drop_reason				drop: test drop reasons are emitted
>> > > +	emit_sample 				emit_sample: Sampling packets with psample"
>> > >
>> > >  info() {
>> > >      [ $VERBOSE = 0 ] || echo $*
>> > > @@ -170,6 +171,19 @@ ovs_drop_reason_count()
>> > >  	return `echo "$perf_output" | grep "$pattern" | wc -l`
>> > >  }
>> > >
>> > > +ovs_test_flow_fails () {
>> > > +	ERR_MSG="Flow actions may not be safe on all matching packets"
>> > > +
>> > > +	PRE_TEST=$(dmesg | grep -c "${ERR_MSG}")
>> > > +	ovs_add_flow $@ &> /dev/null $@ && return 1
>> > > +	POST_TEST=$(dmesg | grep -c "${ERR_MSG}")
>> > > +
>> > > +	if [ "$PRE_TEST" == "$POST_TEST" ]; then
>> > > +		return 1
>> > > +	fi
>> > > +	return 0
>> > > +}
>> > > +
>> > >  usage() {
>> > >  	echo
>> > >  	echo "$0 [OPTIONS] [TEST]..."
>> > > @@ -184,6 +198,89 @@ usage() {
>> > >  	exit 1
>> > >  }
>> > >
>> > > +
>> > > +# emit_sample test
>> > > +# - use emit_sample to observe packets
>> > > +test_emit_sample() {
>> > > +	sbx_add "test_emit_sample" || return $?
>> > > +
>> > > +	# Add a datapath with per-vport dispatching.
>> > > +	ovs_add_dp "test_emit_sample" emit_sample -V 2:1 || return 1
>> > > +
>> > > +	info "create namespaces"
>> > > +	ovs_add_netns_and_veths "test_emit_sample" "emit_sample" \
>> > > +		client c0 c1 172.31.110.10/24 -u || return 1
>> > > +	ovs_add_netns_and_veths "test_emit_sample" "emit_sample" \
>> > > +		server s0 s1 172.31.110.20/24 -u || return 1
>> > > +
>> > > +	# Check if emit_sample actions can be configured.
>> > > +	ovs_add_flow "test_emit_sample" emit_sample \
>> > > +	'in_port(1),eth(),eth_type(0x0806),arp()' 'emit_sample(group=1)'
>> > > +	if [ $? == 1 ]; then
>> > > +		info "no support for emit_sample - skipping"
>> > > +		ovs_exit_sig
>> > > +		return $ksft_skip
>> > > +	fi
>> > > +
>> > > +	ovs_del_flows "test_emit_sample" emit_sample
>> > > +
>> > > +	# Allow ARP
>> > > +	ovs_add_flow "test_emit_sample" emit_sample \
>> > > +		'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
>> > > +	ovs_add_flow "test_emit_sample" emit_sample \
>> > > +		'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
>> > > +
>> > > +	# Test action verification.
>> > > +	OLDIFS=$IFS
>> > > +	IFS='*'
>> > > +	min_key='in_port(1),eth(),eth_type(0x0800),ipv4()'
>> > > +	for testcase in \
>> > > +		"cookie to large"*"emit_sample(group=1,cookie=1615141312111009080706050403020100)" \
>> > > +		"no group with cookie"*"emit_sample(cookie=abcd)" \
>> > > +		"no group"*"sample()";
>> > > +	do
>> > > +		set -- $testcase;
>> > > +		ovs_test_flow_fails "test_emit_sample" emit_sample $min_key $2
>> > > +		if [ $? == 1 ]; then
>> > > +			info "failed - $1"
>> > > +			return 1
>> > > +		fi
>> > > +	done
>> > > +	IFS=$OLDIFS
>> > > +
>> > > +	# Sample first 14 bytes of all traffic.
>> > > +	ovs_add_flow "test_emit_sample" emit_sample \
>> > > +	"in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10,proto=1),icmp()" "trunc(14),emit_sample(group=1,cookie=c0ffee),2"
>> > > +
>> > > +	# Sample all traffic. In this case, use a sample() action with both
>> > > +	# emit_sample and an upcall emulating simultaneous local sampling and
>> > > +	# sFlow / IPFIX.
>> > > +	nlpid=$(grep -E "listening on upcall packet handler" $ovs_dir/s0.out | cut -d ":" -f 2 | tr -d ' ')
>> > > +	ovs_add_flow "test_emit_sample" emit_sample \
>> > > +	"in_port(2),eth(),eth_type(0x0800),ipv4(src=172.31.110.20,proto=1),icmp()" "sample(sample=100%,actions(emit_sample(group=2,cookie=eeff0c),userspace(pid=${nlpid},userdata=eeff0c))),1"
>> > > +
>> > > +	# Record emit_sample data.
>> > > +	python3 $ovs_base/ovs-dpctl.py psample >$ovs_dir/psample.out 2>$ovs_dir/psample.err &
>> > > +	pid=$!
>> > > +	on_exit "ovs_sbx test_emit_sample kill -TERM $pid 2>/dev/null"
>> >
>> >   Maybe ovs_netns_spawn_daemon ?
>> >
>>
>> I'll take a look at it, thanks.
>>
>
> I've looked into ovs_netns_spawn_daemon and I think it'll not be useful
> for this command since it needs to run in the default namespace. I can
> add a new "ovs_spawn_daemon" so it's reusable. WDYT?

Okay

>> [...]