On 30.09.2015 05:23, Gurucharan Shetty wrote:
> I think Ben's patch would have fixed the problem had pollfd.fd
> actually held a negative value. Documentation in msdn says that it
> can, but when I step through the debugger, I see that a -1 set to that
> shows up us a large positive integer. For e.g., if I apply this
> hypothetical patch, I don't see test failures.
>
> diff --git a/lib/poll-loop.c b/lib/poll-loop.c
> index 36eb5ac..850eeac 100644
> --- a/lib/poll-loop.c
> +++ b/lib/poll-loop.c
> @@ -297,7 +297,7 @@ free_poll_nodes(struct poll_loop *loop)
> HMAP_FOR_EACH_SAFE (node, next, hmap_node, &loop->poll_nodes) {
> hmap_remove(&loop->poll_nodes, &node->hmap_node);
> #ifdef _WIN32
> - if (node->wevent && node->pollfd.fd) {
> + if (node->wevent && node->pollfd.fd <= 4400000) {
> WSAEventSelect(node->pollfd.fd, NULL, 0);
> CloseHandle(node->wevent);
> }
> @@ -341,7 +341,7 @@ poll_block(void)
> pollfds[i] = node->pollfd;
> #ifdef _WIN32
> wevents[i] = node->wevent;
> - if (node->pollfd.fd && node->wevent) {
> + if (node->pollfd.fd <= 4400000 && node->wevent) {
> short int wsa_events = 0;
> if (node->pollfd.events & POLLIN) {
> wsa_events |= FD_READ | FD_ACCEPT | FD_CLOSE;
>
>
> I actually cannot understand what was the bug that created the
> original commit in question. I cannot think of a place in OVS where we
> send fd as zero to create_poll_node().
>
> Ilya,
> Can you explain the original trigger that caused you to submit this
> patch? I would rather revert this unless I understand the reason.
In my case fd 0 was unexpectedly closed and tap interface opened on this fd.
So, this assert was triggered. It is not a common case, I agree, but it is not
right anyway to expect, that we will never get fd 0 as a result of open().
If you want to revert this patch you should forbid opening fd 0 at least
inside netdev-linux.
Best regards, Ilya Maximets.
>
>
> On Tue, Sep 29, 2015 at 5:07 PM, Gurucharan Shetty <[email protected]> wrote:
>> hash_2words takes unsigned int. I guess that is the problem?
>>
>> On Tue, Sep 29, 2015 at 5:00 PM, Alin Serdean
>> <[email protected]> wrote:
>>> I think so. I think for once we should do
>>> https://github.com/openvswitch/ovs/blob/master/lib/poll-loop.c#L119-L123
>>> before
>>> https://github.com/openvswitch/ovs/blob/master/lib/poll-loop.c#L115-L116
>>>
>>> I am compiling under debug and attaching a debugger to it.
>>>
>>> Alin.
>>>
>>>> -----Mesaj original-----
>>>> De la: Gurucharan Shetty [mailto:[email protected]]
>>>> Trimis: Wednesday, September 30, 2015 2:34 AM
>>>> Către: Alin Serdean <[email protected]>
>>>> Cc: Ben Pfaff <[email protected]>; Ilya Maximets <[email protected]>;
>>>> dev <[email protected]>; Dyasly Sergey <[email protected]>
>>>> Subiect: Re: [ovs-dev] [PATCH] poll-loop: fix assertion in poll_create_node
>>>>
>>>> Alin,
>>>> Reverting this commit on tip of master does not give me any unit test
>>>> failures. When you say there is more to it, do you mean that this commit
>>>> actually exposed some other bug?
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
