fixing PEP8 warning in the selftest bpf python files.
Alice Ferrazzi (7):
selftest/bpf: Fix trailing semicolon in the statement
selftest/bpf: optimize import
selftest/bpf: PEP 8: multiple statements on one line (colon)
selftest/bpf: test_offload PEP8 format style fix
selftest/bpf: Fix PEP8 ambiguous variable name
selftest/bpf: remove redundant parenthesis
selftest/bpf: fix E501 line too long
tools/testing/selftests/bpf/tcp_client.py | 24 +++--
tools/testing/selftests/bpf/tcp_server.py | 27 +++--
tools/testing/selftests/bpf/test_offload.py | 107 ++++++++++++++------
3 files changed, 109 insertions(+), 49 deletions(-)
--
2.19.2
fix python PEP8 style issue
Signed-off-by: Alice Ferrazzi <[email protected]>
---
tools/testing/selftests/bpf/tcp_client.py | 4 ++--
tools/testing/selftests/bpf/tcp_server.py | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/tcp_client.py b/tools/testing/selftests/bpf/tcp_client.py
index 7f8200a8702b..c3c44633c801 100755
--- a/tools/testing/selftests/bpf/tcp_client.py
+++ b/tools/testing/selftests/bpf/tcp_client.py
@@ -24,7 +24,7 @@ def send(sock, s):
try: n = sock.send(s)
except (socket.error) as e: n = 0
if n == 0:
- return count;
+ return count
count += n
return count
@@ -45,7 +45,7 @@ while n < 1000:
buf += b'+'
n += 1
-sock.settimeout(1);
+sock.settimeout(1)
n = send(sock, buf)
n = read(sock, 500)
sys.exit(0)
diff --git a/tools/testing/selftests/bpf/tcp_server.py b/tools/testing/selftests/bpf/tcp_server.py
index b39903fca4c8..183d07509a0a 100755
--- a/tools/testing/selftests/bpf/tcp_server.py
+++ b/tools/testing/selftests/bpf/tcp_server.py
@@ -24,7 +24,7 @@ def send(sock, s):
try: n = sock.send(s)
except (socket.error) as e: n = 0
if n == 0:
- return count;
+ return count
count += n
return count
@@ -72,7 +72,7 @@ while True:
address = str(address[0])
readList.append(clientSocket)
else:
- sock.settimeout(1);
+ sock.settimeout(1)
s = read(sock, 1000)
n = send(sock, buf)
sock.close()
--
2.19.2
Fix PEP8 warnings
Signed-off-by: Alice Ferrazzi <[email protected]>
---
tools/testing/selftests/bpf/tcp_client.py | 7 +++----
tools/testing/selftests/bpf/tcp_server.py | 7 ++++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/tcp_client.py b/tools/testing/selftests/bpf/tcp_client.py
index c3c44633c801..ad3de27cd605 100755
--- a/tools/testing/selftests/bpf/tcp_client.py
+++ b/tools/testing/selftests/bpf/tcp_client.py
@@ -3,10 +3,9 @@
# SPDX-License-Identifier: GPL-2.0
#
-import sys, os, os.path, getopt
-import socket, time
-import subprocess
-import select
+import socket
+import sys
+
def read(sock, n):
buf = b''
diff --git a/tools/testing/selftests/bpf/tcp_server.py b/tools/testing/selftests/bpf/tcp_server.py
index 183d07509a0a..f7491517cc49 100755
--- a/tools/testing/selftests/bpf/tcp_server.py
+++ b/tools/testing/selftests/bpf/tcp_server.py
@@ -3,10 +3,11 @@
# SPDX-License-Identifier: GPL-2.0
#
-import sys, os, os.path, getopt
-import socket, time
-import subprocess
+import socket
+import os.path
import select
+import sys
+
def read(sock, n):
buf = b''
--
2.19.2
Reformat the file for fixing PEP 8 style
Signed-off-by: Alice Ferrazzi <[email protected]>
---
tools/testing/selftests/bpf/tcp_client.py | 13 +++++++++----
tools/testing/selftests/bpf/tcp_server.py | 16 +++++++++++-----
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/tcp_client.py b/tools/testing/selftests/bpf/tcp_client.py
index ad3de27cd605..31f022bf06fa 100755
--- a/tools/testing/selftests/bpf/tcp_client.py
+++ b/tools/testing/selftests/bpf/tcp_client.py
@@ -11,17 +11,22 @@ def read(sock, n):
buf = b''
while len(buf) < n:
rem = n - len(buf)
- try: s = sock.recv(rem)
- except (socket.error) as e: return b''
+ try:
+ s = sock.recv(rem)
+ except (socket.error) as e:
+ return b''
buf += s
return buf
+
def send(sock, s):
total = len(s)
count = 0
while count < total:
- try: n = sock.send(s)
- except (socket.error) as e: n = 0
+ try:
+ n = sock.send(s)
+ except (socket.error) as e:
+ n = 0
if n == 0:
return count
count += n
diff --git a/tools/testing/selftests/bpf/tcp_server.py b/tools/testing/selftests/bpf/tcp_server.py
index f7491517cc49..af79210249af 100755
--- a/tools/testing/selftests/bpf/tcp_server.py
+++ b/tools/testing/selftests/bpf/tcp_server.py
@@ -13,17 +13,22 @@ def read(sock, n):
buf = b''
while len(buf) < n:
rem = n - len(buf)
- try: s = sock.recv(rem)
- except (socket.error) as e: return b''
+ try:
+ s = sock.recv(rem)
+ except (socket.error) as e:
+ return b''
buf += s
return buf
+
def send(sock, s):
total = len(s)
count = 0
while count < total:
- try: n = sock.send(s)
- except (socket.error) as e: n = 0
+ try:
+ n = sock.send(s)
+ except (socket.error) as e:
+ n = 0
if n == 0:
return count
count += n
@@ -42,7 +47,8 @@ HostName = socket.gethostname()
serverSocket = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
host = socket.gethostname()
-try: serverSocket.bind((host, 0))
+try:
+ serverSocket.bind((host, 0))
except socket.error as msg:
print('bind fails: ' + str(msg))
--
2.19.2
Signed-off-by: Alice Ferrazzi <[email protected]>
---
tools/testing/selftests/bpf/test_offload.py | 68 ++++++++++++++++-----
1 file changed, 54 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index d59642e70f56..f80c4f13991d 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -29,25 +29,30 @@ log_level = 1
skip_extack = False
bpf_test_dir = os.path.dirname(os.path.realpath(__file__))
pp = pprint.PrettyPrinter()
-devs = [] # devices we created for clean up
-files = [] # files to be removed
-netns = [] # net namespaces to be removed
+devs = [] # devices we created for clean up
+files = [] # files to be removed
+netns = [] # net namespaces to be removed
+
def log_get_sec(level=0):
return "*" * (log_level + level)
+
def log_level_inc(add=1):
global log_level
log_level += add
+
def log_level_dec(sub=1):
global log_level
log_level -= sub
+
def log_level_set(level):
global log_level
log_level = level
+
def log(header, data, level=None):
"""
Output to an optional log.
@@ -67,6 +72,7 @@ def log(header, data, level=None):
logfile.write("\n")
logfile.write(data)
+
def skip(cond, msg):
if not cond:
return
@@ -74,6 +80,7 @@ def skip(cond, msg):
log("SKIP: " + msg, "", level=1)
os.sys.exit(0)
+
def fail(cond, msg):
if not cond:
return
@@ -81,11 +88,13 @@ def fail(cond, msg):
log("FAIL: " + msg, "", level=1)
os.sys.exit(1)
+
def start_test(msg):
log(msg, "", level=1)
log_level_inc()
print(msg)
+
def cmd(cmd, shell=True, include_stderr=False, background=False, fail=True):
"""
Run a command in subprocess and return tuple of (retval, stdout);
@@ -101,6 +110,7 @@ def cmd(cmd, shell=True, include_stderr=False, background=False, fail=True):
return cmd_result(proc, include_stderr=include_stderr, fail=fail)
+
def cmd_result(proc, include_stderr=False, fail=False):
stdout, stderr = proc.communicate()
stdout = stdout.decode("utf-8")
@@ -128,11 +138,13 @@ def cmd_result(proc, include_stderr=False, fail=False):
else:
return proc.returncode, stdout
+
def rm(f):
cmd("rm -f %s" % (f))
if f in files:
files.remove(f)
+
def tool(name, args, flags, JSON=True, ns="", fail=True, include_stderr=False):
params = ""
if JSON:
@@ -158,10 +170,12 @@ def tool(name, args, flags, JSON=True, ns="", fail=True, include_stderr=False):
else:
return ret, out
+
def bpftool(args, JSON=True, ns="", fail=True, include_stderr=False):
- return tool("bpftool", args, {"json":"-p"}, JSON=JSON, ns=ns,
+ return tool("bpftool", args, {"json": "-p"}, JSON=JSON, ns=ns,
fail=fail, include_stderr=include_stderr)
+
def bpftool_prog_list(expected=None, ns=""):
_, progs = bpftool("prog show", JSON=True, ns=ns, fail=True)
# Remove the base progs
@@ -174,6 +188,7 @@ def bpftool_prog_list(expected=None, ns=""):
(len(progs), expected))
return progs
+
def bpftool_map_list(expected=None, ns=""):
_, maps = bpftool("map show", JSON=True, ns=ns, fail=True)
# Remove the base maps
@@ -186,6 +201,7 @@ def bpftool_map_list(expected=None, ns=""):
(len(maps), expected))
return maps
+
def bpftool_prog_list_wait(expected=0, n_retry=20):
for i in range(n_retry):
nprogs = len(bpftool_prog_list())
@@ -194,6 +210,7 @@ def bpftool_prog_list_wait(expected=0, n_retry=20):
time.sleep(0.05)
raise Exception("Time out waiting for program counts to stabilize want %d, have %d" % (expected, nprogs))
+
def bpftool_map_list_wait(expected=0, n_retry=20):
for i in range(n_retry):
nmaps = len(bpftool_map_list())
@@ -202,6 +219,7 @@ def bpftool_map_list_wait(expected=0, n_retry=20):
time.sleep(0.05)
raise Exception("Time out waiting for map counts to stabilize want %d, have %d" % (expected, nmaps))
+
def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
fail=True, include_stderr=False):
args = "prog load %s %s" % (os.path.join(bpf_test_dir, sample), file_name)
@@ -217,28 +235,35 @@ def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
files.append(file_name)
return res
+
def ip(args, force=False, JSON=True, ns="", fail=True, include_stderr=False):
if force:
args = "-force " + args
- return tool("ip", args, {"json":"-j"}, JSON=JSON, ns=ns,
+ return tool("ip", args, {"json": "-j"}, JSON=JSON, ns=ns,
fail=fail, include_stderr=include_stderr)
+
def tc(args, JSON=True, ns="", fail=True, include_stderr=False):
- return tool("tc", args, {"json":"-p"}, JSON=JSON, ns=ns,
+ return tool("tc", args, {"json": "-p"}, JSON=JSON, ns=ns,
fail=fail, include_stderr=include_stderr)
+
def ethtool(dev, opt, args, fail=True):
return cmd("ethtool %s %s %s" % (opt, dev["ifname"], args), fail=fail)
-def bpf_obj(name, sec=".text", path=bpf_test_dir,):
+
+def bpf_obj(name, sec=".text", path=bpf_test_dir, ):
return "obj %s sec %s" % (os.path.join(path, name), sec)
+
def bpf_pinned(name):
return "pinned %s" % (name)
+
def bpf_bytecode(bytecode):
return "bytecode \"%s\"" % (bytecode)
+
def mknetns(n_retry=10):
for i in range(n_retry):
name = ''.join([random.choice(string.ascii_letters) for i in range(8)])
@@ -248,12 +273,14 @@ def mknetns(n_retry=10):
return name
return None
+
def int2str(fmt, val):
ret = []
for b in struct.pack(fmt, val):
ret.append(int(b))
return " ".join(map(lambda x: str(x), ret))
+
def str2int(strtab):
inttab = []
for i in strtab:
@@ -268,6 +295,7 @@ def str2int(strtab):
(len(strtab)))
return struct.unpack(fmt, ba)[0]
+
class DebugfsDir:
"""
Class for accessing DebugFS directories as a dictionary.
@@ -318,6 +346,7 @@ class DebugfsDir:
return dfs
+
class NetdevSim:
"""
Class for netdevsim netdevice and its attributes.
@@ -340,9 +369,9 @@ class NetdevSim:
def _netdevsim_create(self):
link = "" if self.link is None else "link " + self.link.dev['ifname']
- _, old = ip("link show")
+ _, old = ip("link show")
ip("link add sim%d {link} type netdevsim".format(link=link))
- _, new = ip("link show")
+ _, new = ip("link show")
for dev in new:
f = filter(lambda x: x["ifname"] == dev["ifname"], old)
@@ -384,7 +413,8 @@ class NetdevSim:
if nbound == bound and nprogs == total:
return
time.sleep(0.05)
- raise Exception("Time out waiting for program counts to stabilize want %d/%d, have %d bound, %d loaded" % (bound, total, nbound, nprogs))
+ raise Exception("Time out waiting for program counts to stabilize want %d/%d, have %d bound, %d loaded" % (
+ bound, total, nbound, nprogs))
def set_ns(self, ns):
name = "1" if ns == "" else ns
@@ -473,7 +503,7 @@ class NetdevSim:
if chain is not None:
spec += " chain %d" % (chain)
- return tc("filter {op} dev {dev} {qdisc} {spec} {cls} {params}"\
+ return tc("filter {op} dev {dev} {qdisc} {spec} {cls} {params}"
.format(op=op, dev=self['ifname'], qdisc=qdisc, spec=spec,
cls=cls, params=params),
fail=fail, include_stderr=include_stderr)
@@ -502,6 +532,7 @@ class NetdevSim:
args = "hw-tc-offload %s" % ("on" if enable else "off")
return ethtool(self, "-K", args, fail=fail)
+
################################################################################
def clean_up():
global files, netns, devs
@@ -515,6 +546,7 @@ def clean_up():
files = []
netns = []
+
def pin_prog(file_name, idx=0):
progs = bpftool_prog_list(expected=(idx + 1))
prog = progs[idx]
@@ -523,6 +555,7 @@ def pin_prog(file_name, idx=0):
return file_name, bpf_pinned(file_name)
+
def pin_map(file_name, idx=0, expected=1):
maps = bpftool_map_list(expected=expected)
m = maps[idx]
@@ -531,6 +564,7 @@ def pin_map(file_name, idx=0, expected=1):
return file_name, bpf_pinned(file_name)
+
def check_dev_info_removed(prog_file=None, map_file=None):
bpftool_prog_list(expected=0)
ret, err = bpftool("prog show pin %s" % (prog_file), fail=False)
@@ -546,6 +580,7 @@ def check_dev_info_removed(prog_file=None, map_file=None):
"Showing map with removed device expected ENODEV, error is %s" %
(err["error"]))
+
def check_dev_info(other_ns, ns, prog_file=None, map_file=None, removed=False):
progs = bpftool_prog_list(expected=1, ns=ns)
prog = progs[0]
@@ -568,6 +603,7 @@ def check_dev_info(other_ns, ns, prog_file=None, map_file=None, removed=False):
fail("dev" not in m.keys(), "Device parameters not reported")
fail(dev != m["dev"], "Map's device different than program's")
+
def check_extack(output, reference, args):
if skip_extack:
return
@@ -575,13 +611,16 @@ def check_extack(output, reference, args):
comp = len(lines) >= 2 and lines[1] == 'Error: ' + reference
fail(not comp, "Missing or incorrect netlink extack message")
+
def check_extack_nsim(output, reference, args):
check_extack(output, "netdevsim: " + reference, args)
+
def check_no_extack(res, needle):
fail((res[1] + res[2]).count(needle) or (res[1] + res[2]).count("Warning:"),
"Found '%s' in command output, leaky extack?" % (needle))
+
def check_verifier_log(output, reference):
lines = output.split("\n")
for l in reversed(lines):
@@ -589,6 +628,7 @@ def check_verifier_log(output, reference):
return
fail(True, "Missing or incorrect message from netdevsim in verifier log")
+
def test_spurios_extack(sim, obj, skip_hw, needle):
res = sim.cls_bpf_add_filter(obj, prio=1, handle=1, skip_hw=skip_hw,
include_stderr=True)
@@ -1056,7 +1096,7 @@ try:
sim = NetdevSim()
map_obj = bpf_obj("sample_map_ret0.o")
start_test("Test loading program with maps...")
- sim.set_xdp(map_obj, "offload", JSON=False) # map fixup msg breaks JSON
+ sim.set_xdp(map_obj, "offload", JSON=False) # map fixup msg breaks JSON
start_test("Test bpftool bound info reporting (own ns)...")
check_dev_info(False, "")
@@ -1087,7 +1127,7 @@ try:
sim = NetdevSim()
start_test("Test map update (no flags)...")
- sim.set_xdp(map_obj, "offload", JSON=False) # map fixup msg breaks JSON
+ sim.set_xdp(map_obj, "offload", JSON=False) # map fixup msg breaks JSON
maps = bpftool_map_list(expected=2)
array = maps[0] if maps[0]["type"] == "array" else maps[1]
htab = maps[0] if maps[0]["type"] == "hash" else maps[1]
@@ -1168,7 +1208,7 @@ try:
sim.remove()
sim = NetdevSim()
- sim.set_xdp(map_obj, "offload", JSON=False) # map fixup msg breaks JSON
+ sim.set_xdp(map_obj, "offload", JSON=False) # map fixup msg breaks JSON
sim.remove()
bpftool_map_list_wait(expected=0)
--
2.19.2
Signed-off-by: Alice Ferrazzi <[email protected]>
---
tools/testing/selftests/bpf/test_offload.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 0f9130ebfd2c..b06cc0eea0eb 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -140,7 +140,7 @@ def cmd_result(proc, include_stderr=False, fail=False):
def rm(f):
- cmd("rm -f %s" % (f))
+ cmd("rm -f %s" % f)
if f in files:
files.remove(f)
--
2.19.2
fix PEP8 style issues
Signed-off-by: Alice Ferrazzi <[email protected]>
---
tools/testing/selftests/bpf/test_offload.py | 35 +++++++++++++--------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index b06cc0eea0eb..3f78da0f87d5 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -208,7 +208,8 @@ def bpftool_prog_list_wait(expected=0, n_retry=20):
if nprogs == expected:
return
time.sleep(0.05)
- raise Exception("Time out waiting for program counts to stabilize want %d, have %d" % (expected, nprogs))
+ raise Exception("Time out waiting for program counts \
+to stabilize want %d, have %d" % (expected, nprogs))
def bpftool_map_list_wait(expected=0, n_retry=20):
@@ -217,7 +218,8 @@ def bpftool_map_list_wait(expected=0, n_retry=20):
if nmaps == expected:
return
time.sleep(0.05)
- raise Exception("Time out waiting for map counts to stabilize want %d, have %d" % (expected, nmaps))
+ raise Exception("Time out waiting for map counts to \
+stabilize want %d, have %d" % (expected, nmaps))
def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
@@ -413,7 +415,8 @@ class NetdevSim:
if nbound == bound and nprogs == total:
return
time.sleep(0.05)
- raise Exception("Time out waiting for program counts to stabilize want %d/%d, have %d bound, %d loaded" % (
+ raise Exception("Time out waiting for program counts to \
+stabilize want %d/%d, have %d bound, %d loaded" % (
bound, total, nbound, nprogs))
def set_ns(self, ns):
@@ -533,7 +536,7 @@ class NetdevSim:
return ethtool(self, "-K", args, fail=fail)
-################################################################################
+###############################################################################
def clean_up():
global files, netns, devs
@@ -617,7 +620,8 @@ def check_extack_nsim(output, reference, args):
def check_no_extack(res, needle):
- fail((res[1] + res[2]).count(needle) or (res[1] + res[2]).count("Warning:"),
+ fail((res[1] + res[2]).count(needle) or (res[1] +
+ res[2]).count("Warning:"),
"Found '%s' in command output, leaky extack?" % (needle))
@@ -778,8 +782,9 @@ try:
start_test("Test TC replace bad flags...")
for i in range(3):
for j in range(3):
- ret, _ = sim.cls_bpf_add_filter(obj, op="replace", prio=1, handle=1,
- skip_sw=(j == 1), skip_hw=(j == 2),
+ ret, _ = sim.cls_bpf_add_filter(obj, op="replace", prio=1,
+ handle=1, skip_sw=(j == 1),
+ skip_hw=(j == 2),
fail=False)
fail(bool(ret) != bool(j),
"Software TC incorrect load in replace test, iteration %d" %
@@ -824,9 +829,11 @@ try:
fail(dprog["state"] != "xlated", "Offloaded program state not translated")
fail(dprog["loaded"] != "Y", "Offloaded program is not loaded")
- start_test("Test disabling TC offloads is rejected while filters installed...")
+ start_test("Test disabling TC offloads is rejected while \
+filters installed...")
ret, _ = sim.set_ethtool_tc_offloads(False, fail=False)
- fail(ret == 0, "Driver should refuse to disable TC offloads with filters installed...")
+ fail(ret == 0, "Driver should refuse to disable TC offloads with filters \
+installed...")
start_test("Test qdisc removal frees things...")
sim.tc_flush_filters()
@@ -922,7 +929,8 @@ try:
offload = bpf_pinned("/sys/fs/bpf/offload")
ret, _, err = sim.set_xdp(offload, "drv", fail=False, include_stderr=True)
fail(ret == 0, "attached offloaded XDP program to drv")
- check_extack(err, "using device-bound program without HW_MODE flag is not supported.", args)
+ check_extack(err, "using device-bound program without HW_MODE flag is not \
+supported.", args)
rm("/sys/fs/bpf/offload")
sim.wait_for_flush()
@@ -1072,8 +1080,8 @@ try:
delay_msec = 500
sim.dfs["bpf_bind_verifier_delay"] = delay_msec
start = time.time()
- cmd_line = "tc filter add dev %s ingress bpf %s da skip_sw" % \
- (sim['ifname'], obj)
+ cmd_line = "tc filter add dev %s ingress bpf %s da skip_sw" %
+ (sim['ifname'], obj)
tc_proc = cmd(cmd_line, background=True, fail=False)
# Wait for the verifier to start
while sim.dfs_num_bound_progs() <= 2:
@@ -1244,7 +1252,8 @@ try:
ret, _ = simA.set_xdp(progB, "offload", force=True, JSON=False, fail=False)
fail(ret == 0, "cross-ASIC program allowed")
for d in simB:
- ret, _ = d.set_xdp(progA, "offload", force=True, JSON=False, fail=False)
+ ret, _ = d.set_xdp(progA, "offload", force=True, JSON=False,
+ fail=False)
fail(ret == 0, "cross-ASIC program allowed")
start_test("Test multi-dev ASIC cross-dev install...")
--
2.19.2
Signed-off-by: Alice Ferrazzi <[email protected]>
---
tools/testing/selftests/bpf/test_offload.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index f80c4f13991d..0f9130ebfd2c 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -623,8 +623,8 @@ def check_no_extack(res, needle):
def check_verifier_log(output, reference):
lines = output.split("\n")
- for l in reversed(lines):
- if l == reference:
+ for line in reversed(lines):
+ if line == reference:
return
fail(True, "Missing or incorrect message from netdevsim in verifier log")
--
2.19.2
[ +Jakub ]
On 12/11/2018 12:56 PM, Alice Ferrazzi wrote:
> fix PEP8 style issues
>
> Signed-off-by: Alice Ferrazzi <[email protected]>
> ---
> tools/testing/selftests/bpf/test_offload.py | 35 +++++++++++++--------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
> index b06cc0eea0eb..3f78da0f87d5 100755
> --- a/tools/testing/selftests/bpf/test_offload.py
> +++ b/tools/testing/selftests/bpf/test_offload.py
> @@ -208,7 +208,8 @@ def bpftool_prog_list_wait(expected=0, n_retry=20):
> if nprogs == expected:
> return
> time.sleep(0.05)
> - raise Exception("Time out waiting for program counts to stabilize want %d, have %d" % (expected, nprogs))
> + raise Exception("Time out waiting for program counts \
> +to stabilize want %d, have %d" % (expected, nprogs))
>
>
> def bpftool_map_list_wait(expected=0, n_retry=20):
> @@ -217,7 +218,8 @@ def bpftool_map_list_wait(expected=0, n_retry=20):
> if nmaps == expected:
> return
> time.sleep(0.05)
> - raise Exception("Time out waiting for map counts to stabilize want %d, have %d" % (expected, nmaps))
> + raise Exception("Time out waiting for map counts to \
> +stabilize want %d, have %d" % (expected, nmaps))
>
>
> def bpftool_prog_load(sample, file_name, maps=[], prog_type="xdp", dev=None,
> @@ -413,7 +415,8 @@ class NetdevSim:
> if nbound == bound and nprogs == total:
> return
> time.sleep(0.05)
> - raise Exception("Time out waiting for program counts to stabilize want %d/%d, have %d bound, %d loaded" % (
> + raise Exception("Time out waiting for program counts to \
> +stabilize want %d/%d, have %d bound, %d loaded" % (
> bound, total, nbound, nprogs))
I'll leave the test_offload.py ones up to Jakub, but to me it seems this particular
change here would actually make the code look worse and harder to grep for error
messages, so my preference would be to at least leave the error messages as-is. Also
seems it's not a 'must' in pep8 [0].
[0] https://www.python.org/dev/peps/pep-0008/#maximum-line-length
Thanks,
Daniel
On Tue, 11 Dec 2018 20:56:04 +0900, Alice Ferrazzi wrote:
> Signed-off-by: Alice Ferrazzi <[email protected]>
Thanks, are you just running pylint to catch those? I tried in the
past but it's very noisy and it lacked "understanding" of some python3
modules. Would you mind giving us a quick 101 of how to catch such
errors? :)
> -def bpf_obj(name, sec=".text", path=bpf_test_dir,):
> +
> +def bpf_obj(name, sec=".text", path=bpf_test_dir, ):
> return "obj %s sec %s" % (os.path.join(path, name), sec)
>
I think this one is just a typo. bpf_obj() seems to always get called
with one param, no?
On Tue, 11 Dec 2018 20:56:06 +0900, Alice Ferrazzi wrote:
> Signed-off-by: Alice Ferrazzi <[email protected]>
> ---
> tools/testing/selftests/bpf/test_offload.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
> index 0f9130ebfd2c..b06cc0eea0eb 100755
> --- a/tools/testing/selftests/bpf/test_offload.py
> +++ b/tools/testing/selftests/bpf/test_offload.py
> @@ -140,7 +140,7 @@ def cmd_result(proc, include_stderr=False, fail=False):
>
>
> def rm(f):
> - cmd("rm -f %s" % (f))
> + cmd("rm -f %s" % f)
> if f in files:
> files.remove(f)
>
Is this in PEP8, too?
On Wed, 12 Dec 2018 11:24:02 +0100, Daniel Borkmann wrote:
> > - raise Exception("Time out waiting for program counts to stabilize want %d/%d, have %d bound, %d loaded" % (
> > + raise Exception("Time out waiting for program counts to \
> > +stabilize want %d/%d, have %d bound, %d loaded" % (
> > bound, total, nbound, nprogs))
>
> I'll leave the test_offload.py ones up to Jakub, but to me it seems
> this particular change here would actually make the code look worse
> and harder to grep for error messages, so my preference would be to
> at least leave the error messages as-is. Also seems it's not a 'must'
> in pep8 [0].
>
> [0] https://www.python.org/dev/peps/pep-0008/#maximum-line-length
Thanks, agreed, this has been run through checkpatch, so the line
lengths should be fine (and we don't want error messages split).
On 12/12/18 19:04, Jakub Kicinski wrote:
> On Tue, 11 Dec 2018 20:56:06 +0900, Alice Ferrazzi wrote:
>> Signed-off-by: Alice Ferrazzi <[email protected]>
>> ---
>> tools/testing/selftests/bpf/test_offload.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
>> index 0f9130ebfd2c..b06cc0eea0eb 100755
>> --- a/tools/testing/selftests/bpf/test_offload.py
>> +++ b/tools/testing/selftests/bpf/test_offload.py
>> @@ -140,7 +140,7 @@ def cmd_result(proc, include_stderr=False, fail=False):
>>
>>
>> def rm(f):
>> - cmd("rm -f %s" % (f))
>> + cmd("rm -f %s" % f)
>> if f in files:
>> files.remove(f)
>>
> Is this in PEP8, too?
I don't know, but it shouldn't be.
If f is a sequence type, both the old and new code can break here,
throwing a TypeError. It should be cmd("rm -f %s" % (f,)). The
presence of the brackets suggests to me that that's what the
original author intended.
Now, it's unlikely that we'd ever want to pass a list or tuple
here, since 'rm' wouldn't understand the result, but the proper
way to deal with that is an assertion with a meaningful message,
since the TypeError here will have the non-obvious message "not
all arguments converted during string formatting".
-Ed
On Wed, 12 Dec 2018 21:15:52 +0000, Edward Cree wrote:
> On 12/12/18 19:04, Jakub Kicinski wrote:
> > On Tue, 11 Dec 2018 20:56:06 +0900, Alice Ferrazzi wrote:
> >> Signed-off-by: Alice Ferrazzi <[email protected]>
> >> ---
> >> tools/testing/selftests/bpf/test_offload.py | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
> >> index 0f9130ebfd2c..b06cc0eea0eb 100755
> >> --- a/tools/testing/selftests/bpf/test_offload.py
> >> +++ b/tools/testing/selftests/bpf/test_offload.py
> >> @@ -140,7 +140,7 @@ def cmd_result(proc, include_stderr=False, fail=False):
> >>
> >>
> >> def rm(f):
> >> - cmd("rm -f %s" % (f))
> >> + cmd("rm -f %s" % f)
> >> if f in files:
> >> files.remove(f)
> >>
> > Is this in PEP8, too?
> I don't know, but it shouldn't be.
> If f is a sequence type, both the old and new code can break here,
> throwing a TypeError. It should be cmd("rm -f %s" % (f,)). The
> presence of the brackets suggests to me that that's what the
> original author intended.
Agreed, that was my intention, I didn't know about the comma option.
> Now, it's unlikely that we'd ever want to pass a list or tuple
> here, since 'rm' wouldn't understand the result, but the proper
> way to deal with that is an assertion with a meaningful message,
> since the TypeError here will have the non-obvious message "not
> all arguments converted during string formatting".
Interesting, thanks for the analysis!
FYI, we noticed the following commit (built with gcc-7):
commit: b870be2f1bb4fc7add1f061fc5915b8439f9b7e0 ("[PATCH 7/7] selftest/bpf: fix E501 line too long")
url: https://github.com/0day-ci/linux/commits/Alice-Ferrazzi/selftest-bpf-fix-PEP8-warnings/20181212-053030
base: https://git.kernel.org/cgit/linux/kernel/git/bpf/bpf-next.git master
in testcase: kernel_selftests
with following parameters:
group: kselftests-00
test-description: The kernel contains a set of "self tests" under the tools/testing/selftests/ directory. These are intended to be small unit tests to exercise individual code paths in the kernel.
test-url: https://www.kernel.org/doc/Documentation/kselftest.txt
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks,
Rong Chen