Hi Alin,

Thanks for the patch. Shashank is giving it a run. How do we prevent this issue 
in the future?

For starters, I think we need to consolidate the usage under nl_* functions to 
ensure it's not doing something differently in vport_*.

We need to look into investing in the test framework for catching basic Windows 
failures like these. 

Thanks,
Sairam

On 11/12/18, 8:51 AM, "[email protected] on behalf of Alin 
Gabriel Serdean" <[email protected] on behalf of 
[email protected]> wrote:

    Fixes:
    
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fopenvswitch%2Fovs-issues%2Fissues%2F164&amp;data=02%7C01%7Cvsairam%40vmware.com%7C76dd26cc93e145b3a12008d648bf0d08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636776382753818056&amp;sdata=E%2FWSjXq%2BMfQlsGG9Fw59FblJxGTjOXYVFpNUKJtyp0k%3D&amp;reserved=0
    
    Signed-off-by: Alin Gabriel Serdean <[email protected]>
    ---
     lib/dpif-netlink.c | 58 
+++++++++++++++++++++++++++++++++++++++++++++++++++++-
     1 file changed, 57 insertions(+), 1 deletion(-)
    
    diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
    index 69c145cc3..5994c3445 100644
    --- a/lib/dpif-netlink.c
    +++ b/lib/dpif-netlink.c
    @@ -246,6 +246,38 @@ static int dpif_netlink_port_query__(const struct 
dpif_netlink *dpif,
                                          odp_port_t port_no, const char 
*port_name,
                                          struct dpif_port *dpif_port);
     
    +
    +static struct nl_sock *
    +vport_create_socksp_windows(struct dpif_netlink *dpif, int *error)
    +OVS_REQ_WRLOCK(dpif->upcall_lock)
    +{
    +    struct nl_sock *socksp;
    +    size_t i;
    +    /* Pick netlink sockets to use in a round-robin fashion from each
    +     * handler's pool of sockets. */
    +    struct dpif_handler *handler = &dpif->handlers[0];
    +    struct dpif_windows_vport_sock *sock_pool = handler->vport_sock_pool;
    +    size_t index = handler->last_used_pool_idx;
    +    /* A pool of sockets is allocated when the handler is initialized. */
    +    if (sock_pool == NULL) {
    +        *error = EINVAL;
    +        return NULL;
    +    }
    +    ovs_assert(index < VPORT_SOCK_POOL_SIZE);
    +    socksp = sock_pool[index].nl_sock;
    +    ovs_assert(socksp);
    +    index = (index == VPORT_SOCK_POOL_SIZE - 1) ? 0 : index + 1;
    +    handler->last_used_pool_idx = index;
    +    *error = 0;
    +    return socksp;
    +}
    +static void
    +vport_del_socksp_windows(struct dpif_netlink *dpif, struct nl_sock *socksp)
    +{
    +    socksp = NULL;
    +}
    +
    +
     static struct dpif_netlink *
     dpif_netlink_cast(const struct dpif *dpif)
     {
    @@ -716,7 +748,12 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, 
const char *name,
         int error = 0;
     
         if (dpif->handlers) {
    -        if (nl_sock_create(NETLINK_GENERIC, &socksp)) {
    +#ifdef _WIN32
    +        socksp = vport_create_socksp_windows(dpif, &error);
    +#else
    +        error = nl_sock_create(NETLINK_GENERIC, &socksp);
    +#endif
    +        if (!socksp) {
                 return error;
             }
         }
    @@ -748,7 +785,11 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, 
const char *name,
                           dpif_name(&dpif->dpif), *port_nop);
             }
     
    +#ifdef _WIN32
    +        vport_del_socksp_windows(dpif, socksp);
    +#else
             nl_sock_destroy(socksp);
    +#endif
             goto exit;
         }
     
    @@ -763,7 +804,11 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, 
const char *name,
             request.dp_ifindex = dpif->dp_ifindex;
             request.port_no = *port_nop;
             dpif_netlink_vport_transact(&request, NULL, NULL);
    +#ifdef _WIN32
    +        vport_del_socksp_windows(dpif, socksp);
    +#else
             nl_sock_destroy(socksp);
    +#endif
             goto exit;
         }
     
    @@ -2249,15 +2294,26 @@ dpif_netlink_refresh_channels(struct dpif_netlink 
*dpif, uint32_t n_handlers)
                 || !vport_get_pid(dpif, port_no, &upcall_pid)) {
                 struct nl_sock *socksp;
     
    +#ifdef _WIN32
    +            socksp = vport_create_socksp_windows(dpif, &error);
    +            if (!socksp) {
    +                goto error;
    +            }
    +#else
                 if (nl_sock_create(NETLINK_GENERIC, &socksp)) {
                     goto error;
                 }
    +#endif
     
                 error = vport_add_channel(dpif, vport.port_no, socksp);
                 if (error) {
                     VLOG_INFO("%s: could not add channels for port %s",
                               dpif_name(&dpif->dpif), vport.name);
    +#ifdef _WIN32
    +                vport_del_socksp_windows(dpif, socksp);
    +#else
                     nl_sock_destroy(socksp);
    +#endif
                     retval = error;
                     goto error;
                 }
    -- 
    2.16.1.windows.1
    
    _______________________________________________
    dev mailing list
    [email protected]
    
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&amp;data=02%7C01%7Cvsairam%40vmware.com%7C76dd26cc93e145b3a12008d648bf0d08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636776382753818056&amp;sdata=7ywMj5jsv3iLcXrTuDlqIvvZcojQtmlwVDjDsAGi3Es%3D&amp;reserved=0
    

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to