From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH] net/bonding: fix segfault when creating bonded device Date: Wed, 31 Oct 2018 16:44:21 +0100 Message-ID: <9177844.5q68cmoSxg@xps> References: <1540994346-17532-1-git-send-email-radu.nicolau@intel.com> <2490703.Y8HZJr6zFH@xps> <2adb03da-ff43-3ab4-5e5a-0a44622d5f8d@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: Radu Nicolau , dev@dpdk.org, declan.doherty@intel.com, chas3@att.com, ferruh.yigit@intel.com, arybchenko@solarflare.com To: Chas Williams <3chas3@gmail.com> Return-path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 543D6239 for ; Wed, 31 Oct 2018 16:44:17 +0100 (CET) In-Reply-To: <2adb03da-ff43-3ab4-5e5a-0a44622d5f8d@gmail.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 31/10/2018 16:37, Chas Williams: > > On 10/31/2018 11:06 AM, Thomas Monjalon wrote: > > 31/10/2018 14:59, Radu Nicolau: > >> After the patch below the call to rte_eth_bond_8023ad_agg_selection_set > >> from probe() segfaults; there is no need to call the function, just set > >> the mode directly. > >> > >> Fixes: 391797f04208 ("drivers/bus: move driver assignment to end of probing") > > > > It would not segfault if you call rte_eth_dev_probing_finish() at the > > real end of the probing function. Then the port will be considered not > > valid in rte_eth_bond_8023ad_agg_selection_set(). > > It does not solve your problem but it is more correct. > > So I suggest to revert this patch (which was a wrong fix): > > http://git.dpdk.org/dpdk/commit/?id=1620175 > > Or just make the change proposed in this commit and also move this > section before the probing finish. This is performing initial setup of > the interface and it doesn't need to use the public API to do this. > And this should be done before the public API can access the device. Yes, this is what I proposed, but said differently :) Please add Fixes: 1620175b400e ("net/bonding: fix invalid port id") > > Then the issue is to allow configuring a port before the end of probing. > > That shouldn't be allowed of course. > > > This patch is workarounding the public API which checks port validity. > > I think it is a good approach.