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

Eric Garver e at erig.me
Tue Mar 28 22:44:06 UTC 2017


On Mon, Mar 27, 2017 at 06:07:39PM -0700, Joe Stringer wrote:
> On 16 March 2017 at 15:10, Eric Garver <e at 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 at redhat.com>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
> > Signed-off-by: Eric Garver <e at 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.


More information about the dev mailing list