Re: [bpf-next V4 PATCH 2/8] bpf: devmap prepare xdp frames for bulking

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Wed, 23 May 2018 11:54:38 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> > +	for (i = 0; i < bq->count; i++) {
> > +		struct xdp_frame *xdpf = bq->q[i];
> > +		int err;
> > +
> > +		err = dev->netdev_ops->ndo_xdp_xmit(dev, xdpf);
> > +		if (err) {
> > +			drops++;
> > +			xdp_return_frame(xdpf);
> > +		}
> > +		processed++;  
> 
> This sort of thing makes it really hard to review. 'processed' and
> 'drops' are not read anywhere in this function. So I need to go and
> check all the other patches whether there's further logic involved here
> or not. I had to review your series after applying all patches in a
> local branch, please never do this. Add the logic in a patch where it's
> self-contained and obvious to review.

Sorry, 'processed' and 'drops' were used by the tracepoint that Alexei
asked me to split into another (next patch).  And I can see that I have
renamed 'processed' to 'sent' in the next tracepoint patch, which makes
reviewing even harder sorry.  Those lines should have been moved to the
tracepoint patch. My mistake when splitting up the patches.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer