From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Subject: Re: [memnic PATCH 3/5] pmd: implement stats of MEMNIC
Date: Mon, 24 Mar 2014 16:25:44 +0100
Message-ID: <1588240.i7QGELTBWt@xps13>
References: <7F861DC0615E0C47A872E6F3C5FCDDBD01073B19@BPXM14GP.gisp.nec.co.jp>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev-VfR2kkLFssw@public.gmane.org, Hayato Momma <h-momma-JhyGz2TFV9J8UrSeD/g0lQ@public.gmane.org>
To: Hiroshi Shimamoto <h-shimamoto-ehU+Cx/zZe18UrSeD/g0lQ@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD01073B19-ZmjkEB1lVlLt6d3pZDjeaEtBU8KWyXPq@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

Hi,

11/03/2014 05:38, Hiroshi Shimamoto:
> From: Hiroshi Shimamoto <h-shimamoto-ehU+Cx/zZe18UrSeD/g0lQ@public.gmane.org>
> 
> Implement missing feature to account statistics.
> This patch adds just an infrastructure.
> 
> Signed-off-by: Hiroshi Shimamoto <h-shimamoto-ehU+Cx/zZe18UrSeD/g0lQ@public.gmane.org>
> Reviewed-by: Hayato Momma <h-momma-JhyGz2TFV9J8UrSeD/g0lQ@public.gmane.org>

[...]

> @@ -51,6 +51,7 @@ struct memnic_adapter {
>  	int up_idx, down_idx;
>  	struct rte_mempool *mp;
>  	struct ether_addr mac_addr;
> +	struct rte_eth_stats stats[RTE_MAX_LCORE];
>  };

Could you make a comment to explain why you allocate a structure per core?
It is easier to read when locking strategy is described.

> +	for (i = 0; i < RTE_MAX_LCORE; i++) {
> +		struct rte_eth_stats *st = &adapter->stats[i];
> +
> +		memset(st, 0, sizeof(*st));
> +	}

Could you use only one memset for the array?

-- 
Thomas