Re: patch make struct protosw pr_input non-variadic

[Tyler Retzlaff <tech-net%netbsd.org@localhost>]

On Mon, May 16, 2016 at 08:25:23AM +0700, Robert Elz wrote:
>   | Do we need more discussion or shall we proceed?
> 
> Back in the good old days - I mean 4BSD, and certainly 4.1 .. 4.3 BSD
> (I wasn't involved as much with 4.4, so can't really say for that one
> way or the other) no-one would have even dared suggest a change anything
> like this without backing it up with performance measurements, gprof
> data showing that (at least) it was not worse then before, and before and
> after throughput measurements.

I can't actually understand how we got on to the topic of performance.

> 
> We seem to have lost that here, and comments are made (both side) based upon
> speculation about what seems likely might hurt, improve, or not affect
> performamce, rather than by any actual testing.

I've yet to see anyone highlight a section of the patch that actually
represents a change to how protocol input has changed in a way that would
harm performance.

        SOFTNET_LOCK();
-       (*inetsw[ip_protox[nh]].pr_input)(m, off, nh);
+       (*inetsw[ip_protox[nh]].ippr_input)(m, off, nh);
        SOFTNET_UNLOCK();

This patch is a re-structure it just moved a pointer to the input function
out of one structure and into another.  The way that pointer is used has
not been changed.

> It is different for an obvious bug fix (code that did not work before has
> 0 performance after all, anything is better) but when the change is intended
> in some way to improve things, it ought be shown that it does.   The
> "cleanliness" improvement seems obvious here, but if there were to be a
> performance downside (and I am not saying that there is, I have not measured
> it either) then there would need to be a decision on whether or not the code
> improvement is worth the cost (of course, what sometimes happens, is that
> cleaner code is also faster, and everyone is happy.)

I'm encouraged that you observe it as a cleanliness improvement. I would
hope that the patch (that will follow this) will also be considered as
such.

The benefit is not intended to be a performance improvement the
perceived benefits were highlighted by Taylor here.

http://mail-index.netbsd.org/tech-net/2016/05/14/msg005869.html

It may be of interest that a review of the callers of pr_input do not
always pass 2 variadic arguments as most input functions seem to unpack.
A situation that would not be possible if this (and subsequent change)
is made. Probably the callers "know" which implementation of input
function they will get but not brilliant in terms of an abstraction.