Re: [PATCH net-next v2 10/10] ethtool: netlink: always pass genl_info to .prepare_data
From: Jiri Pirko <jiri@resnulli.us>
Date: 2023-08-11 06:42:49
Fri, Aug 11, 2023 at 01:38:45AM CEST, kuba@kernel.org wrote: [...]
quoted hunk ↗ jump to hunk
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index f7b3171a0aad..3bbd5afb7b31 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c@@ -444,12 +444,12 @@ static int ethnl_default_doit(struct sk_buff *skb, struct genl_info *info)static int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev, const struct ethnl_dump_ctx *ctx, - struct netlink_callback *cb) + const struct genl_info *info) { void *ehdr; int ret; - ehdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, + ehdr = genlmsg_put(skb, info->snd_portid, info->snd_seq, ðtool_genl_family, NLM_F_MULTI, ctx->ops->reply_cmd); if (!ehdr)@@ -457,7 +457,7 @@ static int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev,ethnl_init_reply_data(ctx->reply_data, ctx->ops, dev); rtnl_lock(); - ret = ctx->ops->prepare_data(ctx->req_info, ctx->reply_data, NULL); + ret = ctx->ops->prepare_data(ctx->req_info, ctx->reply_data, info); rtnl_unlock(); if (ret < 0) goto out;@@ -495,7 +495,7 @@ static int ethnl_default_dumpit(struct sk_buff *skb,dev_hold(dev); rtnl_unlock(); - ret = ethnl_default_dump_one(skb, dev, ctx, cb); + ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb)); rtnl_lock(); dev_put(dev);@@ -647,11 +647,14 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,struct ethnl_reply_data *reply_data; const struct ethnl_request_ops *ops; struct ethnl_req_info *req_info; + struct genl_info info; struct sk_buff *skb; void *reply_payload; int reply_len; int ret; + genl_info_init_ntf(&info, ðtool_genl_family, cmd); + if (WARN_ONCE(cmd > ETHTOOL_MSG_KERNEL_MAX || !ethnl_default_notify_ops[cmd], "unexpected notification type %u\n", cmd))@@ -670,7 +673,7 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,req_info->flags |= ETHTOOL_FLAG_COMPACT_BITSETS; ethnl_init_reply_data(reply_data, ops, dev); - ret = ops->prepare_data(req_info, reply_data, NULL); + ret = ops->prepare_data(req_info, reply_data, &info); if (ret < 0) goto err_cleanup; ret = ops->reply_size(req_info, reply_data);
[...]
quoted hunk ↗ jump to hunk
@@ -24,7 +24,7 @@ const struct nla_policy ethnl_wol_get_policy[] = {static int wol_prepare_data(const struct ethnl_req_info *req_base, struct ethnl_reply_data *reply_base, - struct genl_info *info) + const struct genl_info *info) { struct wol_reply_data *data = WOL_REPDATA(reply_base); struct net_device *dev = reply_base->dev;@@ -39,7 +39,8 @@ static int wol_prepare_data(const struct ethnl_req_info *req_base,dev->ethtool_ops->get_wol(dev, &data->wol); ethnl_ops_complete(dev); /* do not include password in notifications */ - data->show_sopass = info && (data->wol.supported & WAKE_MAGICSECURE); + data->show_sopass = genl_info_is_ntf(info) && + (data->wol.supported & WAKE_MAGICSECURE);
I believe that you are missing "!" here: data->show_sopass = !genl_info_is_ntf(info) && (data->wol.supported & WAKE_MAGICSECURE); But, you are changing the output for dumpit if I'm not mistaken. ethnl_default_dump_one() currently calls this with info==NULL too, not only ethnl_default_notify(). Anyway, the genl_info_is_ntf() itself seems a bit odd to me. The only user is here and I doubt there ever going to be any other. This conditional per-op attr fill seems a bit odd. Can't you handle this in side ethtool somehow? IDK :/
return 0; } -- 2.41.0