From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2 3/4] net/failsafe: replace local
	sub-device with shared data
Date: Tue, 05 Mar 2019 18:58:04 +0100
Message-ID: <172443910.dRk5910QrU@xps>
References: <1551779507-10857-1-git-send-email-rasland@mellanox.com>
 <1551779507-10857-3-git-send-email-rasland@mellanox.com>
 <20190305173825.2ho27gqfx7f72xqb@bidouze.vm.6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Cc: Raslan Darawsheh <rasland@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "stephen@networkplumber.org" <stephen@networkplumber.org>
To: =?ISO-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com
 [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 4BB344C9C
 for <dev@dpdk.org>; Tue,  5 Mar 2019 18:58:08 +0100 (CET)
In-Reply-To: <20190305173825.2ho27gqfx7f72xqb@bidouze.vm.6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi,

05/03/2019 18:38, Ga=EBtan Rivet:
> >  fs_dev_remove(struct sub_device *sdev)
[...]
> > -		rte_eth_dev_close(PORT_ID(sdev));
> > +		rte_eth_dev_close(edev->data->port_id);
>=20
> Ok I see. I missed that during the first reading, the private_data is
> zeroed on dev_close(), so ETH(sdev) becomes invalid here.

I don't follow you. What do you mean with this comment?

> What happens when a primary process closes a device before a secondary?
> Is the secondary unable to stop / close its own then? Isn't there some
> missing uninit?

Is the secondary process supposed to do any closing?
The device management should be done only by the primary process.

Note: anyway all this hotplug related code should be dropped
from failsafe to be replaced by EAL hotplug management.

> This seems dangerous to me. Why not instead allocating a per-process
> slab of memory that would hold the relevant references and outlive the
> shared data (a per-process rte_eth_dev private data...).

Which data do you think should be allocated per process?