Received-SPF: pass (google.com: domain of linux-kernel+bounces-157833-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 24 Apr 2024 16:19:49 -0700
From: Joe Damato <jdamato@fastly.com>
To: linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	davem@davemloft.net, kuba@kernel.org, shuah@kernel.org
Subject: ynl: maybe minor bug?
Message-ID: <ZimTlf_ISC2n8snQ@LQ3V64L9R2>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi there:

I am probably just doing something wrong, but I tried to run
tools/testing/selftests/drivers/net/stats.py today and hit what is possibly
a bug?

Background info: Python 3.12.3

I'm using net-next at commit 9dd15d5088e9 ("Merge branch
'sparx5-port-mirroring'") with a couple driver modifications added on top
of it that don't seem relevant to the two test failures I'm hitting:

1. "loopback has no stats", and
2. "Try to get stats for lowest unused ifindex but not 0"

Both of these tests expect the ynl library to raise an exception, but I
don't think it does, from tools/net/ynl/lib/ynl.py, the _ops method:

  if nl_msg.error:
      raise NlError(nl_msg)
  if nl_msg.done:
      if nl_msg.extack:
          print("Netlink warning:")
          print(nl_msg)

And the code in net/core/netdev-genl.c seems to set:

 else {
        NL_SET_BAD_ATTR(info->extack,
                        info->attrs[NETDEV_A_QSTATS_IFINDEX]);
        err = netdev ? -EOPNOTSUPP : -ENODEV;

which is what cli.py says:

$ ./cli.py --spec ../../../Documentation/netlink/specs/netdev.yaml \
           --dump qstats-get --json '{"ifindex": "1"}'
Netlink warning:
nl_len = 28 (12) nl_flags = 0x202 nl_type = 3
	extack: {'bad-attr': '.ifindex'}
[]

that seems to be the warning print out from the above
tools/net/ynl/lib/ynl.py snippet, not an NlError, which is what you'd get
if you tried ifindex 0 (which is listed as out of range in the YAML spec):

$ ./cli.py --spec ../../../Documentation/netlink/specs/netdev.yaml \
           --dump qstats-get --json '{"ifindex": "0"}'

Netlink error: Numerical result out of range
nl_len = 108 (92) nl_flags = 0x300 nl_type = 2
	error: -34
        extack: {'msg': 'integer out of range', 'policy': {'min-value': 1,
        'max-value': 4294967295, 'type': 'u32'}, 'bad-attr': '.ifindex'}

I'm not sure whether:

1. tools/net/ynl/lib/ynl.py should be raising NlError when there is an
   extack in this case (I think this is probably the way to go?), or

2. the tests should be changed so that they don't expect an exception to be
   raised but (ideally?) hide the warning report from tools/net/ynl/lib/ynl.py
   when the warning is expected.

I don't know python at all so this is definitely wrong, but here's a small
change I made to fix the test (a similar change was made for the test which
follows).

The following patch is not intended to be seriously considered for
application, just to highlight the issue I am hitting:

diff --git a/tools/testing/selftests/drivers/net/stats.py b/tools/testing/selftests/drivers/net/stats.py
index 7a7b16b180e2..d9f5d1f3ed34 100755
--- a/tools/testing/selftests/drivers/net/stats.py
+++ b/tools/testing/selftests/drivers/net/stats.py
@@ -115,9 +115,8 @@ def qstat_by_ifindex(cfg) -> None:
     ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.ifindex')

     # loopback has no stats
-    with ksft_raises(NlError) as cm:
-        netfam.qstats_get({"ifindex": 1}, dump=True)
-    ksft_eq(cm.exception.nl_msg.error, -95)
+    stats = netfam.qstats_get({"ifindex": 1}, dump=True)
+    ksft_eq(cm.exception.nl_msg.error, -34)
     ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.ifindex')


Thanks,
Joe