[ovs-dev] [PATCH] Windows: Fix broken kernel userspace communication

Alin Serdean aserdean at cloudbasesolutions.com
Thu Nov 15 17:27:19 UTC 2018


> -----Mesaj original-----
> De la: ovs-dev-bounces at openvswitch.org <ovs-dev-
> bounces at openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, November 15, 2018 7:12 PM
> Către: Alin Gabriel Serdean <aserdean at ovn.org>
> Cc: dev at openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] Windows: Fix broken kernel userspace
> communication
> 
> On Thu, Nov 15, 2018 at 06:42:38PM +0200, Alin Gabriel Serdean wrote:
> > Patch:
> >
> https://github.com/openvswitch/ovs/commit/69c51582ff786a68fc325c1c506
> 2
> > 4715482bc460 broke Windows userpace - kernel communication.
> >
> > On windows we create netlink sockets when the handlers are initiated
> > and reuse them.
> > This patch remaps the usage of the netlink socket pool.
> >
> > Fixes:
> > https://github.com/openvswitch/ovs-issues/issues/164
> >
> > Signed-off-by: Alin Gabriel Serdean <aserdean at ovn.org>
> > Acked-by: Shashank Ram <rams at vmware.com>
> > Tested-by: Shashank Ram <rams at vmware.com>
> 
> Sorry we broke Windows.  I apologize.
[Alin Serdean] No worries. Maybe this increases the awareness about the
need of a CI.
> 
> Usually I prefer to put all the #ifdefs in a single place when I can.
> In this case, I think that means putting them inside
> create_socksp_windows() and del_socksp_windows().  It's easy enough.
> 
> But del_socksp_windows() is a really weird function.  It does nothing:
> it sets one of its parameters to NULL, which is not visible outside the
> function, and it ignores its other parameter.  That would make sense if there
> were multiple implementations, or if it were referred to via function pointer,
> etc., but I don't understand it here.  Why call it at all?
[Alin Serdean] No reason, I forgot to remove when I sent the v1.
> 
> Anyhow, here is one variation that I suggest.  I built it on Linux but not on
> Windows, so maybe I screwed some things up.
> 
> Actually, looking at it again, I think that the first change in
> dpif_netlink_port_add__() fixes a bug: it looks like, previously, an error
> return from nl_sock_create() was passed up to the caller as a success.  I
> guess that should be fixed in a separate patch, first:
[Alin Serdean] Yup I folded it in my patch when I should've sent a different
patch.
I acked your patch.
>         https://mail.openvswitch.org/pipermail/ovs-dev/2018-
> November/353952.html
[Alin Serdean] Thanks a lot for the review and incremental patch!
> 
> --8<--------------------------cut here-------------------------->8--


More information about the dev mailing list