ACK: [J/K][L][SRU][PATCH 1/1] selftests: net: devlink_port_split.py: skip test if no suitable device available

[Roxana Nicolescu]

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 at canonical.com>
> Link: https://lore.kernel.org/r/20230315165353.229590-1-po-hsu.lin@canonical.com
> Signed-off-by: Jakub Kicinski <kuba at kernel.org>
> (cherry picked from commit 3de66d08d37a565eba1adfe1e107593cae978a20)
> Signed-off-by: Po-Hsu Lin <po-hsu.lin at 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 at 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