All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] net/mlx5: use indirect call wrappers
@ 2019-06-06 21:56 Paolo Abeni
  2019-06-06 21:56 ` [PATCH net-next v2 1/3] net/mlx5e: use indirect calls wrapper for skb allocation Paolo Abeni
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Paolo Abeni @ 2019-06-06 21:56 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Saeed Mahameed, Leon Romanovsky

The mlx5_core driver uses several indirect calls in fast-path, some of them
are invoked on each ingress packet, even for the XDP-only traffic.

This series leverage the indirect call wrappers infrastructure the avoid
the expansive RETPOLINE overhead for 2 indirect calls in fast-path.

Each call is addressed on a different patch, plus we need to introduce a couple
of additional helpers to cope with the higher number of possible direct-call
alternatives.

v1 - v2:
 - update the direct call list and use a macro to define it,
   as per Saeed suggestion. An intermediated additional
   macro is needed to allow arg list expansion
 - patch 2/3 is unchanged, as the generated code looks better this way than
   with possible alternative (dropping BP hits)

Paolo Abeni (3):
  net/mlx5e: use indirect calls wrapper for skb allocation
  indirect call wrappers: add helpers for 3 and 4 ways switch
  net/mlx5e: use indirect calls wrapper for the rx packet handler

 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  4 +++
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 27 ++++++++++++++-----
 include/linux/indirect_call_wrapper.h         | 12 +++++++++
 3 files changed, 37 insertions(+), 6 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 1/3] net/mlx5e: use indirect calls wrapper for skb allocation
  2019-06-06 21:56 [PATCH net-next v2 0/3] net/mlx5: use indirect call wrappers Paolo Abeni
@ 2019-06-06 21:56 ` Paolo Abeni
  2019-06-06 21:56 ` [PATCH net-next v2 2/3] indirect call wrappers: add helpers for 3 and 4 ways switch Paolo Abeni
  2019-06-06 21:56 ` [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler Paolo Abeni
  2 siblings, 0 replies; 7+ messages in thread
From: Paolo Abeni @ 2019-06-06 21:56 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Saeed Mahameed, Leon Romanovsky

We can avoid an indirect call per packet wrapping the skb creation
with the appropriate helper.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 13133e7f088e..0fe5f13d07cc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -34,6 +34,7 @@
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <linux/tcp.h>
+#include <linux/indirect_call_wrapper.h>
 #include <net/ip6_checksum.h>
 #include <net/page_pool.h>
 #include <net/inet_ecn.h>
@@ -1092,7 +1093,10 @@ void mlx5e_handle_rx_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
 	wi       = get_frag(rq, ci);
 	cqe_bcnt = be32_to_cpu(cqe->byte_cnt);
 
-	skb = rq->wqe.skb_from_cqe(rq, cqe, wi, cqe_bcnt);
+	skb = INDIRECT_CALL_2(rq->wqe.skb_from_cqe,
+			      mlx5e_skb_from_cqe_linear,
+			      mlx5e_skb_from_cqe_nonlinear,
+			      rq, cqe, wi, cqe_bcnt);
 	if (!skb) {
 		/* probably for XDP */
 		if (__test_and_clear_bit(MLX5E_RQ_FLAG_XDP_XMIT, rq->flags)) {
@@ -1279,8 +1283,10 @@ void mlx5e_handle_rx_cqe_mpwrq(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
 
 	cqe_bcnt = mpwrq_get_cqe_byte_cnt(cqe);
 
-	skb = rq->mpwqe.skb_from_cqe_mpwrq(rq, wi, cqe_bcnt, head_offset,
-					   page_idx);
+	skb = INDIRECT_CALL_2(rq->mpwqe.skb_from_cqe_mpwrq,
+			      mlx5e_skb_from_cqe_mpwrq_linear,
+			      mlx5e_skb_from_cqe_mpwrq_nonlinear,
+			      rq, wi, cqe_bcnt, head_offset, page_idx);
 	if (!skb)
 		goto mpwrq_cqe_out;
 
@@ -1437,7 +1443,10 @@ void mlx5i_handle_rx_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
 	wi       = get_frag(rq, ci);
 	cqe_bcnt = be32_to_cpu(cqe->byte_cnt);
 
-	skb = rq->wqe.skb_from_cqe(rq, cqe, wi, cqe_bcnt);
+	skb = INDIRECT_CALL_2(rq->wqe.skb_from_cqe,
+			      mlx5e_skb_from_cqe_linear,
+			      mlx5e_skb_from_cqe_nonlinear,
+			      rq, cqe, wi, cqe_bcnt);
 	if (!skb)
 		goto wq_free_wqe;
 
@@ -1469,7 +1478,10 @@ void mlx5e_ipsec_handle_rx_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
 	wi       = get_frag(rq, ci);
 	cqe_bcnt = be32_to_cpu(cqe->byte_cnt);
 
-	skb = rq->wqe.skb_from_cqe(rq, cqe, wi, cqe_bcnt);
+	skb = INDIRECT_CALL_2(rq->wqe.skb_from_cqe,
+			      mlx5e_skb_from_cqe_linear,
+			      mlx5e_skb_from_cqe_nonlinear,
+			      rq, cqe, wi, cqe_bcnt);
 	if (unlikely(!skb)) {
 		/* a DROP, save the page-reuse checks */
 		mlx5e_free_rx_wqe(rq, wi, true);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 2/3] indirect call wrappers: add helpers for 3 and 4 ways switch
  2019-06-06 21:56 [PATCH net-next v2 0/3] net/mlx5: use indirect call wrappers Paolo Abeni
  2019-06-06 21:56 ` [PATCH net-next v2 1/3] net/mlx5e: use indirect calls wrapper for skb allocation Paolo Abeni
@ 2019-06-06 21:56 ` Paolo Abeni
  2019-06-06 21:56 ` [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler Paolo Abeni
  2 siblings, 0 replies; 7+ messages in thread
From: Paolo Abeni @ 2019-06-06 21:56 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Saeed Mahameed, Leon Romanovsky

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__) :			\
+				  INDIRECT_CALL_3(f, f3, f2, f1, __VA_ARGS__); \
+	})
 
 #define INDIRECT_CALLABLE_DECLARE(f)	f
 #define INDIRECT_CALLABLE_SCOPE
@@ -30,6 +40,8 @@
 #else
 #define INDIRECT_CALL_1(f, f1, ...) f(__VA_ARGS__)
 #define INDIRECT_CALL_2(f, f2, f1, ...) f(__VA_ARGS__)
+#define INDIRECT_CALL_3(f, f3, f2, f1, ...) f(__VA_ARGS__)
+#define INDIRECT_CALL_4(f, f4, f3, f2, f1, ...) f(__VA_ARGS__)
 #define INDIRECT_CALLABLE_DECLARE(f)
 #define INDIRECT_CALLABLE_SCOPE		static
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler
  2019-06-06 21:56 [PATCH net-next v2 0/3] net/mlx5: use indirect call wrappers Paolo Abeni
  2019-06-06 21:56 ` [PATCH net-next v2 1/3] net/mlx5e: use indirect calls wrapper for skb allocation Paolo Abeni
  2019-06-06 21:56 ` [PATCH net-next v2 2/3] indirect call wrappers: add helpers for 3 and 4 ways switch Paolo Abeni
@ 2019-06-06 21:56 ` Paolo Abeni
  2019-06-07 18:09   ` Saeed Mahameed
  2 siblings, 1 reply; 7+ messages in thread
From: Paolo Abeni @ 2019-06-06 21:56 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Saeed Mahameed, Leon Romanovsky

We can avoid another indirect call per packet wrapping the rx
handler call with the proper helper.

To ensure that even the last listed direct call experience
measurable gain, despite the additional conditionals we must
traverse before reaching it, I tested reversing the order of the
listed options, with performance differences below noise level.

Together with the previous indirect call patch, this gives
~6% performance improvement in raw UDP tput.

v1 -> v2:
 - update the direct call list and use a macro to define it,
   as per Saeed suggestion. An intermediated additional
   macro is needed to allow arg list expansion

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h    | 4 ++++
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 3a183d690e23..52bcdc87cbe2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -148,6 +148,10 @@ struct page_pool;
 
 #define MLX5E_MSG_LEVEL			NETIF_MSG_LINK
 
+#define MLX5_RX_INDIRECT_CALL_LIST \
+	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe, mlx5i_handle_rx_cqe, \
+	mlx5e_ipsec_handle_rx_cqe
+
 #define mlx5e_dbg(mlevel, priv, format, ...)                    \
 do {                                                            \
 	if (NETIF_MSG_##mlevel & (priv)->msglevel)              \
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 0fe5f13d07cc..7faf643eb1b9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -1303,6 +1303,8 @@ void mlx5e_handle_rx_cqe_mpwrq(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
 	mlx5_wq_ll_pop(wq, cqe->wqe_id, &wqe->next.next_wqe_index);
 }
 
+#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f, list, __VA_ARGS__)
+
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
 {
 	struct mlx5e_rq *rq = container_of(cq, struct mlx5e_rq, cq);
@@ -1333,7 +1335,8 @@ int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
 
 		mlx5_cqwq_pop(cqwq);
 
-		rq->handle_rx_cqe(rq, cqe);
+		INDIRECT_CALL_LIST(rq->handle_rx_cqe,
+				   MLX5_RX_INDIRECT_CALL_LIST, rq, cqe);
 	} while ((++work_done < budget) && (cqe = mlx5_cqwq_get_cqe(cqwq)));
 
 out:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler
  2019-06-06 21:56 ` [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler Paolo Abeni
@ 2019-06-07 18:09   ` Saeed Mahameed
  2019-06-10  8:43     ` Paolo Abeni
  0 siblings, 1 reply; 7+ messages in thread
From: Saeed Mahameed @ 2019-06-07 18:09 UTC (permalink / raw)
  To: netdev, pabeni; +Cc: davem, leon

On Thu, 2019-06-06 at 23:56 +0200, Paolo Abeni wrote:
> We can avoid another indirect call per packet wrapping the rx
> handler call with the proper helper.
> 
> To ensure that even the last listed direct call experience
> measurable gain, despite the additional conditionals we must
> traverse before reaching it, I tested reversing the order of the
> listed options, with performance differences below noise level.
> 
> Together with the previous indirect call patch, this gives
> ~6% performance improvement in raw UDP tput.
> 
> v1 -> v2:
>  - update the direct call list and use a macro to define it,
>    as per Saeed suggestion. An intermediated additional
>    macro is needed to allow arg list expansion
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h    | 4 ++++
>  drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 5 ++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 3a183d690e23..52bcdc87cbe2 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -148,6 +148,10 @@ struct page_pool;
>  
>  #define MLX5E_MSG_LEVEL			NETIF_MSG_LINK
>  
> +#define MLX5_RX_INDIRECT_CALL_LIST \
> +	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe,
> mlx5i_handle_rx_cqe, \
> +	mlx5e_ipsec_handle_rx_cqe
> +
>  #define mlx5e_dbg(mlevel, priv, format, ...)                    \
>  do {                                                            \
>  	if (NETIF_MSG_##mlevel & (priv)->msglevel)              \
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 0fe5f13d07cc..7faf643eb1b9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -1303,6 +1303,8 @@ void mlx5e_handle_rx_cqe_mpwrq(struct mlx5e_rq
> *rq, struct mlx5_cqe64 *cqe)
>  	mlx5_wq_ll_pop(wq, cqe->wqe_id, &wqe->next.next_wqe_index);
>  }
>  
> +#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f, list,
> __VA_ARGS__)
> +

Hi Paolo, 

This patch produces some compiler errors:

Please note that mlx5e_ipsec_handle_rx_cqe is only defined when
CONFIG_MLX5_EN_IPSEC is enabled.

02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c: In function
'mlx5e_poll_rx_cq':
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1300:42:
error: implicit declaration of function 'INDIRECT_CALL_4'; did you mean
'INDIRECT_CALL_LIST'? [-Werror=implicit-function-declaration]
02:26:53  #define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f,
list, __VA_ARGS__)
02:26:53                                           ^
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1332:3: note:
in expansion of macro 'INDIRECT_CALL_LIST'
02:26:53    INDIRECT_CALL_LIST(rq->handle_rx_cqe,
02:26:53    ^~~~~~~~~~~~~~~~~~
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en.h:153:2: error:
'mlx5e_ipsec_handle_rx_cqe' undeclared (first use in this function);
did you mean 'mlx5e_fp_handle_rx_cqe'?
02:26:53   mlx5e_ipsec_handle_rx_cqe
02:26:53   ^
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1300:61: note:
in definition of macro 'INDIRECT_CALL_LIST'
02:26:53  #define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f,
list, __VA_ARGS__)
02:26:53                                                              ^
~~~
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1333:8: note:
in expansion of macro 'MLX5_RX_INDIRECT_CALL_LIST'
02:26:53         MLX5_RX_INDIRECT_CALL_LIST, rq, cqe);
02:26:53         ^~~~~~~~~~~~~~~~~~~~~~~~~~
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en.h:153:2: note: each
undeclared identifier is reported only once for each function it
appears in
02:26:53   mlx5e_ipsec_handle_rx_cqe
02:26:53   ^
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1300:61: note:
in definition of macro 'INDIRECT_CALL_LIST'
02:26:53  #define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f,
list, __VA_ARGS__)
02:26:53                                                              ^
~~~
02:26:53 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1333:8: note:
in expansion of macro 'MLX5_RX_INDIRECT_CALL_LIST'
02:26:53         MLX5_RX_INDIRECT_CALL_LIST, rq, cqe);
02:26:53         ^~~~~~~~~~~~~~~~~~~~~~~~~~


>  int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
>  {
>  	struct mlx5e_rq *rq = container_of(cq, struct mlx5e_rq, cq);
> @@ -1333,7 +1335,8 @@ int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int
> budget)
>  
>  		mlx5_cqwq_pop(cqwq);
>  
> -		rq->handle_rx_cqe(rq, cqe);
> +		INDIRECT_CALL_LIST(rq->handle_rx_cqe,
> +				   MLX5_RX_INDIRECT_CALL_LIST, rq,
> cqe);
>  	} while ((++work_done < budget) && (cqe =
> mlx5_cqwq_get_cqe(cqwq)));
>  
>  out:

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler
  2019-06-07 18:09   ` Saeed Mahameed
@ 2019-06-10  8:43     ` Paolo Abeni
  2019-06-10 22:20       ` Saeed Mahameed
  0 siblings, 1 reply; 7+ messages in thread
From: Paolo Abeni @ 2019-06-10  8:43 UTC (permalink / raw)
  To: Saeed Mahameed, netdev; +Cc: davem, leon

Hi,

On Fri, 2019-06-07 at 18:09 +0000, Saeed Mahameed wrote:
> On Thu, 2019-06-06 at 23:56 +0200, Paolo Abeni wrote:
> > We can avoid another indirect call per packet wrapping the rx
> > handler call with the proper helper.
> > 
> > To ensure that even the last listed direct call experience
> > measurable gain, despite the additional conditionals we must
> > traverse before reaching it, I tested reversing the order of the
> > listed options, with performance differences below noise level.
> > 
> > Together with the previous indirect call patch, this gives
> > ~6% performance improvement in raw UDP tput.
> > 
> > v1 -> v2:
> >  - update the direct call list and use a macro to define it,
> >    as per Saeed suggestion. An intermediated additional
> >    macro is needed to allow arg list expansion
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx5/core/en.h    | 4 ++++
> >  drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 5 ++++-
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > index 3a183d690e23..52bcdc87cbe2 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > @@ -148,6 +148,10 @@ struct page_pool;
> >  
> >  #define MLX5E_MSG_LEVEL			NETIF_MSG_LINK
> >  
> > +#define MLX5_RX_INDIRECT_CALL_LIST \
> > +	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe,
> > mlx5i_handle_rx_cqe, \
> > +	mlx5e_ipsec_handle_rx_cqe
> > +
> >  #define mlx5e_dbg(mlevel, priv, format, ...)                    \
> >  do {                                                            \
> >  	if (NETIF_MSG_##mlevel & (priv)->msglevel)              \
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > index 0fe5f13d07cc..7faf643eb1b9 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > @@ -1303,6 +1303,8 @@ void mlx5e_handle_rx_cqe_mpwrq(struct mlx5e_rq
> > *rq, struct mlx5_cqe64 *cqe)
> >  	mlx5_wq_ll_pop(wq, cqe->wqe_id, &wqe->next.next_wqe_index);
> >  }
> >  
> > +#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f, list,
> > __VA_ARGS__)
> > +
> 
> Hi Paolo, 
> 
> This patch produces some compiler errors:
> 
> Please note that mlx5e_ipsec_handle_rx_cqe is only defined when
> CONFIG_MLX5_EN_IPSEC is enabled.

I'm sorry, I dumbly did not fuzz vs mlx5 build options.

It looks like that, to cope with all the possible mixes, a not-so-nice
macro maze is required; something alike the following:

#if defined(CONFIG_MLX5_EN_IPSEC) && defined (CONFIG_MLX5_CORE_IPOIB)

#define MLX5_RX_INDIRECT_CALL_LIST \
	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe, mlx5i_handle_rx_cqe, \
	mlx5e_ipsec_handle_rx_cqe
#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f, list, __VA_ARGS__)

#elif defined(CONFIG_MLX5_EN_IPSEC)

#define MLX5_RX_INDIRECT_CALL_LIST \
	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe, \
	mlx5e_ipsec_handle_rx_cqe
#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_3(f, list, __VA_ARGS__)

#elif defined(CONFIG_MLX5_CORE_IPOIB)

#define MLX5_RX_INDIRECT_CALL_LIST \
	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe, mlx5i_handle_rx_cqe
#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_3(f, list, __VA_ARGS__)

#else

#define MLX5_RX_INDIRECT_CALL_LIST \
	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe
#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_2(f, list, __VA_ARGS__)

#endif

If you are ok with the above, I can include it in v3, otherwise I can
either:

* drop patch 2/3 and use only the 2 alternatives
(mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe) that are available
regardless of the driver build options

* drop both patches 2/3 and 3/3

Any feedback welcome, thanks!

Paolo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler
  2019-06-10  8:43     ` Paolo Abeni
@ 2019-06-10 22:20       ` Saeed Mahameed
  0 siblings, 0 replies; 7+ messages in thread
From: Saeed Mahameed @ 2019-06-10 22:20 UTC (permalink / raw)
  To: netdev, pabeni; +Cc: davem, leon

On Mon, 2019-06-10 at 10:43 +0200, Paolo Abeni wrote:
> Hi,
> 
> On Fri, 2019-06-07 at 18:09 +0000, Saeed Mahameed wrote:
> > On Thu, 2019-06-06 at 23:56 +0200, Paolo Abeni wrote:
> > > We can avoid another indirect call per packet wrapping the rx
> > > handler call with the proper helper.
> > > 
> > > To ensure that even the last listed direct call experience
> > > measurable gain, despite the additional conditionals we must
> > > traverse before reaching it, I tested reversing the order of the
> > > listed options, with performance differences below noise level.
> > > 
> > > Together with the previous indirect call patch, this gives
> > > ~6% performance improvement in raw UDP tput.
> > > 
> > > v1 -> v2:
> > >  - update the direct call list and use a macro to define it,
> > >    as per Saeed suggestion. An intermediated additional
> > >    macro is needed to allow arg list expansion
> > > 
> > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > ---
> > >  drivers/net/ethernet/mellanox/mlx5/core/en.h    | 4 ++++
> > >  drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 5 ++++-
> > >  2 files changed, 8 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > > b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > > index 3a183d690e23..52bcdc87cbe2 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > > @@ -148,6 +148,10 @@ struct page_pool;
> > >  
> > >  #define MLX5E_MSG_LEVEL			NETIF_MSG_LINK
> > >  
> > > +#define MLX5_RX_INDIRECT_CALL_LIST \
> > > +	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe,
> > > mlx5i_handle_rx_cqe, \
> > > +	mlx5e_ipsec_handle_rx_cqe
> > > +
> > >  #define mlx5e_dbg(mlevel, priv, format,
> > > ...)                    \
> > >  do
> > > {                                                            \
> > >  	if (NETIF_MSG_##mlevel & (priv)->msglevel)              \
> > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > > b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > > index 0fe5f13d07cc..7faf643eb1b9 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > > @@ -1303,6 +1303,8 @@ void mlx5e_handle_rx_cqe_mpwrq(struct
> > > mlx5e_rq
> > > *rq, struct mlx5_cqe64 *cqe)
> > >  	mlx5_wq_ll_pop(wq, cqe->wqe_id, &wqe->next.next_wqe_index);
> > >  }
> > >  
> > > +#define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f,
> > > list,
> > > __VA_ARGS__)
> > > +
> > 
> > Hi Paolo, 
> > 
> > This patch produces some compiler errors:
> > 
> > Please note that mlx5e_ipsec_handle_rx_cqe is only defined when
> > CONFIG_MLX5_EN_IPSEC is enabled.
> 
> I'm sorry, I dumbly did not fuzz vs mlx5 build options.
> 
> It looks like that, to cope with all the possible mixes, a not-so-
> nice
> macro maze is required; something alike the following:
> 
> #if defined(CONFIG_MLX5_EN_IPSEC) && defined (CONFIG_MLX5_CORE_IPOIB)
> 
> #define MLX5_RX_INDIRECT_CALL_LIST \
> 	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe,
> mlx5i_handle_rx_cqe, \
> 	mlx5e_ipsec_handle_rx_cqe
> #define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_4(f, list,
> __VA_ARGS__)
> 
> #elif defined(CONFIG_MLX5_EN_IPSEC)
> 
> #define MLX5_RX_INDIRECT_CALL_LIST \
> 	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe, \
> 	mlx5e_ipsec_handle_rx_cqe
> #define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_3(f, list,
> __VA_ARGS__)
> 
> #elif defined(CONFIG_MLX5_CORE_IPOIB)
> 
> #define MLX5_RX_INDIRECT_CALL_LIST \
> 	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe,
> mlx5i_handle_rx_cqe
> #define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_3(f, list,
> __VA_ARGS__)
> 
> #else
> 
> #define MLX5_RX_INDIRECT_CALL_LIST \
> 	mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe
> #define INDIRECT_CALL_LIST(f, list, ...) INDIRECT_CALL_2(f, list,
> __VA_ARGS__)
> 
> #endif
> 
> If you are ok with the above, I can include it in v3, otherwise I can
> either:
> 
> * drop patch 2/3 and use only the 2 alternatives
> (mlx5e_handle_rx_cqe_mpwrq, mlx5e_handle_rx_cqe) that are available
> regardless of the driver build options
> 

yea, the above is too much, maybe we can simplify, I will revisit it
later, for now, let's have the 2 functions that are always available,
after all they are the ones that really matter.

> * drop both patches 2/3 and 3/3
> 
> Any feedback welcome, thanks!
> 
> Paolo
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-06-10 22:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-06 21:56 [PATCH net-next v2 0/3] net/mlx5: use indirect call wrappers Paolo Abeni
2019-06-06 21:56 ` [PATCH net-next v2 1/3] net/mlx5e: use indirect calls wrapper for skb allocation Paolo Abeni
2019-06-06 21:56 ` [PATCH net-next v2 2/3] indirect call wrappers: add helpers for 3 and 4 ways switch Paolo Abeni
2019-06-06 21:56 ` [PATCH net-next v2 3/3] net/mlx5e: use indirect calls wrapper for the rx packet handler Paolo Abeni
2019-06-07 18:09   ` Saeed Mahameed
2019-06-10  8:43     ` Paolo Abeni
2019-06-10 22:20       ` Saeed Mahameed

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.