Re: patch make struct protosw pr_input non-variadic

[Tyler Retzlaff <rtr%netbsd.org@localhost>]

So I lost track.

Have we addressed all the concerns that have been raised?

* We're removing type-punning.
* We aren't adding additional levels of indirection in the hot path.
* We aren't badly casting structs.

Do we need more discussion or shall we proceed?

Thanks for the feedback.


On 5/15/2016 11:10 AM, Taylor R Campbell wrote:
>    Date: Sun, 15 May 2016 10:42:24 -0400
>    From: Thor Lancelot Simon <tls%panix.com@localhost>
>
>    On Fri, May 13, 2016 at 11:14:13PM +0000, Christos Zoulas wrote:
>    > >
>    > >This patch:
>    > >* moves pr_input out of struct protosw and places it into <proto>sw
>    > >  specitic structure.
>    > >* converts domain::dom_protosw to an array of pointers
>    >
>    > Why? What's the point of the extra indirection? You could just stick
>    > and & in front of the array element assignment.
>
>    I want to reemphasize the potentially negative performance impact of
>    this kind of change.  It turns the branch into a computed branch in a
>    way that will defeat branch prediction on many CPUs and thus cause
>    stalls.  Don't call through pointers if you don't have to.
>
> It may well have a negative performance impact to change
>
> 	(*inetsw[ip_proto[nh]].pr_input)(m, off, nh)
>
> in ip_input to
>
> 	(*inetsw[ip_proto[nh]]->pr_input)(m, off, nh)
>
> That's why this patch does *not* do that.  The actual change to
> ip_input is only
>
> 	(*inetsw[ip_proto[nh]].ippr_input)(m, off, nh)
>
> where we can now give struct ipprotosw::ippr_input a true prototype to
> distinguish it from, e.g., the IPv6 code which passes &m and &off
> instead of m and off.
>
> The only place the extra indirection is used is in paths that are cold
> anyway, where the caller is iterating over all protocols in a domain,
> mostly in uipc_domain.c, during kernel initialization, socket
> creation, routing table updates, &c.