Subject: Re: [RFC PATCH net-next v6 05/15] netdev: support binding dma-buf to netdevice

From: Jakub Kicinski <[email-masked]>
Date: Thu, 7 Mar 2024 19:58:28 -0800

On Mon,  4 Mar 2024 18:01:40 -0800 Mina Almasry wrote:
> +	if (!dev || !dev->netdev_ops)
> +		return -EINVAL;

too defensive

> +	if (!dev->netdev_ops->ndo_queue_stop ||
> +	    !dev->netdev_ops->ndo_queue_mem_free ||
> +	    !dev->netdev_ops->ndo_queue_mem_alloc ||
> +	    !dev->netdev_ops->ndo_queue_start)
> +		return -EOPNOTSUPP;
> +
> +	new_mem = dev->netdev_ops->ndo_queue_mem_alloc(dev, rxq_idx);
> +	if (!new_mem)
> +		return -ENOMEM;
> +
> +	err = dev->netdev_ops->ndo_queue_stop(dev, rxq_idx, &old_mem);
> +	if (err)
> +		goto err_free_new_mem;
> +
> +	err = dev->netdev_ops->ndo_queue_start(dev, rxq_idx, new_mem);
> +	if (err)
> +		goto err_start_queue;
> +
> +	dev->netdev_ops->ndo_queue_mem_free(dev, old_mem);

nice :)

> +	rxq = __netif_get_rx_queue(dev, rxq_idx);
> +
> +	if (rxq->binding)

nit: a few places have an empty line between call and error check

> +		return -EEXIST;

> +	if (!capable(CAP_NET_ADMIN))
> +		return -EPERM;

this can be a flag on the netlink policy, no?

	flags: [ admin-perm ]

on the op

> +	dmabuf = dma_buf_get(dmabuf_fd);
> +	if (IS_ERR_OR_NULL(dmabuf))
> +		return -EBADFD;

> +	hdr = genlmsg_put(rsp, info->snd_portid, info->snd_seq,

genlmsg_iput()

> +static int netdev_netlink_notify(struct notifier_block *nb, unsigned long state,
> +				 void *_notify)
> +{
> +	struct netlink_notify *notify = _notify;
> +	struct netdev_dmabuf_binding *rbinding;
> +
> +	if (state != NETLINK_URELEASE || notify->protocol != NETLINK_GENERIC)
> +		return NOTIFY_DONE;
> +
> +	rtnl_lock();
> +
> +	list_for_each_entry(rbinding, &netdev_rbinding_list, list) {
> +		if (rbinding->owner_nlportid == notify->portid) {
> +			netdev_unbind_dmabuf(rbinding);
> +			break;
> +		}
> +	}
> +
> +	rtnl_unlock();
> +
> +	return NOTIFY_OK;
> +}

While you were not looking we added three new members to the netlink
family:

 * @sock_priv_size: the size of per-socket private memory
 * @sock_priv_init: the per-socket private memory initializer
 * @sock_priv_destroy: the per-socket private memory destructor

You should be able to associate state with a netlink socket
and have it auto-destroyed if the socket closes.
LMK if that doesn't work for you, I was hoping it would fit nicely.

I just realized now that the code gen doesn't know how to spit
those members out, but I'll send a patch tomorrow, you can hack
it manually until that gets in.