| Message ID | 20241213152244.3080955-6-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5712e323d4c3ad03bba4d28f83e80593171ac3f1 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netdev: fix repeated netlink messages in queue dumps | expand |
Jakub Kicinski <kuba@kernel.org> writes: > Sanity check netlink dumps, to make sure dumps don't have > repeated entries or gaps in IDs. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: shuah@kernel.org > CC: linux-kselftest@vger.kernel.org > --- > tools/testing/selftests/drivers/net/stats.py | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/drivers/net/stats.py b/tools/testing/selftests/drivers/net/stats.py > index 63e3c045a3b2..031ac9def6c0 100755 > --- a/tools/testing/selftests/drivers/net/stats.py > +++ b/tools/testing/selftests/drivers/net/stats.py > @@ -110,6 +110,23 @@ rtnl = RtnlFamily() > ksft_ge(triple[1][key], triple[0][key], comment="bad key: " + key) > ksft_ge(triple[2][key], triple[1][key], comment="bad key: " + key) > > + # Sanity check the dumps > + queues = NetdevFamily(recv_size=4096).qstats_get({"scope": "queue"}, dump=True) > + # Reformat the output into {ifindex: {rx: [id, id, ...], tx: [id, id, ...]}} > + parsed = {} > + for entry in queues: > + ifindex = entry["ifindex"] > + if ifindex not in parsed: > + parsed[ifindex] = {"rx":[], "tx": []} > + parsed[ifindex][entry["queue-type"]].append(entry['queue-id']) BTW setdefault() exists for exactly these add-unless-already-exists scenarios: parsed_entry = parsed.setdefault(ifindex, {"rx":[], "tx": []}) parsed_entry[entry["queue-type"]].append(entry['queue-id']) Sometimes this can be used to inline the whole expression, such as mydict.setdefault(key, []).append(value), but that would be unwieldy here. Anyway, consider rewriting, but it's a nit, it's readable just fine as is. Reviewed-by: Petr Machata <petrm@nvidia.com> > + # Now, validate > + for ifindex, queues in parsed.items(): > + for qtype in ['rx', 'tx']: > + ksft_eq(len(queues[qtype]), len(set(queues[qtype])), > + comment="repeated queue keys") > + ksft_eq(len(queues[qtype]), max(queues[qtype]) + 1, > + comment="missing queue keys") > + > # Test invalid dumps > # 0 is invalid > with ksft_raises(NlError) as cm: > @@ -158,7 +175,7 @@ rtnl = RtnlFamily() > > > def main() -> None: > - with NetDrvEnv(__file__) as cfg: > + with NetDrvEnv(__file__, queue_count=100) as cfg: > ksft_run([check_pause, check_fec, pkt_byte_sum, qstat_by_ifindex, > check_down], > args=(cfg, ))
On Mon, 16 Dec 2024 11:53:58 +0100 Petr Machata wrote: > > + if ifindex not in parsed: > > + parsed[ifindex] = {"rx":[], "tx": []} > > + parsed[ifindex][entry["queue-type"]].append(entry['queue-id']) > > BTW setdefault() exists for exactly these add-unless-already-exists > scenarios: > > parsed_entry = parsed.setdefault(ifindex, {"rx":[], "tx": []}) > parsed_entry[entry["queue-type"]].append(entry['queue-id']) > > Sometimes this can be used to inline the whole expression, such as > mydict.setdefault(key, []).append(value), but that would be unwieldy here. Ack, I used setdefault() initially but it made the line too incomprehensible. Too many things get indexed at once..
diff --git a/tools/testing/selftests/drivers/net/stats.py b/tools/testing/selftests/drivers/net/stats.py index 63e3c045a3b2..031ac9def6c0 100755 --- a/tools/testing/selftests/drivers/net/stats.py +++ b/tools/testing/selftests/drivers/net/stats.py @@ -110,6 +110,23 @@ rtnl = RtnlFamily() ksft_ge(triple[1][key], triple[0][key], comment="bad key: " + key) ksft_ge(triple[2][key], triple[1][key], comment="bad key: " + key) + # Sanity check the dumps + queues = NetdevFamily(recv_size=4096).qstats_get({"scope": "queue"}, dump=True) + # Reformat the output into {ifindex: {rx: [id, id, ...], tx: [id, id, ...]}} + parsed = {} + for entry in queues: + ifindex = entry["ifindex"] + if ifindex not in parsed: + parsed[ifindex] = {"rx":[], "tx": []} + parsed[ifindex][entry["queue-type"]].append(entry['queue-id']) + # Now, validate + for ifindex, queues in parsed.items(): + for qtype in ['rx', 'tx']: + ksft_eq(len(queues[qtype]), len(set(queues[qtype])), + comment="repeated queue keys") + ksft_eq(len(queues[qtype]), max(queues[qtype]) + 1, + comment="missing queue keys") + # Test invalid dumps # 0 is invalid with ksft_raises(NlError) as cm: @@ -158,7 +175,7 @@ rtnl = RtnlFamily() def main() -> None: - with NetDrvEnv(__file__) as cfg: + with NetDrvEnv(__file__, queue_count=100) as cfg: ksft_run([check_pause, check_fec, pkt_byte_sum, qstat_by_ifindex, check_down], args=(cfg, ))
Sanity check netlink dumps, to make sure dumps don't have repeated entries or gaps in IDs. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: shuah@kernel.org CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/drivers/net/stats.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)