On Tue, May 16, 2017 at 06:16:03PM -0700, Joe Stringer wrote:
> On 7 May 2017 at 06:43, 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 <casca...@redhat.com>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@redhat.com>
> > Signed-off-by: Eric Garver <e...@erig.me>
> > ---
> > lib/automake.mk | 3 +
> > lib/dpif-netlink-rtnl.c | 227
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/dpif-netlink-rtnl.h | 47 ++++++++++
> > lib/dpif-netlink.c | 29 ++++++-
> > lib/dpif-netlink.h | 2 +
> > 5 files changed, 307 insertions(+), 1 deletion(-)
> > 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 faace7993e79..f5baba27179f 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..906e05145190
> > --- /dev/null
> > +++ b/lib/dpif-netlink-rtnl.c
> > @@ -0,0 +1,227 @@
> > +/*
> > + * 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 <net/if.h>
> > +#include <linux/ip.h>
> > +#include <linux/rtnetlink.h>
> > +
> > +#include "dpif-netlink.h"
> > +#include "netdev-vport.h"
> > +#include "netlink-socket.h"
> > +
> > +static const struct nl_policy rtlink_policy[] = {
> > + [IFLA_LINKINFO] = { .type = NL_A_NESTED },
> > +};
> > +static const struct nl_policy linkinfo_policy[] = {
> > + [IFLA_INFO_KIND] = { .type = NL_A_STRING },
> > + [IFLA_INFO_DATA] = { .type = NL_A_NESTED },
> > +};
> > +
> > +static int
> > +rtnl_transact(uint32_t type, uint32_t flags, const char *name,
> > + struct ofpbuf **reply)
> > +{
> > + struct ofpbuf request;
> > + int err;
> > +
> > + ofpbuf_init(&request, 0);
> > + nl_msg_put_nlmsghdr(&request, 0, type, flags);
> > + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg));
> > + nl_msg_put_string(&request, IFLA_IFNAME, name);
> > +
> > + err = nl_transact(NETLINK_ROUTE, &request, reply);
> > + ofpbuf_uninit(&request);
> > +
> > + return err;
> > +}
> > +
> > +static int
> > +dpif_netlink_rtnl_destroy(const char *name)
> > +{
> > + return rtnl_transact(RTM_DELLINK, NLM_F_REQUEST | NLM_F_ACK, name,
> > NULL);
> > +}
> > +
> > +static int OVS_UNUSED
> > +dpif_netlink_rtnl_getlink(const char *name, struct ofpbuf **reply)
> > +{
> > + return rtnl_transact(RTM_GETLINK, NLM_F_REQUEST, name, reply);
> > +}
> > +
> > +static int OVS_UNUSED
> > +rtnl_policy_parse(const char *kind, struct ofpbuf *reply,
> > + const struct nl_policy *policy,
> > + struct nlattr *tnl_info[],
> > + size_t policy_size)
> > +{
> > + struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
> > + struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
> > + struct ifinfomsg *ifmsg;
> > + int error = 0;
> > +
> > + ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg);
> > + if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg,
> > + rtlink_policy, rtlink, ARRAY_SIZE(rtlink_policy))
> > + || !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy,
> > + linkinfo, ARRAY_SIZE(linkinfo_policy))
> > + || strcmp(nl_attr_get_string(linkinfo[IFLA_INFO_KIND]), kind)
> > + || !nl_parse_nested(linkinfo[IFLA_INFO_DATA], policy,
> > + tnl_info, policy_size)) {
> > + error = EINVAL;
> > + }
> > +
> > + return error;
> > +}
> > +
> > +static int
> > +dpif_netlink_rtnl_verify(const struct netdev_tunnel_config OVS_UNUSED
> > *tnl_cfg,
> > + enum ovs_vport_type type, const char OVS_UNUSED
> > *name)
> > +{
> > + switch (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;
> > +}
> > +
> > +static int OVS_UNUSED
> > +dpif_netlink_rtnl_create(const struct netdev_tunnel_config OVS_UNUSED
> > *tnl_cfg,
> > + const char *name, enum ovs_vport_type type,
> > + const char *kind, uint32_t flags)
> > +{
> > + size_t linkinfo_off, infodata_off;
> > + struct ifinfomsg *ifinfo;
> > + struct ofpbuf request;
> > + int err;
> > +
> > + ofpbuf_init(&request, 0);
> > + nl_msg_put_nlmsghdr(&request, 0, RTM_NEWLINK, flags);
> > + ifinfo = ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg));
> > + ifinfo->ifi_change = ifinfo->ifi_flags = IFF_UP;
> > + nl_msg_put_string(&request, IFLA_IFNAME, name);
> > + nl_msg_put_u32(&request, IFLA_MTU, UINT16_MAX);
> > + linkinfo_off = nl_msg_start_nested(&request, IFLA_LINKINFO);
> > + nl_msg_put_string(&request, IFLA_INFO_KIND, kind);
> > + infodata_off = nl_msg_start_nested(&request, IFLA_INFO_DATA);
> > +
> > + /* tunnel unique info */
> > + switch (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:
> > + err = EOPNOTSUPP;
> > + goto exit;
> > + }
> > +
> > + nl_msg_end_nested(&request, infodata_off);
> > + nl_msg_end_nested(&request, linkinfo_off);
> > +
> > + err = nl_transact(NETLINK_ROUTE, &request, NULL);
> > +
> > +exit:
> > + ofpbuf_uninit(&request);
> > +
> > + return err;
> > +}
> > +
> > +int
> > +dpif_netlink_rtnl_port_create(struct netdev *netdev)
> > +{
> > + const struct netdev_tunnel_config *tnl_cfg;
> > + char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> > + enum ovs_vport_type type;
> > + bool retried = false;
> > + const char *name;
> > + uint32_t flags;
> > + int err;
> > +
> > + tnl_cfg = netdev_get_tunnel_config(netdev);
> > + if (!tnl_cfg) {
> > + return EINVAL;
> > + }
> > +
> > + type = netdev_to_ovs_vport_type(netdev_get_type(netdev));
> > + name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
> > + flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
> > +
> > +try_again:
> > + switch (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:
> > + err = EOPNOTSUPP;
> > + }
> > +
> > + if (!err || (err == EEXIST && !retried)) {
> > + int err2 = dpif_netlink_rtnl_verify(tnl_cfg, type, name);
> > + if (err2 && err == EEXIST) {
> > + err2 = dpif_netlink_rtnl_destroy(name);
> > + if (!err2) {
> > + retried = true;
> > + goto try_again;
> > + }
> > + }
> > + err = err2;
> > + }
>
> I found the above a bit tricky to follow: combines error and success
> cases, backwards loops with an extra bool, etc. I wonder if it's
> actually easier to grok if it's just written out longhand. Here's a
> suggested diff on top of the series that might achieve this (with a
> couple of extra bits of logging), do you think it improves the patch?
I initially wrote it long hand style, but thought it a bit repetitive.
I'll send another revision after making this changes and looking into
the GENEVE issue.
Thanks.
Eric.
>
> ---
> lib/dpif-netlink-rtnl.c | 91
> +++++++++++++++++++++++++++++++++----------------
> 1 file changed, 62 insertions(+), 29 deletions(-)
>
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 2669750a65bf..a9d5178cd575 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -25,6 +25,9 @@
> #include "dpif-netlink.h"
> #include "netdev-vport.h"
> #include "netlink-socket.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
>
> /* On some older systems, these enums are not defined. */
> #ifndef IFLA_VXLAN_MAX
> @@ -304,13 +307,36 @@ exit:
> return err;
> }
>
> +static const char *
> +vport_type_to_kind(enum ovs_vport_type type)
> +{
> + switch (type) {
> + case OVS_VPORT_TYPE_VXLAN:
> + return "vxlan";
> + case OVS_VPORT_TYPE_GRE:
> + return "gretap";
> + case OVS_VPORT_TYPE_GENEVE:
> + return "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:
> + break;
> + }
> +
> + return NULL;
> +}
> +
> int
> dpif_netlink_rtnl_port_create(struct netdev *netdev)
> {
> const struct netdev_tunnel_config *tnl_cfg;
> char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> enum ovs_vport_type type;
> - bool retried = false;
> + const char *kind;
> const char *name;
> uint32_t flags;
> int err;
> @@ -321,40 +347,47 @@ dpif_netlink_rtnl_port_create(struct netdev *netdev)
> }
>
> type = netdev_to_ovs_vport_type(netdev_get_type(netdev));
> + kind = vport_type_to_kind(type);
> + if (!kind) {
> + return EOPNOTSUPP;
> + }
> +
> name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
> flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
>
> -try_again:
> - switch (type) {
> - case OVS_VPORT_TYPE_VXLAN:
> - err = dpif_netlink_rtnl_create(tnl_cfg, name, type, "vxlan", flags);
> - break;
> - case OVS_VPORT_TYPE_GRE:
> - err = dpif_netlink_rtnl_create(tnl_cfg, name, type, "gretap", flags);
> - break;
> - case OVS_VPORT_TYPE_GENEVE:
> - err = dpif_netlink_rtnl_create(tnl_cfg, name, type, "geneve", flags);
> - break;
> - 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:
> - err = EOPNOTSUPP;
> - }
> + err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
>
> - if (!err || (err == EEXIST && !retried)) {
> - int err2 = dpif_netlink_rtnl_verify(tnl_cfg, type, name);
> - if (err2 && err == EEXIST) {
> - err2 = dpif_netlink_rtnl_destroy(name);
> - if (!err2) {
> - retried = true;
> - goto try_again;
> + /* If the device exists, validate and/or attempt to recreate it. */
> + if (err == EEXIST) {
> + err = dpif_netlink_rtnl_verify(tnl_cfg, type, name);
> + if (!err) {
> + return 0;
> + } else {
> + err = dpif_netlink_rtnl_destroy(name);
> + if (err) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
> 5);
> +
> + VLOG_WARN_RL(&rl, "RTNL device %s exists and cannot be "
> + "deleted: %s", name, ovs_strerror(err));
> + return err;
> }
> + err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
> + }
> + }
> + if (err) {
> + return err;
> + }
> +
> + err = dpif_netlink_rtnl_verify(tnl_cfg, type, name);
> + if (err) {
> + int err2 = dpif_netlink_rtnl_destroy(name);
> +
> + if (err2) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +
> + VLOG_WARN_RL(&rl, "Failed to delete device %s during rtnl port "
> + "creation: %s", name, ovs_strerror(err2));
> }
> - err = err2;
> }
>
> return err;
> --
> 2.11.1
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
