From: Patrik Andersson R <patrik.r.andersson@ericsson.com>
To: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Cc: "Xie, Huawei" <huawei.xie@intel.com>,
Daniele Di Proietto <diproiettod@vmware.com>,
"dev@dpdk.org" <dev@dpdk.org>,
Thomas Monjalon <thomas.monjalon@6wind.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: Re: [dpdk-dev] [RFC] vhost user: add error handling for fd > 1023
Date: Mon, 11 Apr 2016 13:21:02 +0200 [thread overview]
Message-ID: <570B889E.7050405@ericsson.com> (raw)
In-Reply-To: <CAATJJ0+E72ZdCi20xLfSy=Ybq1ZAieodjbPR_nNOaRC6vkPY4A@mail.gmail.com>
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
> <patrik.r.andersson@ericsson.com
> <mailto:patrik.r.andersson@ericsson.com>> 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.
>
>
>
prev parent reply other threads:[~2016-04-11 11:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-18 9:13 Patrik Andersson
2016-03-30 9:05 ` Xie, Huawei
2016-04-05 8:40 ` Patrik Andersson R
2016-04-07 14:49 ` Christian Ehrhardt
2016-04-08 6:47 ` Xie, Huawei
2016-04-11 6:06 ` Patrik Andersson R
2016-04-11 9:34 ` Christian Ehrhardt
2016-04-11 11:21 ` Patrik Andersson R [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=570B889E.7050405@ericsson.com \
--to=patrik.r.andersson@ericsson.com \
--cc=christian.ehrhardt@canonical.com \
--cc=dev@dpdk.org \
--cc=diproiettod@vmware.com \
--cc=huawei.xie@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=thomas.monjalon@6wind.com \
--cc=yuanhan.liu@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).