| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 02 Jul 2024 07:45:21 -0400
From: Aaron Conole <aconole@...hat.com>
To: Adrián Moreno <amorenoz@...hat.com>
Cc: netdev@...r.kernel.org, echaudro@...hat.com, horms@...nel.org,
i.maximets@....org, dev@...nvswitch.org, Pravin B Shelar
<pshelar@....org>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Shuah Khan <shuah@...nel.org>,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v7 10/10] selftests: openvswitch: add psample test
Adrián Moreno <amorenoz@...hat.com> writes:
> On Mon, Jul 01, 2024 at 02:38:44PM GMT, Aaron Conole wrote:
>> Adrian Moreno <amorenoz@...hat.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.
>> >
>> > Signed-off-by: Adrian Moreno <amorenoz@...hat.com>
>> > ---
>> > .../selftests/net/openvswitch/openvswitch.sh | 115 +++++++++++++++++-
>> > .../selftests/net/openvswitch/ovs-dpctl.py | 73 ++++++++++-
>> > 2 files changed, 182 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> > index 15bca0708717..02a366e01004 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
>> > + psample psample: Sampling packets with psample"
>> >
>> > info() {
>> > [ $VERBOSE = 0 ] || echo $*
>> > @@ -102,12 +103,21 @@ ovs_netns_spawn_daemon() {
>> > shift
>> > netns=$1
>> > shift
>> > - info "spawning cmd: $*"
>> > - ip netns exec $netns $* >> $ovs_dir/stdout 2>> $ovs_dir/stderr &
>> > + if [ "$netns" == "_default" ]; then
>> > + $* >> $ovs_dir/stdout 2>> $ovs_dir/stderr &
>> > + else
>> > + ip netns exec $netns $* >> $ovs_dir/stdout 2>> $ovs_dir/stderr &
>> > + fi
>> > pid=$!
>> > ovs_sbx "$sbx" on_exit "kill -TERM $pid 2>/dev/null"
>> > }
>> >
>> > +ovs_spawn_daemon() {
>> > + sbx=$1
>> > + shift
>> > + ovs_netns_spawn_daemon $sbx "_default" $*
>> > +}
>> > +
>> > ovs_add_netns_and_veths () {
>> > info "Adding netns attached: sbx:$1 dp:$2 {$3, $4, $5}"
>> > ovs_sbx "$1" ip netns add "$3" || return 1
>> > @@ -170,6 +180,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 +207,92 @@ usage() {
>> > exit 1
>> > }
>> >
>> > +
>> > +# psample test
>> > +# - use psample to observe packets
>> > +test_psample() {
>> > + sbx_add "test_psample" || return $?
>> > +
>> > + # Add a datapath with per-vport dispatching.
>> > + ovs_add_dp "test_psample" psample -V 2:1 || return 1
>> > +
>> > + info "create namespaces"
>> > + ovs_add_netns_and_veths "test_psample" "psample" \
>> > + client c0 c1 172.31.110.10/24 -u || return 1
>> > + ovs_add_netns_and_veths "test_psample" "psample" \
>> > + server s0 s1 172.31.110.20/24 -u || return 1
>> > +
>> > + # Check if psample actions can be configured.
>> > + ovs_add_flow "test_psample" psample \
>> > + 'in_port(1),eth(),eth_type(0x0806),arp()' 'psample(group=1)'
>>
>> Might be good to redirect this stdout/stderr line to /dev/null -
>> otherwise on an unsupported system there will be the following extra
>> splat:
>>
>> Traceback (most recent call last):
>> File "/home/aconole/git/linux/tools/testing/selftests/net/openvswitch/ovs-dpctl.py", line 2774, in <module>
>> sys.exit(main(sys.argv))
>> ...
>> File "/usr/lib/python3.12/site-packages/pyroute2/netlink/nlsocket.py", line 489, in get
>> raise msg['header']['error']
>> pyroute2.netlink.exceptions.NetlinkError: (22, 'Invalid argument')
>>
>
> I thought knowing the return value was kind of useful but sure, we can
> redirect it to /dev/null.
>
>> > + if [ $? == 1 ]; then
>> > + info "no support for psample - skipping"
>> > + ovs_exit_sig
>> > + return $ksft_skip
>> > + fi
>> > +
>> > + ovs_del_flows "test_psample" psample
>> > +
>> > + # Test action verification.
>> > + OLDIFS=$IFS
>> > + IFS='*'
>> > + min_key='in_port(1),eth(),eth_type(0x0800),ipv4()'
>> > + for testcase in \
>> > + "cookie to
>> > large"*"psample(group=1,cookie=1615141312111009080706050403020100)"
>> > \
>> > + "no group with cookie"*"psample(cookie=abcd)" \
>> > + "no group"*"psample()";
>> > + do
>> > + set -- $testcase;
>> > + ovs_test_flow_fails "test_psample" psample $min_key $2
>> > + if [ $? == 1 ]; then
>> > + info "failed - $1"
>> > + return 1
>> > + fi
>> > + done
>> > + IFS=$OLDIFS
>> > +
>> > + ovs_del_flows "test_psample" psample
>> > + # Allow ARP
>> > + ovs_add_flow "test_psample" psample \
>> > + 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
>> > + ovs_add_flow "test_psample" psample \
>> > + 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
>> > +
>> > + # Sample first 14 bytes of all traffic.
>> > + ovs_add_flow "test_psample" psample \
>> > + "in_port(1),eth(),eth_type(0x0800),ipv4()" \
>> > + "trunc(14),psample(group=1,cookie=c0ffee),2"
>> > +
>> > + # Sample all traffic. In this case, use a sample() action with both
>> > + # psample 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_psample" psample \
>> > + "in_port(2),eth(),eth_type(0x0800),ipv4()" \
>> > +
>> > "sample(sample=100%,actions(psample(group=2,cookie=eeff0c),userspace(pid=${nlpid},userdata=eeff0c))),1"
>> > +
>> > + # Record psample data.
>> > + ovs_spawn_daemon "test_psample" python3 $ovs_base/ovs-dpctl.py
>> > psample-events
>> > +
>> > + # Send a single ping.
>> > + sleep 1
>> > + ovs_sbx "test_psample" 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
>>
>> I wonder if it would be better to check a few times instead of the one
>> shot sleep. There are some constrained environments that may run this
>> test, and if you're worried about some kinds of races, maybe it makes
>> sense to check a few times?
>
> Yes. I thought about that. There are other sleeps in this file that I
> was planning to replace with a "wait_until()" function. Should we do
> this as a follow-up patch to also cover the other instances?
That makes sense to me.
>>
>> Outside of that:
>>
>> Reviewed-by: Aaron Conole <aconole@...hat.com>
>>
>> > +
>> > + # 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/stdout >/dev/null 2>&1 || return 1
>> > + grep -E "rate:4294967295,group:2,cookie:eeff0c" \
>> > + $ovs_dir/stdout >/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 e8dc9af10d4d..1e15b0818074 100644
>> > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> > @@ -28,8 +28,10 @@ try:
>> > from pyroute2.netlink import genlmsg
>> > from pyroute2.netlink import nla
>> > from pyroute2.netlink import nlmsg_atoms
>> > + from pyroute2.netlink.event import EventSocket
>> > from pyroute2.netlink.exceptions eimport NetlinkError
>> > from pyroute2.netlink.generic import GenericNetlinkSocket
>> > + from pyroute2.netlink.nlsocket import Marshal
>> > import pyroute2
>> > import pyroute2.iproute
>> >
>> > @@ -2460,10 +2462,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 PsampleEvent(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()):
>> > @@ -2530,7 +2592,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(
>> > @@ -2605,6 +2667,8 @@ def main(argv):
>> > delfscmd = subparsers.add_parser("del-flows")
>> > delfscmd.add_argument("flsbr", help="Datapath name")
>> >
>> > + subparsers.add_parser("psample-events")
>> > +
>> > args = parser.parse_args()
>> >
>> > if args.verbose > 0:
>> > @@ -2619,6 +2683,9 @@ def main(argv):
>> >
>> > sys.setrecursionlimit(100000)
>> >
>> > + if args.subcommand == "psample-events":
>> > + PsampleEvent().read_samples()
>> > +
>> > if hasattr(args, "showdp"):
>> > found = False
>> > for iface in ndb.interfaces:
>>
Powered by blists - more mailing lists