On Mon, 2019-06-03 at 11:51 +0200, Paolo Abeni wrote:

On Fri, 2019-05-31 at 18:30 +0000, Saeed Mahameed wrote:

quoted

On Fri, 2019-05-31 at 14:53 +0200, Paolo Abeni wrote:

quoted

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@redhat.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.

For macro _1 it make sense to have the likely keyword but for _2 it
doesn't, by looking at most of the usecases of INDIRECT_CALL_2, they
seem to be all around improving tcp/udp related indirection calls in
the protocol stack, and they seem to prefer tcp over udp. But IMHO at
least for the above usecase I think the likely keyword is being misused
here and should be remove from all INDIRECT_CALL_N where N > 1;

Eric, what do you think ?

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!
	\

custom macros can work, but in case you don't want to introduce such
macros in a vendor specific driver, then i think your patches are still
an improvement after all..

In any case, just make sure to use the order i suggested in next patch
with: MLX5_RX_INDIRECT_CALL_LIST

quoted

quoted

+				  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