[ovs-dev,v2,3/7] dpif-netlink: code to create/destroy tunnel ports via rtnetlink

Submitted by Eric Garver on March 16, 2017, 10:10 p.m.

Details

Message ID 20170316221021.11149-4-e@erig.me
State Changes Requested
Delegated to: Joe Stringer
Headers show

Commit Message

Eric Garver March 16, 2017, 10:10 p.m.
In order to be able to add those tunnels, we need to add code to create
the tunnels and add them as NETDEV vports. And when there is no support
to create them, we need to fallback to compatibility code and add them
as tunnel vports.

When removing those tunnels, we need to remove the interfaces as well,
and detecting the right type might be important, at least to distinguish
the tunnel vports that we should remove and the interfaces that we
shouldn't.

Co-authored-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
Signed-off-by: Eric Garver <e@erig.me>
---
 lib/automake.mk         |  3 +++
 lib/dpif-netlink-rtnl.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/dpif-netlink-rtnl.h | 47 +++++++++++++++++++++++++++++++++++++++
 lib/dpif-netlink.c      | 52 +++++++++++++++++++++++++++++++++++--------
 lib/dpif-netlink.h      |  2 ++
 5 files changed, 154 insertions(+), 9 deletions(-)
 create mode 100644 lib/dpif-netlink-rtnl.c
 create mode 100644 lib/dpif-netlink-rtnl.h

Comments

Joe Stringer March 28, 2017, 1:07 a.m.
On 16 March 2017 at 15:10, Eric Garver <e@erig.me> wrote:
> In order to be able to add those tunnels, we need to add code to create
> the tunnels and add them as NETDEV vports. And when there is no support
> to create them, we need to fallback to compatibility code and add them
> as tunnel vports.
>
> When removing those tunnels, we need to remove the interfaces as well,
> and detecting the right type might be important, at least to distinguish
> the tunnel vports that we should remove and the interfaces that we
> shouldn't.
>
> Co-authored-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
> Signed-off-by: Eric Garver <e@erig.me>
> ---
>  lib/automake.mk         |  3 +++
>  lib/dpif-netlink-rtnl.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/dpif-netlink-rtnl.h | 47 +++++++++++++++++++++++++++++++++++++++
>  lib/dpif-netlink.c      | 52 +++++++++++++++++++++++++++++++++++--------
>  lib/dpif-netlink.h      |  2 ++
>  5 files changed, 154 insertions(+), 9 deletions(-)
>  create mode 100644 lib/dpif-netlink-rtnl.c
>  create mode 100644 lib/dpif-netlink-rtnl.h
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index b266af13e4c7..73706529de5a 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -352,6 +352,8 @@ if LINUX
>  lib_libopenvswitch_la_SOURCES += \
>         lib/dpif-netlink.c \
>         lib/dpif-netlink.h \
> +       lib/dpif-netlink-rtnl.c \
> +       lib/dpif-netlink-rtnl.h \
>         lib/if-notifier.c \
>         lib/if-notifier.h \
>         lib/netdev-linux.c \
> @@ -382,6 +384,7 @@ if WIN32
>  lib_libopenvswitch_la_SOURCES += \
>         lib/dpif-netlink.c \
>         lib/dpif-netlink.h \
> +       lib/dpif-netlink-rtnl.h \
>         lib/netdev-windows.c \
>         lib/netlink-conntrack.c \
>         lib/netlink-conntrack.h \
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> new file mode 100644
> index 000000000000..1f816feee569
> --- /dev/null
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright (c) 2017 Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +#include "dpif-netlink-rtnl.h"
> +#include "dpif-netlink.h"
> +
> +
> +int
> +dpif_netlink_rtnl_port_create(struct netdev *netdev)
> +{
> +    switch (netdev_to_ovs_vport_type(netdev_get_type(netdev))) {
> +    case OVS_VPORT_TYPE_VXLAN:
> +    case OVS_VPORT_TYPE_GRE:
> +    case OVS_VPORT_TYPE_GENEVE:
> +    case OVS_VPORT_TYPE_NETDEV:
> +    case OVS_VPORT_TYPE_INTERNAL:
> +    case OVS_VPORT_TYPE_LISP:
> +    case OVS_VPORT_TYPE_STT:
> +    case OVS_VPORT_TYPE_UNSPEC:
> +    case __OVS_VPORT_TYPE_MAX:
> +    default:
> +        return EOPNOTSUPP;
> +    }
> +    return 0;
> +}
> +
> +int
> +dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED, const char *type)
> +{
> +    switch (netdev_to_ovs_vport_type(type)) {
> +    case OVS_VPORT_TYPE_VXLAN:
> +    case OVS_VPORT_TYPE_GRE:
> +    case OVS_VPORT_TYPE_GENEVE:
> +    case OVS_VPORT_TYPE_NETDEV:
> +    case OVS_VPORT_TYPE_INTERNAL:
> +    case OVS_VPORT_TYPE_LISP:
> +    case OVS_VPORT_TYPE_STT:
> +    case OVS_VPORT_TYPE_UNSPEC:
> +    case __OVS_VPORT_TYPE_MAX:
> +    default:
> +        return EOPNOTSUPP;
> +    }
> +    return 0;
> +}
> diff --git a/lib/dpif-netlink-rtnl.h b/lib/dpif-netlink-rtnl.h
> new file mode 100644
> index 000000000000..5fef314a20f6
> --- /dev/null
> +++ b/lib/dpif-netlink-rtnl.h
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright (c) 2017 Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef DPIF_NETLINK_RTNL_H
> +#define DPIF_NETLINK_RTNL_H 1
> +
> +#include <errno.h>
> +
> +#include "netdev.h"
> +
> +/* Declare these to keep sparse happy. */
> +int dpif_netlink_rtnl_port_create(struct netdev *netdev);
> +int dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
> +                                   const char *type);
> +
> +#ifndef __linux__
> +/* Dummy implementations for non Linux builds. */
> +
> +static inline int
> +dpif_netlink_rtnl_port_create(struct netdev *netdev OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +static inline int
> +dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
> +                               const char *type OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +#endif
> +
> +#endif /* DPIF_NETLINK_RTNL_H */
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index ee6a30ad5f10..572e365e8f49 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -34,6 +34,7 @@
>
>  #include "bitmap.h"
>  #include "dpif-provider.h"
> +#include "dpif-netlink-rtnl.h"
>  #include "openvswitch/dynamic-string.h"
>  #include "flow.h"
>  #include "fat-rwlock.h"
> @@ -60,9 +61,6 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink);
>  #ifdef _WIN32
>  #include "wmi.h"
>  enum { WINDOWS = 1 };
> -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);
>  #else
>  enum { WINDOWS = 0 };
>  #endif
> @@ -224,6 +222,10 @@ static void dpif_netlink_vport_to_ofpbuf(const struct dpif_netlink_vport *,
>                                           struct ofpbuf *);
>  static int dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *,
>                                            const struct ofpbuf *);
> +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 dpif_netlink *
>  dpif_netlink_cast(const struct dpif *dpif)
> @@ -783,7 +785,7 @@ get_vport_type(const struct dpif_netlink_vport *vport)
>      return "unknown";
>  }
>
> -static enum ovs_vport_type
> +enum ovs_vport_type
>  netdev_to_ovs_vport_type(const char *type)
>  {
>      if (!strcmp(type, "tap") || !strcmp(type, "system")) {
> @@ -945,8 +947,34 @@ dpif_netlink_port_add_compat(struct dpif_netlink *dpif, struct netdev *netdev,
>
>  }
>
> +static int OVS_UNUSED
> +dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif,
> +                                      struct netdev *netdev,
> +                                      odp_port_t *port_nop)
> +    OVS_REQ_WRLOCK(dpif->upcall_lock)
> +{
> +    int error;
> +    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> +    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> +    const char *name = netdev_vport_get_dpif_port(netdev,
> +                                                  namebuf, sizeof namebuf);
>
> +    error = dpif_netlink_rtnl_port_create(netdev);
> +    if (error) {
> +        if (error != EOPNOTSUPP) {
> +            VLOG_INFO_RL(&rl, "Failed to create %s with rtnetlink. error = %d",
> +                         netdev_get_name(netdev), error);
> +        }
> +        return error;
> +    }
>
> +    error = dpif_netlink_port_add__(dpif, name, OVS_VPORT_TYPE_NETDEV, NULL,
> +                                    port_nop);
> +    if (error) {
> +        dpif_netlink_rtnl_port_destroy(name, netdev_get_type(netdev));
> +    }
> +    return error;
> +}
>
>  static int
>  dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev,
> @@ -967,19 +995,23 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
>      OVS_REQ_WRLOCK(dpif->upcall_lock)
>  {
>      struct dpif_netlink_vport vport;
> +    struct dpif_port dpif_port;
>      int error;
>
> +    error = dpif_netlink_port_query__(dpif, port_no, NULL, &dpif_port);
> +    if (error) {
> +        return error;
> +    }
> +
>      dpif_netlink_vport_init(&vport);
>      vport.cmd = OVS_VPORT_CMD_DEL;
>      vport.dp_ifindex = dpif->dp_ifindex;
>      vport.port_no = port_no;
>  #ifdef _WIN32
> -    struct dpif_port temp_dpif_port;
> -    dpif_netlink_port_query__(dpif, port_no, NULL, &temp_dpif_port);
> -    if (!strcmp(temp_dpif_port.type, "internal")) {
> -        if (!delete_wmi_port(temp_dpif_port.name)){
> +    if (!strcmp(dpif_port.type, "internal")) {
> +        if (!delete_wmi_port(dpif_port.name)) {
>              VLOG_ERR("Could not delete wmi port with name: %s",
> -                     temp_dpif_port.name);
> +                     dpif_port.name);
>          };
>      }
>  #endif
> @@ -987,6 +1019,8 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
>
>      vport_del_channels(dpif, port_no);
>
> +    dpif_port_destroy(&dpif_port);
> +

Looks like windows has a memory leak in the existing path because it
doesn't free the temp_dpif_port.{name,type}. Perhaps we should split
out this change so we can backport the fix to affected branches?
Eric Garver March 28, 2017, 10:44 p.m.
On Mon, Mar 27, 2017 at 06:07:39PM -0700, Joe Stringer wrote:
> On 16 March 2017 at 15:10, Eric Garver <e@erig.me> wrote:
> > In order to be able to add those tunnels, we need to add code to create
> > the tunnels and add them as NETDEV vports. And when there is no support
> > to create them, we need to fallback to compatibility code and add them
> > as tunnel vports.
> >
> > When removing those tunnels, we need to remove the interfaces as well,
> > and detecting the right type might be important, at least to distinguish
> > the tunnel vports that we should remove and the interfaces that we
> > shouldn't.
> >
> > Co-authored-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
> > Signed-off-by: Eric Garver <e@erig.me>
> > ---
> >  lib/automake.mk         |  3 +++
> >  lib/dpif-netlink-rtnl.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/dpif-netlink-rtnl.h | 47 +++++++++++++++++++++++++++++++++++++++
> >  lib/dpif-netlink.c      | 52 +++++++++++++++++++++++++++++++++++--------
> >  lib/dpif-netlink.h      |  2 ++
> >  5 files changed, 154 insertions(+), 9 deletions(-)
> >  create mode 100644 lib/dpif-netlink-rtnl.c
> >  create mode 100644 lib/dpif-netlink-rtnl.h
> >
> > diff --git a/lib/automake.mk b/lib/automake.mk
> > index b266af13e4c7..73706529de5a 100644
> > --- a/lib/automake.mk
> > +++ b/lib/automake.mk
> > @@ -352,6 +352,8 @@ if LINUX
> >  lib_libopenvswitch_la_SOURCES += \
> >         lib/dpif-netlink.c \
> >         lib/dpif-netlink.h \
> > +       lib/dpif-netlink-rtnl.c \
> > +       lib/dpif-netlink-rtnl.h \
> >         lib/if-notifier.c \
> >         lib/if-notifier.h \
> >         lib/netdev-linux.c \
> > @@ -382,6 +384,7 @@ if WIN32
> >  lib_libopenvswitch_la_SOURCES += \
> >         lib/dpif-netlink.c \
> >         lib/dpif-netlink.h \
> > +       lib/dpif-netlink-rtnl.h \
> >         lib/netdev-windows.c \
> >         lib/netlink-conntrack.c \
> >         lib/netlink-conntrack.h \
> > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> > new file mode 100644
> > index 000000000000..1f816feee569
> > --- /dev/null
> > +++ b/lib/dpif-netlink-rtnl.c
> > @@ -0,0 +1,59 @@
> > +/*
> > + * Copyright (c) 2017 Red Hat, Inc.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + *     http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#include <config.h>
> > +
> > +#include "dpif-netlink-rtnl.h"
> > +#include "dpif-netlink.h"
> > +
> > +
> > +int
> > +dpif_netlink_rtnl_port_create(struct netdev *netdev)
> > +{
> > +    switch (netdev_to_ovs_vport_type(netdev_get_type(netdev))) {
> > +    case OVS_VPORT_TYPE_VXLAN:
> > +    case OVS_VPORT_TYPE_GRE:
> > +    case OVS_VPORT_TYPE_GENEVE:
> > +    case OVS_VPORT_TYPE_NETDEV:
> > +    case OVS_VPORT_TYPE_INTERNAL:
> > +    case OVS_VPORT_TYPE_LISP:
> > +    case OVS_VPORT_TYPE_STT:
> > +    case OVS_VPORT_TYPE_UNSPEC:
> > +    case __OVS_VPORT_TYPE_MAX:
> > +    default:
> > +        return EOPNOTSUPP;
> > +    }
> > +    return 0;
> > +}
> > +
> > +int
> > +dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED, const char *type)
> > +{
> > +    switch (netdev_to_ovs_vport_type(type)) {
> > +    case OVS_VPORT_TYPE_VXLAN:
> > +    case OVS_VPORT_TYPE_GRE:
> > +    case OVS_VPORT_TYPE_GENEVE:
> > +    case OVS_VPORT_TYPE_NETDEV:
> > +    case OVS_VPORT_TYPE_INTERNAL:
> > +    case OVS_VPORT_TYPE_LISP:
> > +    case OVS_VPORT_TYPE_STT:
> > +    case OVS_VPORT_TYPE_UNSPEC:
> > +    case __OVS_VPORT_TYPE_MAX:
> > +    default:
> > +        return EOPNOTSUPP;
> > +    }
> > +    return 0;
> > +}
> > diff --git a/lib/dpif-netlink-rtnl.h b/lib/dpif-netlink-rtnl.h
> > new file mode 100644
> > index 000000000000..5fef314a20f6
> > --- /dev/null
> > +++ b/lib/dpif-netlink-rtnl.h
> > @@ -0,0 +1,47 @@
> > +/*
> > + * Copyright (c) 2017 Red Hat, Inc.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + *     http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#ifndef DPIF_NETLINK_RTNL_H
> > +#define DPIF_NETLINK_RTNL_H 1
> > +
> > +#include <errno.h>
> > +
> > +#include "netdev.h"
> > +
> > +/* Declare these to keep sparse happy. */
> > +int dpif_netlink_rtnl_port_create(struct netdev *netdev);
> > +int dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
> > +                                   const char *type);
> > +
> > +#ifndef __linux__
> > +/* Dummy implementations for non Linux builds. */
> > +
> > +static inline int
> > +dpif_netlink_rtnl_port_create(struct netdev *netdev OVS_UNUSED)
> > +{
> > +    return EOPNOTSUPP;
> > +}
> > +
> > +static inline int
> > +dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
> > +                               const char *type OVS_UNUSED)
> > +{
> > +    return EOPNOTSUPP;
> > +}
> > +
> > +#endif
> > +
> > +#endif /* DPIF_NETLINK_RTNL_H */
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index ee6a30ad5f10..572e365e8f49 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -34,6 +34,7 @@
> >
> >  #include "bitmap.h"
> >  #include "dpif-provider.h"
> > +#include "dpif-netlink-rtnl.h"
> >  #include "openvswitch/dynamic-string.h"
> >  #include "flow.h"
> >  #include "fat-rwlock.h"
> > @@ -60,9 +61,6 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink);
> >  #ifdef _WIN32
> >  #include "wmi.h"
> >  enum { WINDOWS = 1 };
> > -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);
> >  #else
> >  enum { WINDOWS = 0 };
> >  #endif
> > @@ -224,6 +222,10 @@ static void dpif_netlink_vport_to_ofpbuf(const struct dpif_netlink_vport *,
> >                                           struct ofpbuf *);
> >  static int dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *,
> >                                            const struct ofpbuf *);
> > +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 dpif_netlink *
> >  dpif_netlink_cast(const struct dpif *dpif)
> > @@ -783,7 +785,7 @@ get_vport_type(const struct dpif_netlink_vport *vport)
> >      return "unknown";
> >  }
> >
> > -static enum ovs_vport_type
> > +enum ovs_vport_type
> >  netdev_to_ovs_vport_type(const char *type)
> >  {
> >      if (!strcmp(type, "tap") || !strcmp(type, "system")) {
> > @@ -945,8 +947,34 @@ dpif_netlink_port_add_compat(struct dpif_netlink *dpif, struct netdev *netdev,
> >
> >  }
> >
> > +static int OVS_UNUSED
> > +dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif,
> > +                                      struct netdev *netdev,
> > +                                      odp_port_t *port_nop)
> > +    OVS_REQ_WRLOCK(dpif->upcall_lock)
> > +{
> > +    int error;
> > +    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> > +    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> > +    const char *name = netdev_vport_get_dpif_port(netdev,
> > +                                                  namebuf, sizeof namebuf);
> >
> > +    error = dpif_netlink_rtnl_port_create(netdev);
> > +    if (error) {
> > +        if (error != EOPNOTSUPP) {
> > +            VLOG_INFO_RL(&rl, "Failed to create %s with rtnetlink. error = %d",
> > +                         netdev_get_name(netdev), error);
> > +        }
> > +        return error;
> > +    }
> >
> > +    error = dpif_netlink_port_add__(dpif, name, OVS_VPORT_TYPE_NETDEV, NULL,
> > +                                    port_nop);
> > +    if (error) {
> > +        dpif_netlink_rtnl_port_destroy(name, netdev_get_type(netdev));
> > +    }
> > +    return error;
> > +}
> >
> >  static int
> >  dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev,
> > @@ -967,19 +995,23 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
> >      OVS_REQ_WRLOCK(dpif->upcall_lock)
> >  {
> >      struct dpif_netlink_vport vport;
> > +    struct dpif_port dpif_port;
> >      int error;
> >
> > +    error = dpif_netlink_port_query__(dpif, port_no, NULL, &dpif_port);
> > +    if (error) {
> > +        return error;
> > +    }
> > +
> >      dpif_netlink_vport_init(&vport);
> >      vport.cmd = OVS_VPORT_CMD_DEL;
> >      vport.dp_ifindex = dpif->dp_ifindex;
> >      vport.port_no = port_no;
> >  #ifdef _WIN32
> > -    struct dpif_port temp_dpif_port;
> > -    dpif_netlink_port_query__(dpif, port_no, NULL, &temp_dpif_port);
> > -    if (!strcmp(temp_dpif_port.type, "internal")) {
> > -        if (!delete_wmi_port(temp_dpif_port.name)){
> > +    if (!strcmp(dpif_port.type, "internal")) {
> > +        if (!delete_wmi_port(dpif_port.name)) {
> >              VLOG_ERR("Could not delete wmi port with name: %s",
> > -                     temp_dpif_port.name);
> > +                     dpif_port.name);
> >          };
> >      }
> >  #endif
> > @@ -987,6 +1019,8 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
> >
> >      vport_del_channels(dpif, port_no);
> >
> > +    dpif_port_destroy(&dpif_port);
> > +
> 
> Looks like windows has a memory leak in the existing path because it
> doesn't free the temp_dpif_port.{name,type}. Perhaps we should split
> out this change so we can backport the fix to affected branches?

Agreed. I'll submit a separate patch so we can backport it other
releases.
Joe Stringer March 28, 2017, 11:49 p.m.
On 28 March 2017 at 15:44, Eric Garver <e@erig.me> wrote:
> On Mon, Mar 27, 2017 at 06:07:39PM -0700, Joe Stringer wrote:
>> On 16 March 2017 at 15:10, Eric Garver <e@erig.me> wrote:
>> > In order to be able to add those tunnels, we need to add code to create
>> > the tunnels and add them as NETDEV vports. And when there is no support
>> > to create them, we need to fallback to compatibility code and add them
>> > as tunnel vports.
>> >
>> > When removing those tunnels, we need to remove the interfaces as well,
>> > and detecting the right type might be important, at least to distinguish
>> > the tunnel vports that we should remove and the interfaces that we
>> > shouldn't.
>> >
>> > Co-authored-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
>> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
>> > Signed-off-by: Eric Garver <e@erig.me>
>> > ---
>> >  lib/automake.mk         |  3 +++
>> >  lib/dpif-netlink-rtnl.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
>> >  lib/dpif-netlink-rtnl.h | 47 +++++++++++++++++++++++++++++++++++++++
>> >  lib/dpif-netlink.c      | 52 +++++++++++++++++++++++++++++++++++--------
>> >  lib/dpif-netlink.h      |  2 ++
>> >  5 files changed, 154 insertions(+), 9 deletions(-)
>> >  create mode 100644 lib/dpif-netlink-rtnl.c
>> >  create mode 100644 lib/dpif-netlink-rtnl.h
>> >
>> > diff --git a/lib/automake.mk b/lib/automake.mk
>> > index b266af13e4c7..73706529de5a 100644
>> > --- a/lib/automake.mk
>> > +++ b/lib/automake.mk
>> > @@ -352,6 +352,8 @@ if LINUX
>> >  lib_libopenvswitch_la_SOURCES += \
>> >         lib/dpif-netlink.c \
>> >         lib/dpif-netlink.h \
>> > +       lib/dpif-netlink-rtnl.c \
>> > +       lib/dpif-netlink-rtnl.h \
>> >         lib/if-notifier.c \
>> >         lib/if-notifier.h \
>> >         lib/netdev-linux.c \
>> > @@ -382,6 +384,7 @@ if WIN32
>> >  lib_libopenvswitch_la_SOURCES += \
>> >         lib/dpif-netlink.c \
>> >         lib/dpif-netlink.h \
>> > +       lib/dpif-netlink-rtnl.h \
>> >         lib/netdev-windows.c \
>> >         lib/netlink-conntrack.c \
>> >         lib/netlink-conntrack.h \
>> > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
>> > new file mode 100644
>> > index 000000000000..1f816feee569
>> > --- /dev/null
>> > +++ b/lib/dpif-netlink-rtnl.c
>> > @@ -0,0 +1,59 @@
>> > +/*
>> > + * Copyright (c) 2017 Red Hat, Inc.
>> > + *
>> > + * Licensed under the Apache License, Version 2.0 (the "License");
>> > + * you may not use this file except in compliance with the License.
>> > + * You may obtain a copy of the License at:
>> > + *
>> > + *     http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> > + * See the License for the specific language governing permissions and
>> > + * limitations under the License.
>> > + */
>> > +
>> > +#include <config.h>
>> > +
>> > +#include "dpif-netlink-rtnl.h"
>> > +#include "dpif-netlink.h"
>> > +
>> > +
>> > +int
>> > +dpif_netlink_rtnl_port_create(struct netdev *netdev)
>> > +{
>> > +    switch (netdev_to_ovs_vport_type(netdev_get_type(netdev))) {
>> > +    case OVS_VPORT_TYPE_VXLAN:
>> > +    case OVS_VPORT_TYPE_GRE:
>> > +    case OVS_VPORT_TYPE_GENEVE:
>> > +    case OVS_VPORT_TYPE_NETDEV:
>> > +    case OVS_VPORT_TYPE_INTERNAL:
>> > +    case OVS_VPORT_TYPE_LISP:
>> > +    case OVS_VPORT_TYPE_STT:
>> > +    case OVS_VPORT_TYPE_UNSPEC:
>> > +    case __OVS_VPORT_TYPE_MAX:
>> > +    default:
>> > +        return EOPNOTSUPP;
>> > +    }
>> > +    return 0;
>> > +}
>> > +
>> > +int
>> > +dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED, const char *type)
>> > +{
>> > +    switch (netdev_to_ovs_vport_type(type)) {
>> > +    case OVS_VPORT_TYPE_VXLAN:
>> > +    case OVS_VPORT_TYPE_GRE:
>> > +    case OVS_VPORT_TYPE_GENEVE:
>> > +    case OVS_VPORT_TYPE_NETDEV:
>> > +    case OVS_VPORT_TYPE_INTERNAL:
>> > +    case OVS_VPORT_TYPE_LISP:
>> > +    case OVS_VPORT_TYPE_STT:
>> > +    case OVS_VPORT_TYPE_UNSPEC:
>> > +    case __OVS_VPORT_TYPE_MAX:
>> > +    default:
>> > +        return EOPNOTSUPP;
>> > +    }
>> > +    return 0;
>> > +}
>> > diff --git a/lib/dpif-netlink-rtnl.h b/lib/dpif-netlink-rtnl.h
>> > new file mode 100644
>> > index 000000000000..5fef314a20f6
>> > --- /dev/null
>> > +++ b/lib/dpif-netlink-rtnl.h
>> > @@ -0,0 +1,47 @@
>> > +/*
>> > + * Copyright (c) 2017 Red Hat, Inc.
>> > + *
>> > + * Licensed under the Apache License, Version 2.0 (the "License");
>> > + * you may not use this file except in compliance with the License.
>> > + * You may obtain a copy of the License at:
>> > + *
>> > + *     http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> > + * See the License for the specific language governing permissions and
>> > + * limitations under the License.
>> > + */
>> > +
>> > +#ifndef DPIF_NETLINK_RTNL_H
>> > +#define DPIF_NETLINK_RTNL_H 1
>> > +
>> > +#include <errno.h>
>> > +
>> > +#include "netdev.h"
>> > +
>> > +/* Declare these to keep sparse happy. */
>> > +int dpif_netlink_rtnl_port_create(struct netdev *netdev);
>> > +int dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
>> > +                                   const char *type);
>> > +
>> > +#ifndef __linux__
>> > +/* Dummy implementations for non Linux builds. */
>> > +
>> > +static inline int
>> > +dpif_netlink_rtnl_port_create(struct netdev *netdev OVS_UNUSED)
>> > +{
>> > +    return EOPNOTSUPP;
>> > +}
>> > +
>> > +static inline int
>> > +dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
>> > +                               const char *type OVS_UNUSED)
>> > +{
>> > +    return EOPNOTSUPP;
>> > +}
>> > +
>> > +#endif
>> > +
>> > +#endif /* DPIF_NETLINK_RTNL_H */
>> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>> > index ee6a30ad5f10..572e365e8f49 100644
>> > --- a/lib/dpif-netlink.c
>> > +++ b/lib/dpif-netlink.c
>> > @@ -34,6 +34,7 @@
>> >
>> >  #include "bitmap.h"
>> >  #include "dpif-provider.h"
>> > +#include "dpif-netlink-rtnl.h"
>> >  #include "openvswitch/dynamic-string.h"
>> >  #include "flow.h"
>> >  #include "fat-rwlock.h"
>> > @@ -60,9 +61,6 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink);
>> >  #ifdef _WIN32
>> >  #include "wmi.h"
>> >  enum { WINDOWS = 1 };
>> > -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);
>> >  #else
>> >  enum { WINDOWS = 0 };
>> >  #endif
>> > @@ -224,6 +222,10 @@ static void dpif_netlink_vport_to_ofpbuf(const struct dpif_netlink_vport *,
>> >                                           struct ofpbuf *);
>> >  static int dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *,
>> >                                            const struct ofpbuf *);
>> > +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 dpif_netlink *
>> >  dpif_netlink_cast(const struct dpif *dpif)
>> > @@ -783,7 +785,7 @@ get_vport_type(const struct dpif_netlink_vport *vport)
>> >      return "unknown";
>> >  }
>> >
>> > -static enum ovs_vport_type
>> > +enum ovs_vport_type
>> >  netdev_to_ovs_vport_type(const char *type)
>> >  {
>> >      if (!strcmp(type, "tap") || !strcmp(type, "system")) {
>> > @@ -945,8 +947,34 @@ dpif_netlink_port_add_compat(struct dpif_netlink *dpif, struct netdev *netdev,
>> >
>> >  }
>> >
>> > +static int OVS_UNUSED
>> > +dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif,
>> > +                                      struct netdev *netdev,
>> > +                                      odp_port_t *port_nop)
>> > +    OVS_REQ_WRLOCK(dpif->upcall_lock)
>> > +{
>> > +    int error;
>> > +    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
>> > +    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>> > +    const char *name = netdev_vport_get_dpif_port(netdev,
>> > +                                                  namebuf, sizeof namebuf);
>> >
>> > +    error = dpif_netlink_rtnl_port_create(netdev);
>> > +    if (error) {
>> > +        if (error != EOPNOTSUPP) {
>> > +            VLOG_INFO_RL(&rl, "Failed to create %s with rtnetlink. error = %d",
>> > +                         netdev_get_name(netdev), error);
>> > +        }
>> > +        return error;
>> > +    }
>> >
>> > +    error = dpif_netlink_port_add__(dpif, name, OVS_VPORT_TYPE_NETDEV, NULL,
>> > +                                    port_nop);
>> > +    if (error) {
>> > +        dpif_netlink_rtnl_port_destroy(name, netdev_get_type(netdev));
>> > +    }
>> > +    return error;
>> > +}
>> >
>> >  static int
>> >  dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev,
>> > @@ -967,19 +995,23 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
>> >      OVS_REQ_WRLOCK(dpif->upcall_lock)
>> >  {
>> >      struct dpif_netlink_vport vport;
>> > +    struct dpif_port dpif_port;
>> >      int error;
>> >
>> > +    error = dpif_netlink_port_query__(dpif, port_no, NULL, &dpif_port);
>> > +    if (error) {
>> > +        return error;
>> > +    }
>> > +
>> >      dpif_netlink_vport_init(&vport);
>> >      vport.cmd = OVS_VPORT_CMD_DEL;
>> >      vport.dp_ifindex = dpif->dp_ifindex;
>> >      vport.port_no = port_no;
>> >  #ifdef _WIN32
>> > -    struct dpif_port temp_dpif_port;
>> > -    dpif_netlink_port_query__(dpif, port_no, NULL, &temp_dpif_port);
>> > -    if (!strcmp(temp_dpif_port.type, "internal")) {
>> > -        if (!delete_wmi_port(temp_dpif_port.name)){
>> > +    if (!strcmp(dpif_port.type, "internal")) {
>> > +        if (!delete_wmi_port(dpif_port.name)) {
>> >              VLOG_ERR("Could not delete wmi port with name: %s",
>> > -                     temp_dpif_port.name);
>> > +                     dpif_port.name);
>> >          };
>> >      }
>> >  #endif
>> > @@ -987,6 +1019,8 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
>> >
>> >      vport_del_channels(dpif, port_no);
>> >
>> > +    dpif_port_destroy(&dpif_port);
>> > +
>>
>> Looks like windows has a memory leak in the existing path because it
>> doesn't free the temp_dpif_port.{name,type}. Perhaps we should split
>> out this change so we can backport the fix to affected branches?
>
> Agreed. I'll submit a separate patch so we can backport it other
> releases.

Thanks!

Patch hide | download patch | download mbox

diff --git a/lib/automake.mk b/lib/automake.mk
index b266af13e4c7..73706529de5a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -352,6 +352,8 @@  if LINUX
 lib_libopenvswitch_la_SOURCES += \
 	lib/dpif-netlink.c \
 	lib/dpif-netlink.h \
+	lib/dpif-netlink-rtnl.c \
+	lib/dpif-netlink-rtnl.h \
 	lib/if-notifier.c \
 	lib/if-notifier.h \
 	lib/netdev-linux.c \
@@ -382,6 +384,7 @@  if WIN32
 lib_libopenvswitch_la_SOURCES += \
 	lib/dpif-netlink.c \
 	lib/dpif-netlink.h \
+	lib/dpif-netlink-rtnl.h \
 	lib/netdev-windows.c \
 	lib/netlink-conntrack.c \
 	lib/netlink-conntrack.h \
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
new file mode 100644
index 000000000000..1f816feee569
--- /dev/null
+++ b/lib/dpif-netlink-rtnl.c
@@ -0,0 +1,59 @@ 
+/*
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+#include "dpif-netlink-rtnl.h"
+#include "dpif-netlink.h"
+
+
+int
+dpif_netlink_rtnl_port_create(struct netdev *netdev)
+{
+    switch (netdev_to_ovs_vport_type(netdev_get_type(netdev))) {
+    case OVS_VPORT_TYPE_VXLAN:
+    case OVS_VPORT_TYPE_GRE:
+    case OVS_VPORT_TYPE_GENEVE:
+    case OVS_VPORT_TYPE_NETDEV:
+    case OVS_VPORT_TYPE_INTERNAL:
+    case OVS_VPORT_TYPE_LISP:
+    case OVS_VPORT_TYPE_STT:
+    case OVS_VPORT_TYPE_UNSPEC:
+    case __OVS_VPORT_TYPE_MAX:
+    default:
+        return EOPNOTSUPP;
+    }
+    return 0;
+}
+
+int
+dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED, const char *type)
+{
+    switch (netdev_to_ovs_vport_type(type)) {
+    case OVS_VPORT_TYPE_VXLAN:
+    case OVS_VPORT_TYPE_GRE:
+    case OVS_VPORT_TYPE_GENEVE:
+    case OVS_VPORT_TYPE_NETDEV:
+    case OVS_VPORT_TYPE_INTERNAL:
+    case OVS_VPORT_TYPE_LISP:
+    case OVS_VPORT_TYPE_STT:
+    case OVS_VPORT_TYPE_UNSPEC:
+    case __OVS_VPORT_TYPE_MAX:
+    default:
+        return EOPNOTSUPP;
+    }
+    return 0;
+}
diff --git a/lib/dpif-netlink-rtnl.h b/lib/dpif-netlink-rtnl.h
new file mode 100644
index 000000000000..5fef314a20f6
--- /dev/null
+++ b/lib/dpif-netlink-rtnl.h
@@ -0,0 +1,47 @@ 
+/*
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef DPIF_NETLINK_RTNL_H
+#define DPIF_NETLINK_RTNL_H 1
+
+#include <errno.h>
+
+#include "netdev.h"
+
+/* Declare these to keep sparse happy. */
+int dpif_netlink_rtnl_port_create(struct netdev *netdev);
+int dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
+                                   const char *type);
+
+#ifndef __linux__
+/* Dummy implementations for non Linux builds. */
+
+static inline int
+dpif_netlink_rtnl_port_create(struct netdev *netdev OVS_UNUSED)
+{
+    return EOPNOTSUPP;
+}
+
+static inline int
+dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
+                               const char *type OVS_UNUSED)
+{
+    return EOPNOTSUPP;
+}
+
+#endif
+
+#endif /* DPIF_NETLINK_RTNL_H */
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index ee6a30ad5f10..572e365e8f49 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -34,6 +34,7 @@ 
 
 #include "bitmap.h"
 #include "dpif-provider.h"
+#include "dpif-netlink-rtnl.h"
 #include "openvswitch/dynamic-string.h"
 #include "flow.h"
 #include "fat-rwlock.h"
@@ -60,9 +61,6 @@  VLOG_DEFINE_THIS_MODULE(dpif_netlink);
 #ifdef _WIN32
 #include "wmi.h"
 enum { WINDOWS = 1 };
-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);
 #else
 enum { WINDOWS = 0 };
 #endif
@@ -224,6 +222,10 @@  static void dpif_netlink_vport_to_ofpbuf(const struct dpif_netlink_vport *,
                                          struct ofpbuf *);
 static int dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *,
                                           const struct ofpbuf *);
+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 dpif_netlink *
 dpif_netlink_cast(const struct dpif *dpif)
@@ -783,7 +785,7 @@  get_vport_type(const struct dpif_netlink_vport *vport)
     return "unknown";
 }
 
-static enum ovs_vport_type
+enum ovs_vport_type
 netdev_to_ovs_vport_type(const char *type)
 {
     if (!strcmp(type, "tap") || !strcmp(type, "system")) {
@@ -945,8 +947,34 @@  dpif_netlink_port_add_compat(struct dpif_netlink *dpif, struct netdev *netdev,
 
 }
 
+static int OVS_UNUSED
+dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif,
+                                      struct netdev *netdev,
+                                      odp_port_t *port_nop)
+    OVS_REQ_WRLOCK(dpif->upcall_lock)
+{
+    int error;
+    char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+    const char *name = netdev_vport_get_dpif_port(netdev,
+                                                  namebuf, sizeof namebuf);
 
+    error = dpif_netlink_rtnl_port_create(netdev);
+    if (error) {
+        if (error != EOPNOTSUPP) {
+            VLOG_INFO_RL(&rl, "Failed to create %s with rtnetlink. error = %d",
+                         netdev_get_name(netdev), error);
+        }
+        return error;
+    }
 
+    error = dpif_netlink_port_add__(dpif, name, OVS_VPORT_TYPE_NETDEV, NULL,
+                                    port_nop);
+    if (error) {
+        dpif_netlink_rtnl_port_destroy(name, netdev_get_type(netdev));
+    }
+    return error;
+}
 
 static int
 dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev,
@@ -967,19 +995,23 @@  dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
     OVS_REQ_WRLOCK(dpif->upcall_lock)
 {
     struct dpif_netlink_vport vport;
+    struct dpif_port dpif_port;
     int error;
 
+    error = dpif_netlink_port_query__(dpif, port_no, NULL, &dpif_port);
+    if (error) {
+        return error;
+    }
+
     dpif_netlink_vport_init(&vport);
     vport.cmd = OVS_VPORT_CMD_DEL;
     vport.dp_ifindex = dpif->dp_ifindex;
     vport.port_no = port_no;
 #ifdef _WIN32
-    struct dpif_port temp_dpif_port;
-    dpif_netlink_port_query__(dpif, port_no, NULL, &temp_dpif_port);
-    if (!strcmp(temp_dpif_port.type, "internal")) {
-        if (!delete_wmi_port(temp_dpif_port.name)){
+    if (!strcmp(dpif_port.type, "internal")) {
+        if (!delete_wmi_port(dpif_port.name)) {
             VLOG_ERR("Could not delete wmi port with name: %s",
-                     temp_dpif_port.name);
+                     dpif_port.name);
         };
     }
 #endif
@@ -987,6 +1019,8 @@  dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no)
 
     vport_del_channels(dpif, port_no);
 
+    dpif_port_destroy(&dpif_port);
+
     return error;
 }
 
diff --git a/lib/dpif-netlink.h b/lib/dpif-netlink.h
index 6d1505713a72..568b81441ddc 100644
--- a/lib/dpif-netlink.h
+++ b/lib/dpif-netlink.h
@@ -58,4 +58,6 @@  int dpif_netlink_vport_get(const char *name, struct dpif_netlink_vport *reply,
 
 bool dpif_netlink_is_internal_device(const char *name);
 
+enum ovs_vport_type netdev_to_ovs_vport_type(const char *type);
+
 #endif /* dpif-netlink.h */