Re: [PATCH net-next v3 05/24] ovpn: implement interface creation/destruction via netlink

Jakub Kicinski
2024-05-08T00:21:23Z

On Mon, 6 May 2024 03:16:18 +0200 Antonio Quartulli wrote:
> int ovpn_nl_new_iface_doit(struct sk_buff *skb, struct genl_info *info)
> {
> - return -ENOTSUPP;
> + const char *ifname = OVPN_DEFAULT_IFNAME;
> + enum ovpn_mode mode = OVPN_MODE_P2P;
> + struct net_device *dev;
> + struct sk_buff *msg;
> + void *hdr;
> +
> + if (info->attrs[OVPN_A_IFNAME])
> + ifname = nla_data(info->attrs[OVPN_A_IFNAME]);
> +
> + if (info->attrs[OVPN_A_MODE]) {
> + mode = nla_get_u32(info->attrs[OVPN_A_MODE]);
> + pr_debug("ovpn: setting device (%s) mode: %u\n", ifname, mode);
> + }
> +
> + dev = ovpn_iface_create(ifname, mode, genl_info_net(info));
> + if (IS_ERR(dev)) {
> + pr_err("ovpn: error while creating interface %s: %ld\n", ifname,
> + PTR_ERR(dev));

Better to send the error to the caller with NL_SET_ERR_MSG_MOD()

> + return PTR_ERR(dev);
> + }
> +
> + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq, &ovpn_nl_family,
> + 0, OVPN_CMD_NEW_IFACE);

genlmsg_iput() will save you a lot of typing

> + if (!hdr) {
> + netdev_err(dev, "%s: cannot create message header\n", __func__);
> + return -EMSGSIZE;
> + }
> +
> + if (nla_put(msg, OVPN_A_IFNAME, strlen(dev->name) + 1, dev->name)) {

nla_put_string() ?

> + netdev_err(dev, "%s: cannot add ifname to reply\n", __func__);

Probably not worth it, can't happen given the message size

> + genlmsg_cancel(msg, hdr);
> + nlmsg_free(msg);
> + return -EMSGSIZE;
> + }
> +
> + genlmsg_end(msg, hdr);
> +
> + return genlmsg_reply(msg, info);
> }
>
> int ovpn_nl_del_iface_doit(struct sk_buff *skb, struct genl_info *info)
> {
> - return -ENOTSUPP;
> + struct ovpn_struct *ovpn = info->user_ptr[0];
> +
> + rtnl_lock();
> + ovpn_iface_destruct(ovpn);
> + dev_put(ovpn->dev);
> + rtnl_unlock();
> +
> + synchronize_net();

Why? 🤔️