[ovs-dev] [PATCH] [RFC][windows]: Remap VPORT socket pool
aserdean at ovn.org
aserdean at ovn.org
Tue Nov 13 11:48:04 UTC 2018
Answers inlined.
> Subiect: Re: [ovs-dev] [PATCH] [RFC][windows]: Remap VPORT socket pool
>
> Hi Alin,
>
> Thanks for the patch. Shashank is giving it a run. How do we prevent this
> issue in the future?
[Alin Serdean] Thanks for trying it out, and I'm sorry I haven't caught this issue
in time.
The best way would be to have a CI and run at least some sanity
checks on the windows datapath when it is enabled.
I can work on getting some
tests from the: `make check-kernel` and `make check-kmod` to run on Windows
and we can reuse most of the logic.
Another useful CI would be to run the unit tests on Windows as well.
>
> For starters, I think we need to consolidate the usage under nl_* functions to
> ensure it's not doing something differently in vport_*.
[Alin Serdean] I agree. We should also spend some cycles switching to IOCPs
IMO.
>
> We need to look into investing in the test framework for catching basic
> Windows failures like these.
[Alin Serdean] We can reuse the test framework that is used on Linux as I pointed
above the biggest issue IMO is where and how we run it.
>
> Thanks,
> Sairam
>
> On 11/12/18, 8:51 AM, "ovs-dev-bounces at openvswitch.org on behalf of Alin
> Gabriel Serdean" <ovs-dev-bounces at openvswitch.org on behalf of
> aserdean at ovn.org> wrote:
>
> Fixes:
>
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
> .com%2Fopenvswitch%2Fovs-
> issues%2Fissues%2F164&data=02%7C01%7Cvsairam%40vmware.com%
> 7C76dd26cc93e145b3a12008d648bf0d08%7Cb39138ca3cee4b4aa4d6cd83d9dd
> 62f0%7C1%7C0%7C636776382753818056&sdata=E%2FWSjXq%2BMfQls
> GG9Fw59FblJxGTjOXYVFpNUKJtyp0k%3D&reserved=0
>
> Signed-off-by: Alin Gabriel Serdean <aserdean at ovn.org>
> ---
> 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
> dev at openvswitch.org
>
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.o
> penvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Cvsairam%40vmware.com%7C76dd26cc93e145b
> 3a12008d648bf0d08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C
> 636776382753818056&sdata=7ywMj5jsv3iLcXrTuDlqIvvZcojQtmlwVDjDs
> AGi3Es%3D&reserved=0
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list