DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhao, Bing" <ilovethull@163.com>
To: Yuanhan Liu <yliu@fridaylinux.org>
Cc: dev@dpdk.org, bing.zhao@hxt-semitech.com
Subject: Re: [dpdk-dev] [PATCH] reset src fd field to -1 in fdset_move of vhost
Date: Mon, 22 Jan 2018 15:52:11 +0800	[thread overview]
Message-ID: <e48f3f2c-4c6a-bbe9-4a6d-cc29ac38176f@163.com> (raw)
In-Reply-To: <20180119143729.GP29540@yliu-mob>

On 2018/1/19 22:37, Yuanhan Liu wrote:
> On Thu, Dec 21, 2017 at 05:15:40PM +0800, Bing Zhao wrote:
>> In the fdset_move, after copying the fd&rwfds from the src to the dst, the fd should be set to -1. Or else in some cases, there will be a fault missing. E.g:
>> Before: 1 -1 3 4 -1 6 7 -1 9 10
>> After: 1 10 3 4 9 6 7 -1 9 10
>> Then the index7 will be returned correctly for the first time, but if another fd is to be added, it will fail.
> 
> Hi,
> 
> Have you met a real issue? I'm a bit doubt about that, since the fd array
> is also guarded by "pfdset->num", which makes sure we will not access
> those invalid entries (i.e. the last 2 entries in above example).
> 
> 	--yliu
> 
>> Signed-off-by: Bing Zhao <bing.zhao@hxt-semitech.com>
>> ---
>>   lib/librte_vhost/fd_man.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/librte_vhost/fd_man.c b/lib/librte_vhost/fd_man.c
>> index 4c6fed418..48594dd7f 100644
>> --- a/lib/librte_vhost/fd_man.c
>> +++ b/lib/librte_vhost/fd_man.c
>> @@ -63,6 +63,7 @@ fdset_move(struct fdset *pfdset, int dst, int src)
>>   {
>>   	pfdset->fd[dst]    = pfdset->fd[src];
>>   	pfdset->rwfds[dst] = pfdset->rwfds[src];
>> +	pfdset->fd[src].fd = -1;
>>   }
>>   
>>   static void
>> -- 
>> 2.11.0.windows.1
>>
Hello Yuanhan,
Thanks for your information. The answer is "no", and I just study the 
code and notice this. But yes you're right, I missed this. At first I 
thought there was a "-1" check in the "fdset_add_fd", indeed there isn't 
:). And no matter "-1" or other values in the "fd" element, if the "num" 
index points at that position, all the fields will be rewritten. So 
there is no problem.

Thanks again and please just ignore this.

      reply	other threads:[~2018-01-22  7:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-21  9:15 Bing Zhao
2018-01-19 14:37 ` Yuanhan Liu
2018-01-22  7:52   ` Zhao, Bing [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=e48f3f2c-4c6a-bbe9-4a6d-cc29ac38176f@163.com \
    --to=ilovethull@163.com \
    --cc=bing.zhao@hxt-semitech.com \
    --cc=dev@dpdk.org \
    --cc=yliu@fridaylinux.org \
    /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).