On Mon, 2019-06-03 at 22:27 +0000, Saeed Mahameed wrote:

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

quoted

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;

I experimented a bit with gcc 8.3.1 and some BP hint variations:

* with current macros we have single test for fN and an incresing
number of conditional jumps and tests for the following functions, as
the generated code looks somehow alike:

	cmp f4, function_ptr
	jne test_f3
	call f4
post_call:
	// ...

	// ...
test_f3:
	cmp f3, function_ptr
	jne test_f2
	call f3
	jmp post_call

test_f2:
	cmp f2, function_ptr
	//...

* keeping 'likely' only on INDIRECT_CALL_1 we get a conditinal jump for
fN and the number of conditional jumps and tests grows for the next
functions, as the generated code looks somehow alike:

	cmp f4, function_ptr
	je call_f4
	cmp f3, function_ptr
	je call_f3
	//...
	cmp f1, function_ptr
	jne indirect_call
	call f1
post_call:
	// ...

	// ...
call_f4:
	call f4
	jmp post_call
call_f3:
	call f3
	jmp post_call
	// ...


* without any BP hints, is quite alike the above, except for the last
test, the indirect call don't need an additional jump: 

	cmp f4, function_ptr
	je call_f4
	cmp f3, function_ptr
	je call_f3
	//...
	cmp f1, function_ptr
	je call_f1
	call retpoline_helper

I think the first option should be overall better then the 2nd. The 3rd
one is the worse.

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

Sure! will do in next iteration, as soon as the above topic is settled.

Thanks!

Paolo