[PATCH v14 02/20] netlink: add new family to manage ULP_DDP enablement and stats
Jakub Kicinski
kuba at kernel.org
Fri Sep 8 14:49:09 PDT 2023
On Wed, 6 Sep 2023 11:30:00 +0000 Aurelien Aptel wrote:
> Add a new netlink family to get/set ULP DDP capabilities on a network
> device and to retrieve statistics.
>
> The messages use the genetlink infrastructure and are specified in a
> YAML file which was used to generate some of the files in this commit:
>
> ./tools/net/ynl/ynl-gen-c.py --mode kernel \
> --spec ./Documentation/netlink/specs/ulp_ddp.yaml --header \
> -o net/core/ulp_ddp_gen_nl.h
> ./tools/net/ynl/ynl-gen-c.py --mode kernel \
> --spec ./Documentation/netlink/specs/ulp_ddp.yaml --source \
> -o net/core/ulp_ddp_gen_nl.c
> ./tools/net/ynl/ynl-gen-c.py --mode uapi \
> --spec ./Documentation/netlink/specs/ulp_ddp.yaml --header \
> > include/uapi/linux/ulp_ddp_nl.h
Looks mostly good, but we lost dump support. TAL at netdev_nl_dev_get_dumpit(),
the iteration is not that hard, I reckon we should support dumps.
Few extra nits below.
> +static struct ulp_ddp_netdev_caps *netdev_ulp_ddp_caps(struct net_device *dev)
> +{
> +#ifdef CONFIG_ULP_DDP
> + return &dev->ulp_ddp_caps;
> +#else
> + return NULL;
> +#endif
Are the ifdefs still needed? Can't we compile out all of the family code
if the config is not selected?
> +/* pre_doit */
> +int ulp_ddp_get_netdev(const struct genl_split_ops *ops,
> + struct sk_buff *skb, struct genl_info *info)
> +{
> + struct reply_data *data;
> +
> + if (GENL_REQ_ATTR_CHECK(info, ULP_DDP_A_DEV_IFINDEX))
> + return -EINVAL;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->ifindex = nla_get_u32(info->attrs[ULP_DDP_A_DEV_IFINDEX]);
> + data->dev = netdev_get_by_index(genl_info_net(info),
> + data->ifindex,
> + &data->tracker,
> + GFP_KERNEL);
> +
nit pointless empty line.
> + if (!data->dev) {
> + kfree(data);
> + return -EINVAL;
ENOENT ? Maybe also
NL_SET_BAD_ATTR(info->extack, info->attrs[ULP_DDP_A_DEV_IFINDEX])
?
> +int ulp_ddp_nl_set_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> + struct reply_data *data = info->user_ptr[0];
> + unsigned long wanted, wanted_mask;
> + struct sk_buff *rsp;
> + bool notify;
> + int ret;
> +
> + if (GENL_REQ_ATTR_CHECK(info, ULP_DDP_A_DEV_WANTED) ||
> + GENL_REQ_ATTR_CHECK(info, ULP_DDP_A_DEV_WANTED_MASK))
> + return -EINVAL;
> +
> + rsp = genlmsg_new(reply_size(ULP_DDP_CMD_STATS), GFP_KERNEL);
> + if (!rsp)
> + return -EMSGSIZE;
> +
> + wanted = nla_get_u64(info->attrs[ULP_DDP_A_DEV_WANTED]);
> + wanted_mask = nla_get_u64(info->attrs[ULP_DDP_A_DEV_WANTED_MASK]);
> +
> + ret = apply_bits(data, &wanted, &wanted_mask, info->extack);
> + if (ret < 0)
> + return ret;
leaks rsp
> + notify = !!ret;
> + ret = prepare_data(info, data, ULP_DDP_CMD_SET);
> + if (ret)
> + return ret;
> +
> + ret = fill_data(rsp, data, ULP_DDP_CMD_SET, info->snd_portid, info->snd_seq, 0);
fill_date() can probably use genlmsg_iput() ?
--
pw-bot: cr
More information about the Linux-nvme
mailing list