From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH v5 1/2] librte_ether: add protection against
	overwrite device data
Date: Thu, 06 Oct 2016 16:20:47 +0200
Message-ID: <1556703.BdlfNHUksg@xps13>
References: <1474974783-4861-2-git-send-email-marcinx.kerlin@intel.com>
 <1538060.S3DiiuebUu@xps13>
 <68D830D942438745AD09BAFA99E33E812BE4BC@IRSMSX102.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>,
 "Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com>
To: "Kerlin, MarcinX" <marcinx.kerlin@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44])
 by dpdk.org (Postfix) with ESMTP id C87B85A0A
 for <dev@dpdk.org>; Thu,  6 Oct 2016 16:20:55 +0200 (CEST)
Received: by mail-wm0-f44.google.com with SMTP id i130so6934991wmg.1
 for <dev@dpdk.org>; Thu, 06 Oct 2016 07:20:55 -0700 (PDT)
In-Reply-To: <68D830D942438745AD09BAFA99E33E812BE4BC@IRSMSX102.ger.corp.intel.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2016-10-06 13:57, Kerlin, MarcinX:
> Hi Thomas,
> 
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 
> > Hi Marcin,
> > 
> > 2016-09-30 16:00, Marcin Kerlin:
> > > Added protection against overwrite device data in array
> > > rte_eth_dev_data[] for the next secondary applications. Secondary
> > > process appends in the first free place rather than at the beginning.
> > > This behavior prevents overwriting devices data of primary process by
> > secondary process.
> > 
> > I've just realized that you are trying to fix an useless code.
> > Why not just remove the array rte_eth_dev_data[] at first?
> 
> because pointer to rte_eth_dev_data in rte_eth_devices[] is 
> just to array rte_eth_dev_data[].
> 
> rte_ethdev.c:214 
> eth_dev->data = &rte_eth_dev_data[port_id];

This line indicates that the pointer data is to the struct rte_eth_dev_data
of the port_id, not the array of every devices.

> > We already have the array rte_eth_devices[] and there is a pointer to
> > rte_eth_dev_data in rte_eth_dev.
> 
> As you write above there is a pointer, but after run secondary testpmd this pointer
> will indicate data which hold from now data for secondary testpmd.
[...]
I think I've understood the bug.
I'm just saying you are fixing a weird design (rte_eth_dev_data[]).

> > Is it just a workaround to be able to lookup the rte_eth_dev_data memzone in
> > the secondary process?
> 
> No it is not workaround, it is protection against overwrite device data.
> I think that my cover letter good explain what is wrong. I did there
> short debug log.

I'm talking about the initial introduction of rte_eth_dev_data[]
which seems to be a workaround for multi-process without touching
rte_eth_devices[] allocated as a global variable (not a memzone).

> > So wouldn't it be saner to have rte_eth_devices[] in a memzone?
> 
> So you mean that move rte_eth_devices[] to memzone + remove rte_eth_dev_data[].

Yes

> What will indicate pointer inside rte_eth_dev  rte_eth_devices[]:
> (struct rte_eth_dev_data *data;  /**< Pointer to device data */)

After thinking more about it, I realize that rte_eth_devices cannot be
in a shared memzone because of its local pointers.

Sorry for the noise, I'll reconsider your patch.