Skip to content

Commit

Permalink
Merge branch 'netdev-support-dumping-a-single-netdev-in-qstats'
Browse files Browse the repository at this point in the history 
Jakub Kicinski says:

====================
netdev: support dumping a single netdev in qstats

I was writing a test for page pool which depended on qstats,
and got tired of having to filter dumps in user space.
Add support for dumping stats for a single netdev.

To get there we first need to add full support for extack
in dumps (and fix a dump error handling bug in YNL, sent
separately to the net tree).
====================

Link: https://lore.kernel.org/r/20240420023543.3300306-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Apr 23, 2024
2 parents 1af2dfa + 2371092 commit b2c8599
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 81 deletions.
1 change: 1 addition & 0 deletions Documentation/netlink/specs/netdev.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ operations:
dump:
request:
attributes:
- ifindex
- scope
reply:
attributes:
Expand Down
1 change: 1 addition & 0 deletions net/core/netdev-genl-gen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ static const struct nla_policy netdev_napi_get_dump_nl_policy[NETDEV_A_NAPI_IFIN

/* NETDEV_CMD_QSTATS_GET - dump */
static const struct nla_policy netdev_qstats_get_nl_policy[NETDEV_A_QSTATS_SCOPE + 1] = {
[NETDEV_A_QSTATS_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
[NETDEV_A_QSTATS_SCOPE] = NLA_POLICY_MASK(NLA_UINT, 0x1),
};

Expand Down
52 changes: 39 additions & 13 deletions net/core/netdev-genl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -639,35 +639,61 @@ netdev_nl_stats_by_netdev(struct net_device *netdev, struct sk_buff *rsp,
return -EMSGSIZE;
}

static int
netdev_nl_qstats_get_dump_one(struct net_device *netdev, unsigned int scope,
struct sk_buff *skb, const struct genl_info *info,
struct netdev_nl_dump_ctx *ctx)
{
if (!netdev->stat_ops)
return 0;

switch (scope) {
case 0:
return netdev_nl_stats_by_netdev(netdev, skb, info);
case NETDEV_QSTATS_SCOPE_QUEUE:
return netdev_nl_stats_by_queue(netdev, skb, info, ctx);
}

return -EINVAL; /* Should not happen, per netlink policy */
}

int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
struct netlink_callback *cb)
{
struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
const struct genl_info *info = genl_info_dump(cb);
struct net *net = sock_net(skb->sk);
struct net_device *netdev;
unsigned int ifindex;
unsigned int scope;
int err = 0;

scope = 0;
if (info->attrs[NETDEV_A_QSTATS_SCOPE])
scope = nla_get_uint(info->attrs[NETDEV_A_QSTATS_SCOPE]);

rtnl_lock();
for_each_netdev_dump(net, netdev, ctx->ifindex) {
if (!netdev->stat_ops)
continue;
ifindex = 0;
if (info->attrs[NETDEV_A_QSTATS_IFINDEX])
ifindex = nla_get_u32(info->attrs[NETDEV_A_QSTATS_IFINDEX]);

switch (scope) {
case 0:
err = netdev_nl_stats_by_netdev(netdev, skb, info);
break;
case NETDEV_QSTATS_SCOPE_QUEUE:
err = netdev_nl_stats_by_queue(netdev, skb, info, ctx);
break;
rtnl_lock();
if (ifindex) {
netdev = __dev_get_by_index(net, ifindex);
if (netdev && netdev->stat_ops) {
err = netdev_nl_qstats_get_dump_one(netdev, scope, skb,
info, ctx);
} else {
NL_SET_BAD_ATTR(info->extack,
info->attrs[NETDEV_A_QSTATS_IFINDEX]);
err = netdev ? -EOPNOTSUPP : -ENODEV;
}
} else {
for_each_netdev_dump(net, netdev, ctx->ifindex) {
err = netdev_nl_qstats_get_dump_one(netdev, scope, skb,
info, ctx);
if (err < 0)
break;
}
if (err < 0)
break;
}
rtnl_unlock();

Expand Down
135 changes: 70 additions & 65 deletions net/netlink/af_netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,69 @@ __nlmsg_put(struct sk_buff *skb, u32 portid, u32 seq, int type, int len, int fla
}
EXPORT_SYMBOL(__nlmsg_put);

static size_t
netlink_ack_tlv_len(struct netlink_sock *nlk, int err,
const struct netlink_ext_ack *extack)
{
size_t tlvlen;

if (!extack || !test_bit(NETLINK_F_EXT_ACK, &nlk->flags))
return 0;

tlvlen = 0;
if (extack->_msg)
tlvlen += nla_total_size(strlen(extack->_msg) + 1);
if (extack->cookie_len)
tlvlen += nla_total_size(extack->cookie_len);

/* Following attributes are only reported as error (not warning) */
if (!err)
return tlvlen;

if (extack->bad_attr)
tlvlen += nla_total_size(sizeof(u32));
if (extack->policy)
tlvlen += netlink_policy_dump_attr_size_estimate(extack->policy);
if (extack->miss_type)
tlvlen += nla_total_size(sizeof(u32));
if (extack->miss_nest)
tlvlen += nla_total_size(sizeof(u32));

return tlvlen;
}

static void
netlink_ack_tlv_fill(struct sk_buff *in_skb, struct sk_buff *skb,
const struct nlmsghdr *nlh, int err,
const struct netlink_ext_ack *extack)
{
if (extack->_msg)
WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG, extack->_msg));
if (extack->cookie_len)
WARN_ON(nla_put(skb, NLMSGERR_ATTR_COOKIE,
extack->cookie_len, extack->cookie));

if (!err)
return;

if (extack->bad_attr &&
!WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
(u8 *)extack->bad_attr >= in_skb->data + in_skb->len))
WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
(u8 *)extack->bad_attr - (const u8 *)nlh));
if (extack->policy)
netlink_policy_dump_write_attr(skb, extack->policy,
NLMSGERR_ATTR_POLICY);
if (extack->miss_type)
WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_MISS_TYPE,
extack->miss_type));
if (extack->miss_nest &&
!WARN_ON((u8 *)extack->miss_nest < in_skb->data ||
(u8 *)extack->miss_nest > in_skb->data + in_skb->len))
WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_MISS_NEST,
(u8 *)extack->miss_nest - (const u8 *)nlh));
}

/*
* It looks a bit ugly.
* It would be better to create kernel thread.
Expand All @@ -2175,6 +2238,7 @@ static int netlink_dump_done(struct netlink_sock *nlk, struct sk_buff *skb,
struct netlink_ext_ack *extack)
{
struct nlmsghdr *nlh;
size_t extack_len;

nlh = nlmsg_put_answer(skb, cb, NLMSG_DONE, sizeof(nlk->dump_done_errno),
NLM_F_MULTI | cb->answer_flags);
Expand All @@ -2184,10 +2248,14 @@ static int netlink_dump_done(struct netlink_sock *nlk, struct sk_buff *skb,
nl_dump_check_consistent(cb, nlh);
memcpy(nlmsg_data(nlh), &nlk->dump_done_errno, sizeof(nlk->dump_done_errno));

if (extack->_msg && test_bit(NETLINK_F_EXT_ACK, &nlk->flags)) {
extack_len = netlink_ack_tlv_len(nlk, nlk->dump_done_errno, extack);
if (extack_len) {
nlh->nlmsg_flags |= NLM_F_ACK_TLVS;
if (!nla_put_string(skb, NLMSGERR_ATTR_MSG, extack->_msg))
if (skb_tailroom(skb) >= extack_len) {
netlink_ack_tlv_fill(cb->skb, skb, cb->nlh,
nlk->dump_done_errno, extack);
nlmsg_end(skb, nlh);
}
}

return 0;
Expand Down Expand Up @@ -2406,69 +2474,6 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
}
EXPORT_SYMBOL(__netlink_dump_start);

static size_t
netlink_ack_tlv_len(struct netlink_sock *nlk, int err,
const struct netlink_ext_ack *extack)
{
size_t tlvlen;

if (!extack || !test_bit(NETLINK_F_EXT_ACK, &nlk->flags))
return 0;

tlvlen = 0;
if (extack->_msg)
tlvlen += nla_total_size(strlen(extack->_msg) + 1);
if (extack->cookie_len)
tlvlen += nla_total_size(extack->cookie_len);

/* Following attributes are only reported as error (not warning) */
if (!err)
return tlvlen;

if (extack->bad_attr)
tlvlen += nla_total_size(sizeof(u32));
if (extack->policy)
tlvlen += netlink_policy_dump_attr_size_estimate(extack->policy);
if (extack->miss_type)
tlvlen += nla_total_size(sizeof(u32));
if (extack->miss_nest)
tlvlen += nla_total_size(sizeof(u32));

return tlvlen;
}

static void
netlink_ack_tlv_fill(struct sk_buff *in_skb, struct sk_buff *skb,
struct nlmsghdr *nlh, int err,
const struct netlink_ext_ack *extack)
{
if (extack->_msg)
WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG, extack->_msg));
if (extack->cookie_len)
WARN_ON(nla_put(skb, NLMSGERR_ATTR_COOKIE,
extack->cookie_len, extack->cookie));

if (!err)
return;

if (extack->bad_attr &&
!WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
(u8 *)extack->bad_attr >= in_skb->data + in_skb->len))
WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
(u8 *)extack->bad_attr - (u8 *)nlh));
if (extack->policy)
netlink_policy_dump_write_attr(skb, extack->policy,
NLMSGERR_ATTR_POLICY);
if (extack->miss_type)
WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_MISS_TYPE,
extack->miss_type));
if (extack->miss_nest &&
!WARN_ON((u8 *)extack->miss_nest < in_skb->data ||
(u8 *)extack->miss_nest > in_skb->data + in_skb->len))
WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_MISS_NEST,
(u8 *)extack->miss_nest - (u8 *)nlh));
}

void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
const struct netlink_ext_ack *extack)
{
Expand Down
62 changes: 59 additions & 3 deletions tools/testing/selftests/drivers/net/stats.py
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0

from lib.py import ksft_run, ksft_exit
from lib.py import ksft_in, ksft_true, KsftSkipEx, KsftXfailEx
from lib.py import ksft_run, ksft_exit, ksft_pr
from lib.py import ksft_ge, ksft_eq, ksft_in, ksft_true, ksft_raises, KsftSkipEx, KsftXfailEx
from lib.py import EthtoolFamily, NetdevFamily, RtnlFamily, NlError
from lib.py import NetDrvEnv

Expand Down Expand Up @@ -77,9 +77,65 @@ def stat_cmp(rstat, qstat):
raise Exception("Qstats are lower, fetched later")


def qstat_by_ifindex(cfg) -> None:
global netfam
global rtnl

# Construct a map ifindex -> [dump, by-index, dump]
ifindexes = {}
stats = netfam.qstats_get({}, dump=True)
for entry in stats:
ifindexes[entry['ifindex']] = [entry, None, None]

for ifindex in ifindexes.keys():
entry = netfam.qstats_get({"ifindex": ifindex}, dump=True)
ksft_eq(len(entry), 1)
ifindexes[entry[0]['ifindex']][1] = entry[0]

stats = netfam.qstats_get({}, dump=True)
for entry in stats:
ifindexes[entry['ifindex']][2] = entry

if len(ifindexes) == 0:
raise KsftSkipEx("No ifindex supports qstats")

# Now make sure the stats match/make sense
for ifindex, triple in ifindexes.items():
all_keys = triple[0].keys() | triple[1].keys() | triple[2].keys()

for key in all_keys:
ksft_ge(triple[1][key], triple[0][key], comment="bad key: " + key)
ksft_ge(triple[2][key], triple[1][key], comment="bad key: " + key)

# Test invalid dumps
# 0 is invalid
with ksft_raises(NlError) as cm:
netfam.qstats_get({"ifindex": 0}, dump=True)
ksft_eq(cm.exception.nl_msg.error, -34)
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)
ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.ifindex')

# Try to get stats for lowest unused ifindex but not 0
devs = rtnl.getlink({}, dump=True)
all_ifindexes = set([dev["ifi-index"] for dev in devs])
lowest = 2
while lowest in all_ifindexes:
lowest += 1

with ksft_raises(NlError) as cm:
netfam.qstats_get({"ifindex": lowest}, dump=True)
ksft_eq(cm.exception.nl_msg.error, -19)
ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.ifindex')


def main() -> None:
with NetDrvEnv(__file__) as cfg:
ksft_run([check_pause, check_fec, pkt_byte_sum],
ksft_run([check_pause, check_fec, pkt_byte_sum, qstat_by_ifindex],
args=(cfg, ))
ksft_exit()

Expand Down
18 changes: 18 additions & 0 deletions tools/testing/selftests/net/lib/py/ksft.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,24 @@ def ksft_ge(a, b, comment=""):
_fail("Check failed", a, "<", b, comment)


class ksft_raises:
def __init__(self, expected_type):
self.exception = None
self.expected_type = expected_type

def __enter__(self):
return self

def __exit__(self, exc_type, exc_val, exc_tb):
if exc_type is None:
_fail(f"Expected exception {str(self.expected_type.__name__)}, none raised")
elif self.expected_type != exc_type:
_fail(f"Expected exception {str(self.expected_type.__name__)}, raised {str(exc_type.__name__)}")
self.exception = exc_val
# Suppress the exception if its the expected one
return self.expected_type == exc_type


def ksft_busy_wait(cond, sleep=0.005, deadline=1, comment=""):
end = time.monotonic() + deadline
while True:
Expand Down

0 comments on commit b2c8599

Please sign in to comment.