On 28 March 2017 at 15:44, Eric Garver <[email protected]> wrote: > On Mon, Mar 27, 2017 at 06:07:39PM -0700, Joe Stringer wrote: >> On 16 March 2017 at 15:10, Eric Garver <[email protected]> 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 <[email protected]> >> > Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]> >> > Signed-off-by: Eric Garver <[email protected]> >> > --- >> > 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! _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
