The ides is the following: https://msdn.microsoft.com/en-us/library/windows/desktop/ms740094%28v=vs.85%29.aspx and SOCKET is defined as: typedef UINT_PTR SOCKET;
When we do the following: node->pollfd.fd = fd; and fd = -1, node->pollfd.fd will rollover. Another aspect to the problem is that we can insert multiple times the same wevent into the map. According to the documentation: https://msdn.microsoft.com/en-us/library/windows/desktop/ms687025%28v=vs.85%29.aspx it should not. We should probably split: https://github.com/openvswitch/ovs/blob/master/lib/poll-loop.c#L69-L71 Into two: node->pollfd.fd == fd under *UNIX node->wevent == wevent under MSVC. I think we need a dedicated flag when we want to skip a given fd. Alin. > -----Mesaj original----- > De la: Gurucharan Shetty [mailto:shet...@nicira.com] > Trimis: Wednesday, September 30, 2015 5:24 AM > Către: Alin Serdean <aserd...@cloudbasesolutions.com> > Cc: Ben Pfaff <b...@nicira.com>; Ilya Maximets <i.maxim...@samsung.com>; > dev <dev@openvswitch.org>; Dyasly Sergey <s.dya...@samsung.com> > Subiect: Re: [ovs-dev] [PATCH] poll-loop: fix assertion in poll_create_node > > 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. > > > On Tue, Sep 29, 2015 at 5:07 PM, Gurucharan Shetty <shet...@nicira.com> > wrote: > > hash_2words takes unsigned int. I guess that is the problem? > > > > On Tue, Sep 29, 2015 at 5:00 PM, Alin Serdean > > <aserd...@cloudbasesolutions.com> wrote: > >> I think so. I think for once we should do > >> https://github.com/openvswitch/ovs/blob/master/lib/poll-loop.c#L119-L > >> 123 before > >> https://github.com/openvswitch/ovs/blob/master/lib/poll-loop.c#L115-L > >> 116 > >> > >> I am compiling under debug and attaching a debugger to it. > >> > >> Alin. > >> > >>> -----Mesaj original----- > >>> De la: Gurucharan Shetty [mailto:shet...@nicira.com] > >>> Trimis: Wednesday, September 30, 2015 2:34 AM > >>> Către: Alin Serdean <aserd...@cloudbasesolutions.com> > >>> Cc: Ben Pfaff <b...@nicira.com>; Ilya Maximets > >>> <i.maxim...@samsung.com>; dev <dev@openvswitch.org>; Dyasly > Sergey > >>> <s.dya...@samsung.com> > >>> 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 dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
