| Message ID | 20230320035947.24949-2-po-hsu.lin@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | selftests: net: devlink_port_split.py: skip test if no suitable device available | expand |
On 20/03/2023 04:59, Po-Hsu Lin wrote: > BugLink: https://bugs.launchpad.net/bugs/1937133 > > The `devlink -j port show` command output may not contain the "flavour" > key, an example from Ubuntu 22.10 s390x LPAR(5.19.0-37-generic), with > mlx4 driver and iproute2-5.15.0: > {"port":{"pci/0001:00:00.0/1":{"type":"eth","netdev":"ens301"}, > "pci/0001:00:00.0/2":{"type":"eth","netdev":"ens301d1"}, > "pci/0002:00:00.0/1":{"type":"eth","netdev":"ens317"}, > "pci/0002:00:00.0/2":{"type":"eth","netdev":"ens317d1"}}} > > This will cause a KeyError exception. > > Create a validate_devlink_output() to check for this "flavour" from > devlink command output to avoid this KeyError exception. Also let > it handle the check for `devlink -j dev show` output in main(). > > Apart from this, if the test was not started because the max lanes of > the designated device is 0. The script will still return 0 and thus > causing a false-negative test result. > > Use a found_max_lanes flag to determine if these tests were skipped > due to this reason and return KSFT_SKIP to make it more clear. > > Link: https://bugs.launchpad.net/bugs/1937133 > Fixes: f3348a82e727 ("selftests: net: Add port split test") > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> > Link: https://lore.kernel.org/r/20230315165353.229590-1-po-hsu.lin@canonical.com > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > (cherry picked from commit 3de66d08d37a565eba1adfe1e107593cae978a20) > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> > --- > tools/testing/selftests/net/devlink_port_split.py | 36 +++++++++++++++++++---- > 1 file changed, 31 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/net/devlink_port_split.py b/tools/testing/selftests/net/devlink_port_split.py > index 2b5d6ff..2d84c7a 100755 > --- a/tools/testing/selftests/net/devlink_port_split.py > +++ b/tools/testing/selftests/net/devlink_port_split.py > @@ -59,6 +59,8 @@ class devlink_ports(object): > assert stderr == "" > ports = json.loads(stdout)['port'] > > + validate_devlink_output(ports, 'flavour') > + > for port in ports: > if dev in port: > if ports[port]['flavour'] == 'physical': > @@ -220,6 +222,27 @@ def split_splittable_port(port, k, lanes, dev): > unsplit(port.bus_info) > > > +def validate_devlink_output(devlink_data, target_property=None): > + """ > + Determine if test should be skipped by checking: > + 1. devlink_data contains values > + 2. The target_property exist in devlink_data > + """ > + skip_reason = None > + if any(devlink_data.values()): > + if target_property: > + skip_reason = "{} not found in devlink output, test skipped".format(target_property) > + for key in devlink_data: > + if target_property in devlink_data[key]: > + skip_reason = None > + else: > + skip_reason = 'devlink output is empty, test skipped' > + > + if skip_reason: > + print(skip_reason) > + sys.exit(KSFT_SKIP) > + > + > def make_parser(): > parser = argparse.ArgumentParser(description='A test for port splitting.') > parser.add_argument('--dev', > @@ -240,12 +263,9 @@ def main(cmdline=None): > stdout, stderr = run_command(cmd) > assert stderr == "" > > + validate_devlink_output(json.loads(stdout)) > devs = json.loads(stdout)['dev'] > - if devs: > - dev = list(devs.keys())[0] > - else: > - print("no devlink device was found, test skipped") > - sys.exit(KSFT_SKIP) > + dev = list(devs.keys())[0] > > cmd = "devlink dev show %s" % dev > stdout, stderr = run_command(cmd) > @@ -255,6 +275,7 @@ def main(cmdline=None): > > ports = devlink_ports(dev) > > + found_max_lanes = False > for port in ports.if_names: > max_lanes = get_max_lanes(port.name) > > @@ -277,6 +298,11 @@ def main(cmdline=None): > split_splittable_port(port, lane, max_lanes, dev) > > lane //= 2 > + found_max_lanes = True > + > + if not found_max_lanes: > + print(f"Test not started, no port of device {dev} reports max_lanes") > + sys.exit(KSFT_SKIP) > > > if __name__ == "__main__": Acked-by: Roxana Nicolescu <roxana.nicolescu@canonical.com> One remark I have, I assume this has to be applied to jammy, kinetic and lunar. I would have put [J,K,L] in the subject, I find it a bit confusing the way it is now. Roxana
On Mon, Mar 20, 2023 at 4:24 PM Roxana Nicolescu <roxana.nicolescu@canonical.com> wrote: > > > On 20/03/2023 04:59, Po-Hsu Lin wrote: > > BugLink: https://bugs.launchpad.net/bugs/1937133 > > > > The `devlink -j port show` command output may not contain the "flavour" > > key, an example from Ubuntu 22.10 s390x LPAR(5.19.0-37-generic), with > > mlx4 driver and iproute2-5.15.0: > > {"port":{"pci/0001:00:00.0/1":{"type":"eth","netdev":"ens301"}, > > "pci/0001:00:00.0/2":{"type":"eth","netdev":"ens301d1"}, > > "pci/0002:00:00.0/1":{"type":"eth","netdev":"ens317"}, > > "pci/0002:00:00.0/2":{"type":"eth","netdev":"ens317d1"}}} > > > > This will cause a KeyError exception. > > > > Create a validate_devlink_output() to check for this "flavour" from > > devlink command output to avoid this KeyError exception. Also let > > it handle the check for `devlink -j dev show` output in main(). > > > > Apart from this, if the test was not started because the max lanes of > > the designated device is 0. The script will still return 0 and thus > > causing a false-negative test result. > > > > Use a found_max_lanes flag to determine if these tests were skipped > > due to this reason and return KSFT_SKIP to make it more clear. > > > > Link: https://bugs.launchpad.net/bugs/1937133 > > Fixes: f3348a82e727 ("selftests: net: Add port split test") > > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> > > Link: https://lore.kernel.org/r/20230315165353.229590-1-po-hsu.lin@canonical.com > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > > (cherry picked from commit 3de66d08d37a565eba1adfe1e107593cae978a20) > > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> > > --- > > tools/testing/selftests/net/devlink_port_split.py | 36 +++++++++++++++++++---- > > 1 file changed, 31 insertions(+), 5 deletions(-) > > > > diff --git a/tools/testing/selftests/net/devlink_port_split.py b/tools/testing/selftests/net/devlink_port_split.py > > index 2b5d6ff..2d84c7a 100755 > > --- a/tools/testing/selftests/net/devlink_port_split.py > > +++ b/tools/testing/selftests/net/devlink_port_split.py > > @@ -59,6 +59,8 @@ class devlink_ports(object): > > assert stderr == "" > > ports = json.loads(stdout)['port'] > > > > + validate_devlink_output(ports, 'flavour') > > + > > for port in ports: > > if dev in port: > > if ports[port]['flavour'] == 'physical': > > @@ -220,6 +222,27 @@ def split_splittable_port(port, k, lanes, dev): > > unsplit(port.bus_info) > > > > > > +def validate_devlink_output(devlink_data, target_property=None): > > + """ > > + Determine if test should be skipped by checking: > > + 1. devlink_data contains values > > + 2. The target_property exist in devlink_data > > + """ > > + skip_reason = None > > + if any(devlink_data.values()): > > + if target_property: > > + skip_reason = "{} not found in devlink output, test skipped".format(target_property) > > + for key in devlink_data: > > + if target_property in devlink_data[key]: > > + skip_reason = None > > + else: > > + skip_reason = 'devlink output is empty, test skipped' > > + > > + if skip_reason: > > + print(skip_reason) > > + sys.exit(KSFT_SKIP) > > + > > + > > def make_parser(): > > parser = argparse.ArgumentParser(description='A test for port splitting.') > > parser.add_argument('--dev', > > @@ -240,12 +263,9 @@ def main(cmdline=None): > > stdout, stderr = run_command(cmd) > > assert stderr == "" > > > > + validate_devlink_output(json.loads(stdout)) > > devs = json.loads(stdout)['dev'] > > - if devs: > > - dev = list(devs.keys())[0] > > - else: > > - print("no devlink device was found, test skipped") > > - sys.exit(KSFT_SKIP) > > + dev = list(devs.keys())[0] > > > > cmd = "devlink dev show %s" % dev > > stdout, stderr = run_command(cmd) > > @@ -255,6 +275,7 @@ def main(cmdline=None): > > > > ports = devlink_ports(dev) > > > > + found_max_lanes = False > > for port in ports.if_names: > > max_lanes = get_max_lanes(port.name) > > > > @@ -277,6 +298,11 @@ def main(cmdline=None): > > split_splittable_port(port, lane, max_lanes, dev) > > > > lane //= 2 > > + found_max_lanes = True > > + > > + if not found_max_lanes: > > + print(f"Test not started, no port of device {dev} reports max_lanes") > > + sys.exit(KSFT_SKIP) > > > > > > if __name__ == "__main__": > Acked-by: Roxana Nicolescu <roxana.nicolescu@canonical.com> > > One remark I have, I assume this has to be applied to jammy, kinetic and > lunar. I would have put [J,K,L] in the subject, I find it a bit > confusing the way it is now. Yes that true, I pull this out as sometime the dev team will solely reply to Lunar, I thought this would be helpful. Looks like I should put them together. Thanks. > > Roxana > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/tools/testing/selftests/net/devlink_port_split.py b/tools/testing/selftests/net/devlink_port_split.py index 2b5d6ff..2d84c7a 100755 --- a/tools/testing/selftests/net/devlink_port_split.py +++ b/tools/testing/selftests/net/devlink_port_split.py @@ -59,6 +59,8 @@ class devlink_ports(object): assert stderr == "" ports = json.loads(stdout)['port'] + validate_devlink_output(ports, 'flavour') + for port in ports: if dev in port: if ports[port]['flavour'] == 'physical': @@ -220,6 +222,27 @@ def split_splittable_port(port, k, lanes, dev): unsplit(port.bus_info) +def validate_devlink_output(devlink_data, target_property=None): + """ + Determine if test should be skipped by checking: + 1. devlink_data contains values + 2. The target_property exist in devlink_data + """ + skip_reason = None + if any(devlink_data.values()): + if target_property: + skip_reason = "{} not found in devlink output, test skipped".format(target_property) + for key in devlink_data: + if target_property in devlink_data[key]: + skip_reason = None + else: + skip_reason = 'devlink output is empty, test skipped' + + if skip_reason: + print(skip_reason) + sys.exit(KSFT_SKIP) + + def make_parser(): parser = argparse.ArgumentParser(description='A test for port splitting.') parser.add_argument('--dev', @@ -240,12 +263,9 @@ def main(cmdline=None): stdout, stderr = run_command(cmd) assert stderr == "" + validate_devlink_output(json.loads(stdout)) devs = json.loads(stdout)['dev'] - if devs: - dev = list(devs.keys())[0] - else: - print("no devlink device was found, test skipped") - sys.exit(KSFT_SKIP) + dev = list(devs.keys())[0] cmd = "devlink dev show %s" % dev stdout, stderr = run_command(cmd) @@ -255,6 +275,7 @@ def main(cmdline=None): ports = devlink_ports(dev) + found_max_lanes = False for port in ports.if_names: max_lanes = get_max_lanes(port.name) @@ -277,6 +298,11 @@ def main(cmdline=None): split_splittable_port(port, lane, max_lanes, dev) lane //= 2 + found_max_lanes = True + + if not found_max_lanes: + print(f"Test not started, no port of device {dev} reports max_lanes") + sys.exit(KSFT_SKIP) if __name__ == "__main__":
BugLink: https://bugs.launchpad.net/bugs/1937133 The `devlink -j port show` command output may not contain the "flavour" key, an example from Ubuntu 22.10 s390x LPAR(5.19.0-37-generic), with mlx4 driver and iproute2-5.15.0: {"port":{"pci/0001:00:00.0/1":{"type":"eth","netdev":"ens301"}, "pci/0001:00:00.0/2":{"type":"eth","netdev":"ens301d1"}, "pci/0002:00:00.0/1":{"type":"eth","netdev":"ens317"}, "pci/0002:00:00.0/2":{"type":"eth","netdev":"ens317d1"}}} This will cause a KeyError exception. Create a validate_devlink_output() to check for this "flavour" from devlink command output to avoid this KeyError exception. Also let it handle the check for `devlink -j dev show` output in main(). Apart from this, if the test was not started because the max lanes of the designated device is 0. The script will still return 0 and thus causing a false-negative test result. Use a found_max_lanes flag to determine if these tests were skipped due to this reason and return KSFT_SKIP to make it more clear. Link: https://bugs.launchpad.net/bugs/1937133 Fixes: f3348a82e727 ("selftests: net: Add port split test") Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> Link: https://lore.kernel.org/r/20230315165353.229590-1-po-hsu.lin@canonical.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit 3de66d08d37a565eba1adfe1e107593cae978a20) Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> --- tools/testing/selftests/net/devlink_port_split.py | 36 +++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-)