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

[Po-Hsu Lin]

On Mon, Mar 20, 2023 at 4:24 PM Roxana Nicolescu
<roxana.nicolescu at 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 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.

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 at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team