Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH net-next v2 8/9] selftests: openvswitch: add emit_sample action
       [not found] <20240603185647.2310748-1-amorenoz@redhat.com>
@ 2024-06-03 18:56 ` Adrian Moreno
  2024-06-03 18:56 ` [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test Adrian Moreno
  1 sibling, 0 replies; 8+ messages in thread
From: Adrian Moreno @ 2024-06-03 18:56 UTC (permalink / raw)
  To: netdev
  Cc: aconole, echaudro, horms, i.maximets, dev, Adrian Moreno,
	Pravin B Shelar, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, linux-kselftest, linux-kernel

Add sample and emit_sample action support to ovs-dpctl.py.

Refactor common attribute parsing logic into an external function.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 .../selftests/net/openvswitch/ovs-dpctl.py    | 162 +++++++++++++++++-
 1 file changed, 161 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index a2395c3f37a1..f8b5362aac8c 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -8,6 +8,7 @@ import argparse
 import errno
 import ipaddress
 import logging
+import math
 import multiprocessing
 import re
 import struct
@@ -58,6 +59,7 @@ OVS_FLOW_CMD_DEL = 2
 OVS_FLOW_CMD_GET = 3
 OVS_FLOW_CMD_SET = 4
 
+UINT32_MAX = 0xFFFFFFFF
 
 def macstr(mac):
     outstr = ":".join(["%02X" % i for i in mac])
@@ -267,6 +269,75 @@ def parse_extract_field(
     return str_skipped, data
 
 
+def parse_attrs(actstr, attr_desc):
+    """Parses the given action string and returns a list of netlink
+    attributes based on a list of attribute descriptions.
+
+    Each element in the attribute description list is a tuple such as:
+        (name, attr_name, parse_func)
+    where:
+        name: is the string representing the attribute
+        attr_name: is the name of the attribute as defined in the uAPI.
+        parse_func: is a callable accepting a string and returning either
+            a single object (the parsed attribute value) or a tuple of
+            two values (the parsed attribute value and the remaining string)
+
+    Returns a list of attributes and the remaining string.
+    """
+    def parse_attr(actstr, key, func):
+        actstr = actstr[len(key) :]
+
+        if not func:
+            return None, actstr
+
+        delim = actstr[0]
+        actstr = actstr[1:]
+
+        if delim == "=":
+            pos = strcspn(actstr, ",)")
+            ret = func(actstr[:pos])
+        else:
+            ret = func(actstr)
+
+        if isinstance(ret, tuple):
+            (datum, actstr) = ret
+        else:
+            datum = ret
+            actstr = actstr[strcspn(actstr, ",)"):]
+
+        if delim == "(":
+            if not actstr or actstr[0] != ")":
+                raise ValueError("Action contains unbalanced parentheses")
+
+            actstr = actstr[1:]
+
+        actstr = actstr[strspn(actstr, ", ") :]
+
+        return datum, actstr
+
+    attrs = []
+    attr_desc = list(attr_desc)
+    while actstr and actstr[0] != ")" and attr_desc:
+        found = False
+        for i, (key, attr, func) in enumerate(attr_desc):
+            if actstr.startswith(key):
+                datum, actstr = parse_attr(actstr, key, func)
+                attrs.append([attr, datum])
+                found = True
+                del attr_desc[i]
+
+        if not found:
+            raise ValueError("Unknown attribute: '%s'" % actstr)
+
+        actstr = actstr[strspn(actstr, ", ") :]
+
+    if actstr[0] != ")":
+        raise ValueError("Action string contains extra garbage or has "
+                         "unbalanced parenthesis: '%s'" % actstr)
+
+    return attrs, actstr[1:]
+
+
 class ovs_dp_msg(genlmsg):
     # include the OVS version
     # We need a custom header rather than just being able to rely on
@@ -285,7 +356,7 @@ class ovsactions(nla):
         ("OVS_ACTION_ATTR_SET", "none"),
         ("OVS_ACTION_ATTR_PUSH_VLAN", "none"),
         ("OVS_ACTION_ATTR_POP_VLAN", "flag"),
-        ("OVS_ACTION_ATTR_SAMPLE", "none"),
+        ("OVS_ACTION_ATTR_SAMPLE", "sample"),
         ("OVS_ACTION_ATTR_RECIRC", "uint32"),
         ("OVS_ACTION_ATTR_HASH", "none"),
         ("OVS_ACTION_ATTR_PUSH_MPLS", "none"),
@@ -304,8 +375,85 @@ class ovsactions(nla):
         ("OVS_ACTION_ATTR_ADD_MPLS", "none"),
         ("OVS_ACTION_ATTR_DEC_TTL", "none"),
         ("OVS_ACTION_ATTR_DROP", "uint32"),
+        ("OVS_ACTION_ATTR_EMIT_SAMPLE", "emit_sample"),
     )
 
+    class emit_sample(nla):
+        nla_flags = NLA_F_NESTED
+
+        nla_map = (
+            ("OVS_EMIT_SAMPLE_ATTR_UNSPEC", "none"),
+            ("OVS_EMIT_SAMPLE_ATTR_GROUP", "uint32"),
+            ("OVS_EMIT_SAMPLE_ATTR_COOKIE", "array(uint8)"),
+        )
+
+        def dpstr(self, more=False):
+            args = "group=%d" % self.get_attr("OVS_EMIT_SAMPLE_ATTR_GROUP")
+
+            cookie = self.get_attr("OVS_EMIT_SAMPLE_ATTR_COOKIE")
+            if cookie:
+                args += ",cookie(%s)" % \
+                        "".join(format(x, "02x") for x in cookie)
+
+            return "emit_sample(%s)" % args
+
+        def parse(self, actstr):
+            desc = (
+                ("group", "OVS_EMIT_SAMPLE_ATTR_GROUP", int),
+                ("cookie", "OVS_EMIT_SAMPLE_ATTR_COOKIE",
+                    lambda x: list(bytearray.fromhex(x)))
+            )
+
+            attrs, actstr = parse_attrs(actstr, desc)
+
+            for attr in attrs:
+                self["attrs"].append(attr)
+
+            return actstr
+
+    class sample(nla):
+        nla_flags = NLA_F_NESTED
+
+        nla_map = (
+            ("OVS_SAMPLE_ATTR_UNSPEC", "none"),
+            ("OVS_SAMPLE_ATTR_PROBABILITY", "uint32"),
+            ("OVS_SAMPLE_ATTR_ACTIONS", "ovsactions"),
+        )
+
+        def dpstr(self, more=False):
+            args = []
+
+            args.append("sample={:.2f}%".format(
+                100 * self.get_attr("OVS_SAMPLE_ATTR_PROBABILITY") /
+                UINT32_MAX))
+
+            actions = self.get_attr("OVS_SAMPLE_ATTR_ACTIONS")
+            if actions:
+                args.append("actions(%s)" % actions.dpstr(more))
+
+            return "sample(%s)" % ",".join(args)
+
+        def parse(self, actstr):
+            def parse_nested_actions(actstr):
+                subacts = ovsactions()
+                parsed_len = subacts.parse(actstr)
+                return subacts, actstr[parsed_len :]
+
+            def percent_to_rate(percent):
+                percent = float(percent.strip('%'))
+                return int(math.floor(UINT32_MAX * (percent / 100.0) + .5))
+
+            desc = (
+                ("sample", "OVS_SAMPLE_ATTR_PROBABILITY", percent_to_rate),
+                ("actions", "OVS_SAMPLE_ATTR_ACTIONS", parse_nested_actions),
+            )
+            attrs, actstr = parse_attrs(actstr, desc)
+
+            for attr in attrs:
+                self["attrs"].append(attr)
+
+            return actstr
+
     class ctact(nla):
         nla_flags = NLA_F_NESTED
 
@@ -643,6 +791,18 @@ class ovsactions(nla):
                 self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
                 parsed = True
 
+            elif parse_starts_block(actstr, "sample(", False):
+                sampleact = self.sample()
+                actstr = sampleact.parse(actstr[len("sample(") : ])
+                self["attrs"].append(["OVS_ACTION_ATTR_SAMPLE", sampleact])
+                parsed = True
+
+            elif parse_starts_block(actstr, "emit_sample(", False):
+                emitact = self.emit_sample()
+                actstr = emitact.parse(actstr[len("emit_sample(") : ])
+                self["attrs"].append(["OVS_ACTION_ATTR_EMIT_SAMPLE", emitact])
+                parsed = True
+
             actstr = actstr[strspn(actstr, ", ") :]
             while parencount > 0:
                 parencount -= 1
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test
       [not found] <20240603185647.2310748-1-amorenoz@redhat.com>
  2024-06-03 18:56 ` [PATCH net-next v2 8/9] selftests: openvswitch: add emit_sample action Adrian Moreno
@ 2024-06-03 18:56 ` Adrian Moreno
  2024-06-05 19:43   ` Simon Horman
  2024-06-14 17:07   ` Aaron Conole
  1 sibling, 2 replies; 8+ messages in thread
From: Adrian Moreno @ 2024-06-03 18:56 UTC (permalink / raw)
  To: netdev
  Cc: aconole, echaudro, horms, i.maximets, dev, Adrian Moreno,
	Pravin B Shelar, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, linux-kselftest, linux-kernel

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.

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"
+
+	# Send a single ping.
+	sleep 1
+	ovs_sbx "test_emit_sample" ip netns exec client ping -I c1 172.31.110.20 -c 1 || return 1
+	sleep 1
+
+	# We should have received one userspace action upcall and 2 psample packets.
+	grep -E "userspace action command" $ovs_dir/s0.out >/dev/null 2>&1 || return 1
+
+	# client -> server samples should only contain the first 14 bytes of the packet.
+	grep -E "rate:4294967295,group:1,cookie:c0ffee data:[0-9a-f]{28}$" \
+			 $ovs_dir/psample.out >/dev/null 2>&1 || return 1
+	grep -E "rate:4294967295,group:2,cookie:eeff0c" \
+			 $ovs_dir/psample.out >/dev/null 2>&1 || return 1
+
+	return 0
+}
+
 # drop_reason test
 # - drop packets and verify the right drop reason is reported
 test_drop_reason() {
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index f8b5362aac8c..44fdeb9491a2 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -27,8 +27,10 @@ try:
     from pyroute2.netlink import genlmsg
     from pyroute2.netlink import nla
     from pyroute2.netlink import nlmsg_atoms
-    from pyroute2.netlink.exceptions import NetlinkError
+    from pyroute2.netlink.event import EventSocket
     from pyroute2.netlink.generic import GenericNetlinkSocket
+    from pyroute2.netlink.nlsocket import Marshal
+    from pyroute2.netlink.exceptions import NetlinkError
     import pyroute2
 
 except ModuleNotFoundError:
@@ -575,13 +577,27 @@ class ovsactions(nla):
                 print_str += "userdata="
                 for f in self.get_attr("OVS_USERSPACE_ATTR_USERDATA"):
                     print_str += "%x." % f
-            if self.get_attr("OVS_USERSPACE_ATTR_TUN_PORT") is not None:
+            if self.get_attr("OVS_USERSPACE_ATTR_EGRESS_TUN_PORT") is not None:
                 print_str += "egress_tun_port=%d" % self.get_attr(
-                    "OVS_USERSPACE_ATTR_TUN_PORT"
+                    "OVS_USERSPACE_ATTR_EGRESS_TUN_PORT"
                 )
             print_str += ")"
             return print_str
 
+        def parse(self, actstr):
+            attrs_desc = (
+                ("pid", "OVS_USERSPACE_ATTR_PID", int),
+                ("userdata", "OVS_USERSPACE_ATTR_USERDATA",
+                    lambda x: list(bytearray.fromhex(x))),
+                ("egress_tun_port", "OVS_USERSPACE_ATTR_EGRESS_TUN_PORT", int)
+            )
+
+            attrs, actstr = parse_attrs(actstr, attrs_desc)
+            for attr in attrs:
+                self["attrs"].append(attr)
+
+            return actstr
+
     def dpstr(self, more=False):
         print_str = ""
 
@@ -803,6 +819,25 @@ class ovsactions(nla):
                 self["attrs"].append(["OVS_ACTION_ATTR_EMIT_SAMPLE", emitact])
                 parsed = True
 
+            elif parse_starts_block(actstr, "userspace(", False):
+                uact = self.userspace()
+                actstr = uact.parse(actstr[len("userpsace(") : ])
+                self["attrs"].append(["OVS_ACTION_ATTR_USERSPACE", uact])
+                parsed = True
+
+            elif parse_starts_block(actstr, "trunc", False):
+                parencount += 1
+                actstr, val = parse_extract_field(
+                    actstr,
+                    "trunc(",
+                    r"([0-9]+)",
+                    int,
+                    False,
+                    None,
+                )
+                self["attrs"].append(["OVS_ACTION_ATTR_TRUNC", val])
+                parsed = True
+
             actstr = actstr[strspn(actstr, ", ") :]
             while parencount > 0:
                 parencount -= 1
@@ -2184,10 +2219,70 @@ class OvsFlow(GenericNetlinkSocket):
         print("MISS upcall[%d/%s]: %s" % (seq, pktpres, keystr), flush=True)
 
     def execute(self, packetmsg):
-        print("userspace execute command")
+        print("userspace execute command", flush=True)
 
     def action(self, packetmsg):
-        print("userspace action command")
+        print("userspace action command", flush=True)
+
+
+class psample_sample(genlmsg):
+    nla_map = (
+        ("PSAMPLE_ATTR_IIFINDEX", "none"),
+        ("PSAMPLE_ATTR_OIFINDEX", "none"),
+        ("PSAMPLE_ATTR_ORIGSIZE", "none"),
+        ("PSAMPLE_ATTR_SAMPLE_GROUP", "uint32"),
+        ("PSAMPLE_ATTR_GROUP_SEQ", "none"),
+        ("PSAMPLE_ATTR_SAMPLE_RATE", "uint32"),
+        ("PSAMPLE_ATTR_DATA", "array(uint8)"),
+        ("PSAMPLE_ATTR_GROUP_REFCOUNT", "none"),
+        ("PSAMPLE_ATTR_TUNNEL", "none"),
+        ("PSAMPLE_ATTR_PAD", "none"),
+        ("PSAMPLE_ATTR_OUT_TC", "none"),
+        ("PSAMPLE_ATTR_OUT_TC_OCC", "none"),
+        ("PSAMPLE_ATTR_LATENCY", "none"),
+        ("PSAMPLE_ATTR_TIMESTAMP", "none"),
+        ("PSAMPLE_ATTR_PROTO", "none"),
+        ("PSAMPLE_ATTR_USER_COOKIE", "array(uint8)"),
+    )
+
+    def dpstr(self):
+        fields = []
+        data = ""
+        for (attr, value) in self["attrs"]:
+            if attr == "PSAMPLE_ATTR_SAMPLE_GROUP":
+                fields.append("group:%d" % value)
+            if attr == "PSAMPLE_ATTR_SAMPLE_RATE":
+                fields.append("rate:%d" % value)
+            if attr == "PSAMPLE_ATTR_USER_COOKIE":
+                value = "".join(format(x, "02x") for x in value)
+                fields.append("cookie:%s" % value)
+            if attr == "PSAMPLE_ATTR_DATA" and len(value) > 0:
+                data = "data:%s" % "".join(format(x, "02x") for x in value)
+
+        return ("%s %s" % (",".join(fields), data)).strip()
+
+
+class psample_msg(Marshal):
+    PSAMPLE_CMD_SAMPLE = 0
+    PSAMPLE_CMD_GET_GROUP = 1
+    PSAMPLE_CMD_NEW_GROUP = 2
+    PSAMPLE_CMD_DEL_GROUP = 3
+    PSAMPLE_CMD_SET_FILTER = 4
+    msg_map = {PSAMPLE_CMD_SAMPLE: psample_sample}
+
+
+class Psample(EventSocket):
+    genl_family = "psample"
+    mcast_groups = ["packets"]
+    marshal_class = psample_msg
+
+    def read_samples(self):
+        while True:
+            try:
+                for msg in self.get():
+                    print(msg.dpstr(), flush=True)
+            except NetlinkError as ne:
+                raise ne
 
 
 def print_ovsdp_full(dp_lookup_rep, ifindex, ndb=NDB(), vpl=OvsVport()):
@@ -2247,7 +2342,7 @@ def main(argv):
         help="Increment 'verbose' output counter.",
         default=0,
     )
-    subparsers = parser.add_subparsers()
+    subparsers = parser.add_subparsers(dest="subcommand")
 
     showdpcmd = subparsers.add_parser("show")
     showdpcmd.add_argument(
@@ -2304,6 +2399,8 @@ def main(argv):
     delfscmd = subparsers.add_parser("del-flows")
     delfscmd.add_argument("flsbr", help="Datapath name")
 
+    subparsers.add_parser("psample")
+
     args = parser.parse_args()
 
     if args.verbose > 0:
@@ -2318,6 +2415,9 @@ def main(argv):
 
     sys.setrecursionlimit(100000)
 
+    if args.subcommand == "psample":
+        Psample().read_samples()
+
     if hasattr(args, "showdp"):
         found = False
         for iface in ndb.interfaces:
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test
  2024-06-03 18:56 ` [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test Adrian Moreno
@ 2024-06-05 19:43   ` Simon Horman
  2024-06-10  9:20     ` Adrián Moreno
  2024-06-14 17:07   ` Aaron Conole
  1 sibling, 1 reply; 8+ messages in thread
From: Simon Horman @ 2024-06-05 19:43 UTC (permalink / raw)
  To: Adrian Moreno
  Cc: netdev, aconole, echaudro, i.maximets, dev, Pravin B Shelar,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shuah Khan, linux-kselftest, linux-kernel

On Mon, Jun 03, 2024 at 08:56:43PM +0200, Adrian Moreno wrote:
> 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.
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>

...

> @@ -803,6 +819,25 @@ class ovsactions(nla):
>                  self["attrs"].append(["OVS_ACTION_ATTR_EMIT_SAMPLE", emitact])
>                  parsed = True
>  
> +            elif parse_starts_block(actstr, "userspace(", False):
> +                uact = self.userspace()
> +                actstr = uact.parse(actstr[len("userpsace(") : ])

nit: userspace

     Flagged by checkpatch.pl --codespell

> +                self["attrs"].append(["OVS_ACTION_ATTR_USERSPACE", uact])
> +                parsed = True
> +
> +            elif parse_starts_block(actstr, "trunc", False):
> +                parencount += 1
> +                actstr, val = parse_extract_field(
> +                    actstr,
> +                    "trunc(",
> +                    r"([0-9]+)",
> +                    int,
> +                    False,
> +                    None,
> +                )
> +                self["attrs"].append(["OVS_ACTION_ATTR_TRUNC", val])
> +                parsed = True
> +
>              actstr = actstr[strspn(actstr, ", ") :]
>              while parencount > 0:
>                  parencount -= 1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test
  2024-06-05 19:43   ` Simon Horman
@ 2024-06-10  9:20     ` Adrián Moreno
  0 siblings, 0 replies; 8+ messages in thread
From: Adrián Moreno @ 2024-06-10  9:20 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, aconole, echaudro, i.maximets, dev, Pravin B Shelar,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shuah Khan, linux-kselftest, linux-kernel

On Wed, Jun 05, 2024 at 08:43:14PM GMT, Simon Horman wrote:
> On Mon, Jun 03, 2024 at 08:56:43PM +0200, Adrian Moreno wrote:
> > 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.
> >
> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>
> ...
>
> > @@ -803,6 +819,25 @@ class ovsactions(nla):
> >                  self["attrs"].append(["OVS_ACTION_ATTR_EMIT_SAMPLE", emitact])
> >                  parsed = True
> >
> > +            elif parse_starts_block(actstr, "userspace(", False):
> > +                uact = self.userspace()
> > +                actstr = uact.parse(actstr[len("userpsace(") : ])
>
> nit: userspace
>
>      Flagged by checkpatch.pl --codespell
>

Thanks. Will fix it.

> > +                self["attrs"].append(["OVS_ACTION_ATTR_USERSPACE", uact])
> > +                parsed = True
> > +
> > +            elif parse_starts_block(actstr, "trunc", False):
> > +                parencount += 1
> > +                actstr, val = parse_extract_field(
> > +                    actstr,
> > +                    "trunc(",
> > +                    r"([0-9]+)",
> > +                    int,
> > +                    False,
> > +                    None,
> > +                )
> > +                self["attrs"].append(["OVS_ACTION_ATTR_TRUNC", val])
> > +                parsed = True
> > +
> >              actstr = actstr[strspn(actstr, ", ") :]
> >              while parencount > 0:
> >                  parencount -= 1
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test
  2024-06-03 18:56 ` [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test Adrian Moreno
  2024-06-05 19:43   ` Simon Horman
@ 2024-06-14 17:07   ` Aaron Conole
  2024-06-17  7:18     ` Adrián Moreno
  1 sibling, 1 reply; 8+ messages in thread
From: Aaron Conole @ 2024-06-14 17:07 UTC (permalink / raw)
  To: Adrian Moreno
  Cc: netdev, echaudro, horms, i.maximets, dev, Pravin B Shelar,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shuah Khan, linux-kselftest, linux-kernel

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

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

> +
> +	# Send a single ping.
> +	sleep 1
> +	ovs_sbx "test_emit_sample" ip netns exec client ping -I c1 172.31.110.20 -c 1 || return 1
> +	sleep 1
> +
> +	# We should have received one userspace action upcall and 2 psample packets.
> +	grep -E "userspace action command" $ovs_dir/s0.out >/dev/null 2>&1 || return 1
> +
> +	# client -> server samples should only contain the first 14 bytes of the packet.
> +	grep -E "rate:4294967295,group:1,cookie:c0ffee data:[0-9a-f]{28}$" \
> +			 $ovs_dir/psample.out >/dev/null 2>&1 || return 1
> +	grep -E "rate:4294967295,group:2,cookie:eeff0c" \
> +			 $ovs_dir/psample.out >/dev/null 2>&1 || return 1
> +
> +	return 0
> +}
> +
>  # drop_reason test
>  # - drop packets and verify the right drop reason is reported
>  test_drop_reason() {
> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> index f8b5362aac8c..44fdeb9491a2 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -27,8 +27,10 @@ try:
>      from pyroute2.netlink import genlmsg
>      from pyroute2.netlink import nla
>      from pyroute2.netlink import nlmsg_atoms
> -    from pyroute2.netlink.exceptions import NetlinkError
> +    from pyroute2.netlink.event import EventSocket
>      from pyroute2.netlink.generic import GenericNetlinkSocket
> +    from pyroute2.netlink.nlsocket import Marshal
> +    from pyroute2.netlink.exceptions import NetlinkError

Why did this get moved?

>      import pyroute2
>  
>  except ModuleNotFoundError:
> @@ -575,13 +577,27 @@ class ovsactions(nla):
>                  print_str += "userdata="
>                  for f in self.get_attr("OVS_USERSPACE_ATTR_USERDATA"):
>                      print_str += "%x." % f
> -            if self.get_attr("OVS_USERSPACE_ATTR_TUN_PORT") is not None:
> +            if self.get_attr("OVS_USERSPACE_ATTR_EGRESS_TUN_PORT") is not None:
>                  print_str += "egress_tun_port=%d" % self.get_attr(
> -                    "OVS_USERSPACE_ATTR_TUN_PORT"
> +                    "OVS_USERSPACE_ATTR_EGRESS_TUN_PORT"

Looks like a bugfix here.

>                  )
>              print_str += ")"
>              return print_str
>  
> +        def parse(self, actstr):
> +            attrs_desc = (
> +                ("pid", "OVS_USERSPACE_ATTR_PID", int),
> +                ("userdata", "OVS_USERSPACE_ATTR_USERDATA",
> +                    lambda x: list(bytearray.fromhex(x))),
> +                ("egress_tun_port", "OVS_USERSPACE_ATTR_EGRESS_TUN_PORT", int)
> +            )
> +
> +            attrs, actstr = parse_attrs(actstr, attrs_desc)
> +            for attr in attrs:
> +                self["attrs"].append(attr)
> +
> +            return actstr
> +
>      def dpstr(self, more=False):
>          print_str = ""
>  
> @@ -803,6 +819,25 @@ class ovsactions(nla):
>                  self["attrs"].append(["OVS_ACTION_ATTR_EMIT_SAMPLE", emitact])
>                  parsed = True
>  
> +            elif parse_starts_block(actstr, "userspace(", False):
> +                uact = self.userspace()
> +                actstr = uact.parse(actstr[len("userpsace(") : ])
> +                self["attrs"].append(["OVS_ACTION_ATTR_USERSPACE", uact])
> +                parsed = True
> +
> +            elif parse_starts_block(actstr, "trunc", False):

This should be "trunc("

> +                parencount += 1
> +                actstr, val = parse_extract_field(
> +                    actstr,
> +                    "trunc(",
> +                    r"([0-9]+)",
> +                    int,
> +                    False,
> +                    None,
> +                )
> +                self["attrs"].append(["OVS_ACTION_ATTR_TRUNC", val])
> +                parsed = True
> +
>              actstr = actstr[strspn(actstr, ", ") :]
>              while parencount > 0:
>                  parencount -= 1
> @@ -2184,10 +2219,70 @@ class OvsFlow(GenericNetlinkSocket):
>          print("MISS upcall[%d/%s]: %s" % (seq, pktpres, keystr), flush=True)
>  
>      def execute(self, packetmsg):
> -        print("userspace execute command")
> +        print("userspace execute command", flush=True)
>  
>      def action(self, packetmsg):
> -        print("userspace action command")
> +        print("userspace action command", flush=True)
> +
> +
> +class psample_sample(genlmsg):
> +    nla_map = (
> +        ("PSAMPLE_ATTR_IIFINDEX", "none"),
> +        ("PSAMPLE_ATTR_OIFINDEX", "none"),
> +        ("PSAMPLE_ATTR_ORIGSIZE", "none"),
> +        ("PSAMPLE_ATTR_SAMPLE_GROUP", "uint32"),
> +        ("PSAMPLE_ATTR_GROUP_SEQ", "none"),
> +        ("PSAMPLE_ATTR_SAMPLE_RATE", "uint32"),
> +        ("PSAMPLE_ATTR_DATA", "array(uint8)"),
> +        ("PSAMPLE_ATTR_GROUP_REFCOUNT", "none"),
> +        ("PSAMPLE_ATTR_TUNNEL", "none"),
> +        ("PSAMPLE_ATTR_PAD", "none"),
> +        ("PSAMPLE_ATTR_OUT_TC", "none"),
> +        ("PSAMPLE_ATTR_OUT_TC_OCC", "none"),
> +        ("PSAMPLE_ATTR_LATENCY", "none"),
> +        ("PSAMPLE_ATTR_TIMESTAMP", "none"),
> +        ("PSAMPLE_ATTR_PROTO", "none"),
> +        ("PSAMPLE_ATTR_USER_COOKIE", "array(uint8)"),
> +    )
> +
> +    def dpstr(self):
> +        fields = []
> +        data = ""
> +        for (attr, value) in self["attrs"]:
> +            if attr == "PSAMPLE_ATTR_SAMPLE_GROUP":
> +                fields.append("group:%d" % value)
> +            if attr == "PSAMPLE_ATTR_SAMPLE_RATE":
> +                fields.append("rate:%d" % value)
> +            if attr == "PSAMPLE_ATTR_USER_COOKIE":
> +                value = "".join(format(x, "02x") for x in value)
> +                fields.append("cookie:%s" % value)
> +            if attr == "PSAMPLE_ATTR_DATA" and len(value) > 0:
> +                data = "data:%s" % "".join(format(x, "02x") for x in value)
> +
> +        return ("%s %s" % (",".join(fields), data)).strip()
> +
> +
> +class psample_msg(Marshal):
> +    PSAMPLE_CMD_SAMPLE = 0
> +    PSAMPLE_CMD_GET_GROUP = 1
> +    PSAMPLE_CMD_NEW_GROUP = 2
> +    PSAMPLE_CMD_DEL_GROUP = 3
> +    PSAMPLE_CMD_SET_FILTER = 4
> +    msg_map = {PSAMPLE_CMD_SAMPLE: psample_sample}
> +
> +
> +class Psample(EventSocket):
> +    genl_family = "psample"
> +    mcast_groups = ["packets"]
> +    marshal_class = psample_msg
> +
> +    def read_samples(self):
> +        while True:
> +            try:
> +                for msg in self.get():
> +                    print(msg.dpstr(), flush=True)
> +            except NetlinkError as ne:
> +                raise ne
>  
>  
>  def print_ovsdp_full(dp_lookup_rep, ifindex, ndb=NDB(), vpl=OvsVport()):
> @@ -2247,7 +2342,7 @@ def main(argv):
>          help="Increment 'verbose' output counter.",
>          default=0,
>      )
> -    subparsers = parser.add_subparsers()
> +    subparsers = parser.add_subparsers(dest="subcommand")
>  
>      showdpcmd = subparsers.add_parser("show")
>      showdpcmd.add_argument(
> @@ -2304,6 +2399,8 @@ def main(argv):
>      delfscmd = subparsers.add_parser("del-flows")
>      delfscmd.add_argument("flsbr", help="Datapath name")
>  
> +    subparsers.add_parser("psample")
> +
>      args = parser.parse_args()
>  
>      if args.verbose > 0:
> @@ -2318,6 +2415,9 @@ def main(argv):
>  
>      sys.setrecursionlimit(100000)
>  
> +    if args.subcommand == "psample":
> +        Psample().read_samples()
> +
>      if hasattr(args, "showdp"):
>          found = False
>          for iface in ndb.interfaces:


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test
  2024-06-14 17:07   ` Aaron Conole
@ 2024-06-17  7:18     ` Adrián Moreno
  2024-06-18  9:08       ` Adrián Moreno
  0 siblings, 1 reply; 8+ messages in thread
From: Adrián Moreno @ 2024-06-17  7:18 UTC (permalink / raw)
  To: Aaron Conole
  Cc: netdev, echaudro, horms, i.maximets, dev, Pravin B Shelar,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shuah Khan, linux-kselftest, linux-kernel

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.

> > +
> > +	# Send a single ping.
> > +	sleep 1
> > +	ovs_sbx "test_emit_sample" ip netns exec client ping -I c1 172.31.110.20 -c 1 || return 1
> > +	sleep 1
> > +
> > +	# We should have received one userspace action upcall and 2 psample packets.
> > +	grep -E "userspace action command" $ovs_dir/s0.out >/dev/null 2>&1 || return 1
> > +
> > +	# client -> server samples should only contain the first 14 bytes of the packet.
> > +	grep -E "rate:4294967295,group:1,cookie:c0ffee data:[0-9a-f]{28}$" \
> > +			 $ovs_dir/psample.out >/dev/null 2>&1 || return 1
> > +	grep -E "rate:4294967295,group:2,cookie:eeff0c" \
> > +			 $ovs_dir/psample.out >/dev/null 2>&1 || return 1
> > +
> > +	return 0
> > +}
> > +
> >  # drop_reason test
> >  # - drop packets and verify the right drop reason is reported
> >  test_drop_reason() {
> > diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> > index f8b5362aac8c..44fdeb9491a2 100644
> > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> > @@ -27,8 +27,10 @@ try:
> >      from pyroute2.netlink import genlmsg
> >      from pyroute2.netlink import nla
> >      from pyroute2.netlink import nlmsg_atoms
> > -    from pyroute2.netlink.exceptions import NetlinkError
> > +    from pyroute2.netlink.event import EventSocket
> >      from pyroute2.netlink.generic import GenericNetlinkSocket
> > +    from pyroute2.netlink.nlsocket import Marshal
> > +    from pyroute2.netlink.exceptions import NetlinkError
>
> Why did this get moved?
>

I guess I first removed it and then added it in the wrong order. I'll
restore it, thanks.

> >      import pyroute2
> >
> >  except ModuleNotFoundError:
> > @@ -575,13 +577,27 @@ class ovsactions(nla):
> >                  print_str += "userdata="
> >                  for f in self.get_attr("OVS_USERSPACE_ATTR_USERDATA"):
> >                      print_str += "%x." % f
> > -            if self.get_attr("OVS_USERSPACE_ATTR_TUN_PORT") is not None:
> > +            if self.get_attr("OVS_USERSPACE_ATTR_EGRESS_TUN_PORT") is not None:
> >                  print_str += "egress_tun_port=%d" % self.get_attr(
> > -                    "OVS_USERSPACE_ATTR_TUN_PORT"
> > +                    "OVS_USERSPACE_ATTR_EGRESS_TUN_PORT"
>
> Looks like a bugfix here.
>

Yep. I'll fold it in with the rest of the usersapce action support into
an independent patch.

> >                  )
> >              print_str += ")"
> >              return print_str
> >
> > +        def parse(self, actstr):
> > +            attrs_desc = (
> > +                ("pid", "OVS_USERSPACE_ATTR_PID", int),
> > +                ("userdata", "OVS_USERSPACE_ATTR_USERDATA",
> > +                    lambda x: list(bytearray.fromhex(x))),
> > +                ("egress_tun_port", "OVS_USERSPACE_ATTR_EGRESS_TUN_PORT", int)
> > +            )
> > +
> > +            attrs, actstr = parse_attrs(actstr, attrs_desc)
> > +            for attr in attrs:
> > +                self["attrs"].append(attr)
> > +
> > +            return actstr
> > +
> >      def dpstr(self, more=False):
> >          print_str = ""
> >
> > @@ -803,6 +819,25 @@ class ovsactions(nla):
> >                  self["attrs"].append(["OVS_ACTION_ATTR_EMIT_SAMPLE", emitact])
> >                  parsed = True
> >
> > +            elif parse_starts_block(actstr, "userspace(", False):
> > +                uact = self.userspace()
> > +                actstr = uact.parse(actstr[len("userpsace(") : ])
> > +                self["attrs"].append(["OVS_ACTION_ATTR_USERSPACE", uact])
> > +                parsed = True
> > +
> > +            elif parse_starts_block(actstr, "trunc", False):
>
> This should be "trunc("
>

Probably, yes. The rest of the actions do look for the initial "(".
Thinking in a generalization of the action parsing, could we just look
for the action name, i.e: "trunc" and let errors be raised inside each
parsing logic? In this case, check if "val is not None" after
"parse_extract_field".

> > +                parencount += 1
> > +                actstr, val = parse_extract_field(
> > +                    actstr,
> > +                    "trunc(",
> > +                    r"([0-9]+)",
> > +                    int,
> > +                    False,
> > +                    None,
> > +                )
> > +                self["attrs"].append(["OVS_ACTION_ATTR_TRUNC", val])
> > +                parsed = True
> > +
> >              actstr = actstr[strspn(actstr, ", ") :]
> >              while parencount > 0:
> >                  parencount -= 1
> > @@ -2184,10 +2219,70 @@ class OvsFlow(GenericNetlinkSocket):
> >          print("MISS upcall[%d/%s]: %s" % (seq, pktpres, keystr), flush=True)
> >
> >      def execute(self, packetmsg):
> > -        print("userspace execute command")
> > +        print("userspace execute command", flush=True)
> >
> >      def action(self, packetmsg):
> > -        print("userspace action command")
> > +        print("userspace action command", flush=True)
> > +
> > +
> > +class psample_sample(genlmsg):
> > +    nla_map = (
> > +        ("PSAMPLE_ATTR_IIFINDEX", "none"),
> > +        ("PSAMPLE_ATTR_OIFINDEX", "none"),
> > +        ("PSAMPLE_ATTR_ORIGSIZE", "none"),
> > +        ("PSAMPLE_ATTR_SAMPLE_GROUP", "uint32"),
> > +        ("PSAMPLE_ATTR_GROUP_SEQ", "none"),
> > +        ("PSAMPLE_ATTR_SAMPLE_RATE", "uint32"),
> > +        ("PSAMPLE_ATTR_DATA", "array(uint8)"),
> > +        ("PSAMPLE_ATTR_GROUP_REFCOUNT", "none"),
> > +        ("PSAMPLE_ATTR_TUNNEL", "none"),
> > +        ("PSAMPLE_ATTR_PAD", "none"),
> > +        ("PSAMPLE_ATTR_OUT_TC", "none"),
> > +        ("PSAMPLE_ATTR_OUT_TC_OCC", "none"),
> > +        ("PSAMPLE_ATTR_LATENCY", "none"),
> > +        ("PSAMPLE_ATTR_TIMESTAMP", "none"),
> > +        ("PSAMPLE_ATTR_PROTO", "none"),
> > +        ("PSAMPLE_ATTR_USER_COOKIE", "array(uint8)"),
> > +    )
> > +
> > +    def dpstr(self):
> > +        fields = []
> > +        data = ""
> > +        for (attr, value) in self["attrs"]:
> > +            if attr == "PSAMPLE_ATTR_SAMPLE_GROUP":
> > +                fields.append("group:%d" % value)
> > +            if attr == "PSAMPLE_ATTR_SAMPLE_RATE":
> > +                fields.append("rate:%d" % value)
> > +            if attr == "PSAMPLE_ATTR_USER_COOKIE":
> > +                value = "".join(format(x, "02x") for x in value)
> > +                fields.append("cookie:%s" % value)
> > +            if attr == "PSAMPLE_ATTR_DATA" and len(value) > 0:
> > +                data = "data:%s" % "".join(format(x, "02x") for x in value)
> > +
> > +        return ("%s %s" % (",".join(fields), data)).strip()
> > +
> > +
> > +class psample_msg(Marshal):
> > +    PSAMPLE_CMD_SAMPLE = 0
> > +    PSAMPLE_CMD_GET_GROUP = 1
> > +    PSAMPLE_CMD_NEW_GROUP = 2
> > +    PSAMPLE_CMD_DEL_GROUP = 3
> > +    PSAMPLE_CMD_SET_FILTER = 4
> > +    msg_map = {PSAMPLE_CMD_SAMPLE: psample_sample}
> > +
> > +
> > +class Psample(EventSocket):
> > +    genl_family = "psample"
> > +    mcast_groups = ["packets"]
> > +    marshal_class = psample_msg
> > +
> > +    def read_samples(self):
> > +        while True:
> > +            try:
> > +                for msg in self.get():
> > +                    print(msg.dpstr(), flush=True)
> > +            except NetlinkError as ne:
> > +                raise ne
> >
> >
> >  def print_ovsdp_full(dp_lookup_rep, ifindex, ndb=NDB(), vpl=OvsVport()):
> > @@ -2247,7 +2342,7 @@ def main(argv):
> >          help="Increment 'verbose' output counter.",
> >          default=0,
> >      )
> > -    subparsers = parser.add_subparsers()
> > +    subparsers = parser.add_subparsers(dest="subcommand")
> >
> >      showdpcmd = subparsers.add_parser("show")
> >      showdpcmd.add_argument(
> > @@ -2304,6 +2399,8 @@ def main(argv):
> >      delfscmd = subparsers.add_parser("del-flows")
> >      delfscmd.add_argument("flsbr", help="Datapath name")
> >
> > +    subparsers.add_parser("psample")
> > +
> >      args = parser.parse_args()
> >
> >      if args.verbose > 0:
> > @@ -2318,6 +2415,9 @@ def main(argv):
> >
> >      sys.setrecursionlimit(100000)
> >
> > +    if args.subcommand == "psample":
> > +        Psample().read_samples()
> > +
> >      if hasattr(args, "showdp"):
> >          found = False
> >          for iface in ndb.interfaces:
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test
  2024-06-17  7:18     ` Adrián Moreno
@ 2024-06-18  9:08       ` Adrián Moreno
  2024-06-18 13:27         ` Aaron Conole
  0 siblings, 1 reply; 8+ messages in thread
From: Adrián Moreno @ 2024-06-18  9:08 UTC (permalink / raw)
  To: Aaron Conole
  Cc: netdev, echaudro, horms, i.maximets, dev, Pravin B Shelar,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shuah Khan, linux-kselftest, linux-kernel

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?

> [...]


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test
  2024-06-18  9:08       ` Adrián Moreno
@ 2024-06-18 13:27         ` Aaron Conole
  0 siblings, 0 replies; 8+ messages in thread
From: Aaron Conole @ 2024-06-18 13:27 UTC (permalink / raw)
  To: Adrián Moreno
  Cc: netdev, echaudro, horms, i.maximets, dev, Pravin B Shelar,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shuah Khan, linux-kselftest, linux-kernel

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

>> [...]


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-06-18 13:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20240603185647.2310748-1-amorenoz@redhat.com>
2024-06-03 18:56 ` [PATCH net-next v2 8/9] selftests: openvswitch: add emit_sample action Adrian Moreno
2024-06-03 18:56 ` [PATCH net-next v2 9/9] selftests: openvswitch: add emit_sample test Adrian Moreno
2024-06-05 19:43   ` Simon Horman
2024-06-10  9:20     ` Adrián Moreno
2024-06-14 17:07   ` Aaron Conole
2024-06-17  7:18     ` Adrián Moreno
2024-06-18  9:08       ` Adrián Moreno
2024-06-18 13:27         ` Aaron Conole

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox