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]
Message-ID: <7852500cd0008893985094fa20e2790436391e49.camel@mellanox.com>
Date:   Fri, 18 Oct 2019 20:53:42 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "jonathan.lemon@...il.com" <jonathan.lemon@...il.com>,
        "ilias.apalodimas@...aro.org" <ilias.apalodimas@...aro.org>,
        Tariq Toukan <tariqt@...lanox.com>,
        "brouer@...hat.com" <brouer@...hat.com>
CC:     "kernel-team@...com" <kernel-team@...com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 01/10 net-next] net/mlx5e: RX, Remove RX page-cache

On Wed, 2019-10-16 at 15:50 -0700, Jonathan Lemon wrote:
> From: Tariq Toukan <tariqt@...lanox.com>
> 
> Obsolete the RX page-cache layer, pages are now recycled
> in page_pool.
> 
> This patch introduce a temporary degradation as recycling
> does not keep the pages DMA-mapped. That is fixed in a
> downstream patch.
Tariq,

i think we need to have performance numbers here to show degradation.
i am sure that non XDP traffic TCP/UDP performance will be hit.

> 
> Issue: 1487631
> Signed-off-by: Tariq Toukan <tariqt@...lanox.com>
> 
> Signed-off-by: Jonathan Lemon <jonathan.lemon@...il.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h  | 13 ----
>  .../net/ethernet/mellanox/mlx5/core/en_main.c | 16 -----
>  .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 67 ++---------------
> --
>  3 files changed, 4 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 8d76452cacdc..0595cdcff594 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -583,18 +583,6 @@ struct mlx5e_mpw_info {
>  
>  #define MLX5E_MAX_RX_FRAGS 4
>  
> -/* a single cache unit is capable to serve one napi call (for non-
> striding rq)
> - * or a MPWQE (for striding rq).
> - */
> -#define MLX5E_CACHE_UNIT	(MLX5_MPWRQ_PAGES_PER_WQE >
> NAPI_POLL_WEIGHT ? \
> -				 MLX5_MPWRQ_PAGES_PER_WQE :
> NAPI_POLL_WEIGHT)
> -#define MLX5E_CACHE_SIZE	(4 *
> roundup_pow_of_two(MLX5E_CACHE_UNIT))
> -struct mlx5e_page_cache {
> -	u32 head;
> -	u32 tail;
> -	struct mlx5e_dma_info page_cache[MLX5E_CACHE_SIZE];
> -};
> -
>  struct mlx5e_rq;
>  typedef void (*mlx5e_fp_handle_rx_cqe)(struct mlx5e_rq*, struct
> mlx5_cqe64*);
>  typedef struct sk_buff *
> @@ -658,7 +646,6 @@ struct mlx5e_rq {
>  	struct mlx5e_rq_stats *stats;
>  	struct mlx5e_cq        cq;
>  	struct mlx5e_cq_decomp cqd;
> -	struct mlx5e_page_cache page_cache;
>  	struct hwtstamp_config *tstamp;
>  	struct mlx5_clock      *clock;
>  
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 7569287f8f3c..168be1f800a3 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -612,9 +612,6 @@ static int mlx5e_alloc_rq(struct mlx5e_channel
> *c,
>  		rq->dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
>  	}
>  
> -	rq->page_cache.head = 0;
> -	rq->page_cache.tail = 0;
> -
>  	return 0;
>  
>  err_free:
> @@ -640,8 +637,6 @@ static int mlx5e_alloc_rq(struct mlx5e_channel
> *c,
>  
>  static void mlx5e_free_rq(struct mlx5e_rq *rq)
>  {
> -	int i;
> -
>  	if (rq->xdp_prog)
>  		bpf_prog_put(rq->xdp_prog);
>  
> @@ -655,17 +650,6 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
>  		mlx5e_free_di_list(rq);
>  	}
>  
> -	for (i = rq->page_cache.head; i != rq->page_cache.tail;
> -	     i = (i + 1) & (MLX5E_CACHE_SIZE - 1)) {
> -		struct mlx5e_dma_info *dma_info = &rq-
> >page_cache.page_cache[i];
> -
> -		/* With AF_XDP, page_cache is not used, so this loop is
> not
> -		 * entered, and it's safe to call
> mlx5e_page_release_dynamic
> -		 * directly.
> -		 */
> -		mlx5e_page_release_dynamic(rq, dma_info, false);
> -	}
> -
>  	xdp_rxq_info_unreg(&rq->xdp_rxq);
>  	page_pool_destroy(rq->page_pool);
>  	mlx5_wq_destroy(&rq->wq_ctrl);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index d6a547238de0..a3773f8a4931 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -184,65 +184,9 @@ static inline u32
> mlx5e_decompress_cqes_start(struct mlx5e_rq *rq,
>  	return mlx5e_decompress_cqes_cont(rq, wq, 1, budget_rem) - 1;
>  }
>  
> -static inline bool mlx5e_page_is_reserved(struct page *page)
> -{
> -	return page_is_pfmemalloc(page) || page_to_nid(page) !=
> numa_mem_id();
> -}
> -
> -static inline bool mlx5e_rx_cache_put(struct mlx5e_rq *rq,
> -				      struct mlx5e_dma_info *dma_info)
> -{
> -	struct mlx5e_page_cache *cache = &rq->page_cache;
> -	u32 tail_next = (cache->tail + 1) & (MLX5E_CACHE_SIZE - 1);
> -	struct mlx5e_rq_stats *stats = rq->stats;
> -
> -	if (tail_next == cache->head) {
> -		stats->cache_full++;
> -		return false;
> -	}
> -
> -	if (unlikely(mlx5e_page_is_reserved(dma_info->page))) {
> -		stats->cache_waive++;
> -		return false;
> -	}
> -
> -	cache->page_cache[cache->tail] = *dma_info;
> -	cache->tail = tail_next;
> -	return true;
> -}
> -
> -static inline bool mlx5e_rx_cache_get(struct mlx5e_rq *rq,
> -				      struct mlx5e_dma_info *dma_info)
> -{
> -	struct mlx5e_page_cache *cache = &rq->page_cache;
> -	struct mlx5e_rq_stats *stats = rq->stats;
> -
> -	if (unlikely(cache->head == cache->tail)) {
> -		stats->cache_empty++;
> -		return false;
> -	}
> -
> -	if (page_ref_count(cache->page_cache[cache->head].page) != 1) {
> -		stats->cache_busy++;
> -		return false;
> -	}
> -
> -	*dma_info = cache->page_cache[cache->head];
> -	cache->head = (cache->head + 1) & (MLX5E_CACHE_SIZE - 1);
> -	stats->cache_reuse++;
> -
> -	dma_sync_single_for_device(rq->pdev, dma_info->addr,
> -				   PAGE_SIZE,
> -				   DMA_FROM_DEVICE);
> -	return true;
> -}
> -
>  static inline int mlx5e_page_alloc_pool(struct mlx5e_rq *rq,
>  					struct mlx5e_dma_info
> *dma_info)
>  {
> -	if (mlx5e_rx_cache_get(rq, dma_info))
> -		return 0;
> -
>  	dma_info->page = page_pool_dev_alloc_pages(rq->page_pool);
>  	if (unlikely(!dma_info->page))
>  		return -ENOMEM;
> @@ -276,14 +220,11 @@ void mlx5e_page_release_dynamic(struct mlx5e_rq
> *rq,
>  				struct mlx5e_dma_info *dma_info,
>  				bool recycle)
>  {
> -	if (likely(recycle)) {
> -		if (mlx5e_rx_cache_put(rq, dma_info))
> -			return;
> +	mlx5e_page_dma_unmap(rq, dma_info);
>  
> -		mlx5e_page_dma_unmap(rq, dma_info);
> +	if (likely(recycle)) {
>  		page_pool_recycle_direct(rq->page_pool, dma_info-
> >page);
>  	} else {
> -		mlx5e_page_dma_unmap(rq, dma_info);
>  		page_pool_release_page(rq->page_pool, dma_info->page);
>  		put_page(dma_info->page);
>  	}
> @@ -1167,7 +1108,7 @@ void mlx5e_handle_rx_cqe(struct mlx5e_rq *rq,
> struct mlx5_cqe64 *cqe)
>  	if (!skb) {
>  		/* probably for XDP */
>  		if (__test_and_clear_bit(MLX5E_RQ_FLAG_XDP_XMIT, rq-
> >flags)) {
> -			/* do not return page to cache,
> +			/* do not return page to pool,
>  			 * it will be returned on XDP_TX completion.
>  			 */
>  			goto wq_cyc_pop;
> @@ -1210,7 +1151,7 @@ void mlx5e_handle_rx_cqe_rep(struct mlx5e_rq
> *rq, struct mlx5_cqe64 *cqe)
>  	if (!skb) {
>  		/* probably for XDP */
>  		if (__test_and_clear_bit(MLX5E_RQ_FLAG_XDP_XMIT, rq-
> >flags)) {
> -			/* do not return page to cache,
> +			/* do not return page to pool,
>  			 * it will be returned on XDP_TX completion.
>  			 */
>  			goto wq_cyc_pop;

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ