netdev - Re: [PATCH net-next 2/3] indirect call wrappers: add helpers for 3 and 4 ways switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 03 Jun 2019 11:51:07 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Saeed Mahameed <saeedm@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "leon@...nel.org" <leon@...nel.org>
Subject: Re: [PATCH net-next 2/3] indirect call wrappers: add helpers for 3
 and 4 ways switch

On Fri, 2019-05-31 at 18:30 +0000, Saeed Mahameed wrote:
> On Fri, 2019-05-31 at 14:53 +0200, Paolo Abeni wrote:
> > Experimental results[1] has shown that resorting to several branches
> > and a direct-call is faster than indirect call via retpoline, even
> > when the number of added branches go up 5.
> > 
> > This change adds two additional helpers, to cope with indirect calls
> > with up to 4 available direct call option. We will use them
> > in the next patch.
> > 
> > [1] 
> > https://linuxplumbersconf.org/event/2/contributions/99/attachments/98/117/lpc18_paper_af_xdp_perf-v2.pdf
> > 
> > Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> > ---
> >  include/linux/indirect_call_wrapper.h | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/include/linux/indirect_call_wrapper.h
> > b/include/linux/indirect_call_wrapper.h
> > index 00d7e8e919c6..7c4cac87eaf7 100644
> > --- a/include/linux/indirect_call_wrapper.h
> > +++ b/include/linux/indirect_call_wrapper.h
> > @@ -23,6 +23,16 @@
> >  		likely(f == f2) ? f2(__VA_ARGS__) :			
> > \
> >  				  INDIRECT_CALL_1(f, f1, __VA_ARGS__);	
> > \
> >  	})
> > +#define INDIRECT_CALL_3(f, f3, f2, f1, ...)				
> > \
> > +	({								
> > \
> > +		likely(f == f3) ? f3(__VA_ARGS__) :			
> > \
> > +				  INDIRECT_CALL_2(f, f2, f1,
> > __VA_ARGS__); \
> > +	})
> > +#define INDIRECT_CALL_4(f, f4, f3, f2, f1, ...)			
> > 	\
> > +	({								
> > \
> > +		likely(f == f4) ? f4(__VA_ARGS__) :		
> 
> do we really want "likely" here ? in our cases there is no preference
> on whuch fN is going to have the top priority, all of them are equally
> important and statically configured and guranteed to not change on data
> path .. 

I was a little undecided about that, too. 'likely()' is there mainly
for simmetry with the already existing _1 and _2 variants. In such
macros the branch prediction hint represent a real priority of the
available choices.

To avoid the branch prediction, a new set of macros should be defined,
but that also sounds redundant.

If you have strong opinion against the breanch prediction hint, I could
either drop this patch and the next one or resort to custom macros in
the mlx code.

Any [alternative] suggestions more than welcome!
	\
> > +				  INDIRECT_CALL_3(f, f3, f2, f1,
> > __VA_ARGS__); \
> > +	})
> >  
> 
> Oh the RETPOLINE!
> 
> On which (N) where INDIRECT_CALL_N(f, fN, fN-1, ..., f1,...) , calling
> the indirection function pointer directly is going to be actually
> better than this whole INDIRECT_CALL_N wrapper "if else" dance ?

In commit ce02ef06fcf7a399a6276adb83f37373d10cbbe1, it's measured a
relevant gain even with more than 5 options. I personally would avoid
adding much more options than the above.

Thanks,

Paolo