Subject: Re: [PATCH v7 02/11] net: add ndo to setup/query xdp prog in adapter rx

From: Brenden Blanco <[email-masked]>
Date: Mon, 11 Jul 2016 23:35:00 -0700

On Mon, Jul 11, 2016 at 11:12:24PM -0700, David Miller wrote:
> From: Brenden Blanco <[email-masked]>
> Date: Mon, 11 Jul 2016 14:29:49 -0700
> 
> > +	if (fd >= 0) {
> > +		prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_XDP);
> > +		if (IS_ERR(prog))
> > +			return PTR_ERR(prog);
> > +	}
> > +
> > +	xdp.command = XDP_SETUP_PROG;
> > +	xdp.prog = prog;
> > +	err = ops->ndo_xdp(dev, &xdp);
> > +	if (err < 0 && prog)
> > +		bpf_prog_put(prog);
> 
> I don't understand the reference counting on 'prog' here.
> 
> The enumeration documentation states that no matter what, the passed
> in prog doesn't need to be mangaged by the ->ndo_xdp() method.
> 
> Therefore, if that is true, we must always put the 'prog' here if it
> is non-NULL.  Regardless of the 'err' value.
> 
The documentation is unclear then. What I _meant_ to say is that the callee
is not responsible for putting the program on error, but on success it
takes ownership of the reference. In context of that, does the code make
sense? Is there a more conventional way of handling this?

Thanks for your review.