From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrik Andersson R Subject: Re: [RFC] vhost user: add error handling for fd > 1023 Date: Mon, 11 Apr 2016 13:21:02 +0200 Message-ID: <570B889E.7050405@ericsson.com> References: <1458292380-9258-1-git-send-email-patrik.r.andersson@ericsson.com> <570379F9.6020306@ericsson.com> <570B3ECE.3050905@ericsson.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: "Xie, Huawei" , Daniele Di Proietto , "dev@dpdk.org" , Thomas Monjalon , "Ananyev, Konstantin" , Yuanhan Liu To: Christian Ehrhardt Return-path: Received: from sesbmg22.ericsson.net (sesbmg22.ericsson.net [193.180.251.48]) by dpdk.org (Postfix) with ESMTP id 374C42E8A for ; Mon, 11 Apr 2016 13:21:03 +0200 (CEST) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Yes, that is correct. Closing the socket on failure needs to be added. Regards, Patrik On 04/11/2016 11:34 AM, Christian Ehrhardt wrote: > I like the approach as well to go for the fix for robustness first. > > I was accidentally able to find another testcase to hit the same root > cause. > Adding guests with 15 vhost_user based NICs each while having rxq for > openvswitch-dpdk set to 4 and multiqueue for the guest devices at 4 > already breaks when adding the thirds such guests. > That is way earlier than I would have expected the fd's to be > exhausted but still the same root cause, so just another test for the > same. > > In prep to the wider check to the patch a minor review question from me: > On the section of rte_vhost_driver_register that now detects if there > were issues we might want to close the socketfd as well when bailing out. > Otherwise we would just have another source of fd leaks or would that > be reused later on even now that we have freed vserver-path and > vserver itself? > > ret = fdset_add(&g_vhost_server.fdset, vserver->listenfd, > vserver_new_vq_conn, NULL, vserver); > if (ret < 0) { > pthread_mutex_unlock(&g_vhost_server.server_mutex); > RTE_LOG(ERR, VHOST_CONFIG, > "failed to add listen fd %d to vhost server fdset\n", > vserver->listenfd); > free(vserver->path); > + close(vserver->listenfd); > free(vserver); > return -1; > } > > > Christian Ehrhardt > Software Engineer, Ubuntu Server > Canonical Ltd > > On Mon, Apr 11, 2016 at 8:06 AM, Patrik Andersson R > > wrote: > > I fully agree with this course of action. > > Thank you, > > Patrik > > > > On 04/08/2016 08:47 AM, Xie, Huawei wrote: > > On 4/7/2016 10:52 PM, Christian Ehrhardt wrote: > > I totally agree to that there is no deterministic rule > what to expect. > The only rule is that #fd certainly always is > > #vhost_user devices. > In various setup variants I've crossed fd 1024 anywhere > between 475 > and 970 vhost_user ports. > > Once the discussion continues and we have an updates > version of the > patch with some more agreement I hope I can help to test it. > > Thanks. Let us first temporarily fix this problem for > robustness, then > we consider whether upgrade to (e)poll. > Will check the patch in detail later. Basically it should work > but need > check whether we need extra fixes elsewhere. > > >