DPDK patches and discussions
 help / color / mirror / Atom feed
From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
To: Patrik Andersson R <patrik.r.andersson@ericsson.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 11:34:58 +0200	[thread overview]
Message-ID: <CAATJJ0+E72ZdCi20xLfSy=Ybq1ZAieodjbPR_nNOaRC6vkPY4A@mail.gmail.com> (raw)
In-Reply-To: <570B3ECE.3050905@ericsson.com>

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> 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.
>>
>
>

  reply	other threads:[~2016-04-11  9:35 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 [this message]
2016-04-11 11:21             ` Patrik Andersson R

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='CAATJJ0+E72ZdCi20xLfSy=Ybq1ZAieodjbPR_nNOaRC6vkPY4A@mail.gmail.com' \
    --to=christian.ehrhardt@canonical.com \
    --cc=dev@dpdk.org \
    --cc=diproiettod@vmware.com \
    --cc=huawei.xie@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=patrik.r.andersson@ericsson.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).