Subject: Re: [PATCHv2 net] netpoll: fix rx_hook() interface by passing the skb

From: Antonio Quartulli <[email-masked]>
Date: Thu, 24 Oct 2013 14:01:52 +0200

On Thu, Oct 24, 2013 at 09:43:38AM +0100, David Laight wrote:
> > Subject: [PATCHv2 net] netpoll: fix rx_hook() interface by passing the skb
> > @@ -820,7 +823,10 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
> > 
> >  		len -= iph->ihl*4;
> >  		uh = (struct udphdr *)(((char *)iph) + iph->ihl*4);
> > +		offset = (unsigned char *)(uh + 1) - skb->data;
> >  		ulen = ntohs(uh->len);
> > +		data_len = skb->len - offset;
> > +		source = ntohs(uh->source);
> > 
> >  		if (ulen != len)
> >  			goto out;
> > @@ -834,9 +840,7 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
> >  			if (np->local_port && np->local_port != ntohs(uh->dest))
> >  				continue;
> > 
> > -			np->rx_hook(np, ntohs(uh->source),
> > -				       (char *)(uh+1),
> > -				       ulen - sizeof(struct udphdr));
> > +			np->rx_skb_hook(np, source, skb, offset, data_len);
> >  			hits++;
> >  		}
> >  	} else {
> 
> From a code optimisation point of view you probably don't want to be
> calculating the source, offset and length early.
> It is quite likely that the local variables will have to be written
> to the stack (because of the function calls) - so it is almost
> certainly more efficient to calculate them just before the call.

I thought that computing them once outside the loop was better than
re-computing them during each iteration.

Having them outside makes it also clear that they always have the same value.

Regards,

-- 
Antonio Quartulli