| Message ID | 20250324104012.367366-2-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethtool: Introduce ethnl dump helpers | expand |
On Mon, 24 Mar 2025 11:40:03 +0100 Maxime Chevallier <maxime.chevallier@bootlin.com> wrote: > There are a few netlink commands that rely on the req_info->dev field > being populated by ethnl in their ->prepare_data() and ->fill_reply(). > > For a regular GET request, this will be set by ethnl_default_parse(), > which calls ethnl_parse_header_dev_get(). > > In the case of a DUMP request, the ->prepare_data() and ->fill_reply() > callbacks will be called with the req_info->dev being NULL, which can > cause discrepancies in the behaviour between GET and DUMP results. > > The main impact is that ethnl_req_get_phydev() will not find any > phy_device, impacting : > - plca > - pse-pd > - stats > > Some other commands rely on req_info->dev, namely : > - coalesce in ->fill_reply to look for an irq_moder > > Although cable_test and tunnels also rely on req_info->dev being set, > that's not a problem for these commands as : > - cable_test doesn't support DUMP > - tunnels rolls its own ->dumpit (and sets dev in the req_info). > - phy also has its own ->dumpit > > All other commands use reply_data->dev (probably the correct way of > doing things) and aren't facing this issue. > > Simply set the dev in the req_info context when iterating to dump each > dev. Tested-by: Kory Maincent <kory.maincent@bootlin.com> Reviewed-by: Kory Maincent <kory.maincent@bootlin.com> Thank you, this fixes the PSE dump! Regards,
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index a163d40c6431..6b1725795435 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -591,6 +591,7 @@ static int ethnl_default_dumpit(struct sk_buff *skb, dev_hold(dev); rcu_read_unlock(); + ctx->req_info->dev = dev; ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb)); rcu_read_lock();
There are a few netlink commands that rely on the req_info->dev field being populated by ethnl in their ->prepare_data() and ->fill_reply(). For a regular GET request, this will be set by ethnl_default_parse(), which calls ethnl_parse_header_dev_get(). In the case of a DUMP request, the ->prepare_data() and ->fill_reply() callbacks will be called with the req_info->dev being NULL, which can cause discrepancies in the behaviour between GET and DUMP results. The main impact is that ethnl_req_get_phydev() will not find any phy_device, impacting : - plca - pse-pd - stats Some other commands rely on req_info->dev, namely : - coalesce in ->fill_reply to look for an irq_moder Although cable_test and tunnels also rely on req_info->dev being set, that's not a problem for these commands as : - cable_test doesn't support DUMP - tunnels rolls its own ->dumpit (and sets dev in the req_info). - phy also has its own ->dumpit All other commands use reply_data->dev (probably the correct way of doing things) and aren't facing this issue. Simply set the dev in the req_info context when iterating to dump each dev. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- V4 : New patch (was sent separaltely once) net/ethtool/netlink.c | 1 + 1 file changed, 1 insertion(+)