From 41ca1e0afb4b261a217c9fdaf672ef606e8434f9 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Thu, 1 May 2014 10:53:48 -0700 Subject: [PATCH] netdev-vport: Checks tunnel status change when route-table is reset. Commit 3e912ffcbb (netdev: Add 'change_seq' back to netdev.) added per- netdev change number for indicating status change. Future commits used this change number to optimize the netdev status update to database. However, the work also introduced the bug in the following scenario: - assume interface eth0 has address 1.2.3.4, eth1 has adddress 10.0.0.1. - assume tunnel port p1 is set with remote_ip=10.0.0.5. - after setup, 'ovs-vsctl list interface p1 status' should show the 'tunnel_egress_iface="eth1"'. - now if the address of eth1 is change to 0 via 'ifconfig eth1 0'. - expectedly, after change, 'ovs-vsctl list interface p1 status' should show the 'tunnel_egress_iface="eth0"' However, 'tunnel_egress_iface' will not be updated on current master. This is in that, the 'netdev-vport' module corresponding to p1 does not react to routing related changes. To fix the bug, this commit adds a change sequence number in the route- table module and makes netdev-vport check the sequence number for tunnel status update. Bug #1240626 Signed-off-by: Alex Wang Acked-by: Ben Pfaff --- lib/netdev-provider.h | 1 + lib/netdev-vport.c | 96 +++++++++++++++++++++++++++++++++++++----- lib/netdev-vport.h | 1 + lib/netdev.c | 35 +++++++++++++++ lib/route-table-bsd.c | 6 +++ lib/route-table-stub.c | 8 +++- lib/route-table.c | 17 +++++++- lib/route-table.h | 3 +- 8 files changed, 154 insertions(+), 13 deletions(-) diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index 9d8e67f3a..c2123dc67 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -71,6 +71,7 @@ const char *netdev_get_name(const struct netdev *); struct netdev *netdev_from_name(const char *name); void netdev_get_devices(const struct netdev_class *, struct shash *device_list); +struct netdev **netdev_get_vports(size_t *size); /* A data structure for capturing packets received by a network device. * diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 9e92a8828..c214bf713 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -34,6 +34,7 @@ #include "netdev-provider.h" #include "ofpbuf.h" #include "packets.h" +#include "poll-loop.h" #include "route-table.h" #include "shash.h" #include "socket-util.h" @@ -57,6 +58,8 @@ struct netdev_vport { /* Tunnels. */ struct netdev_tunnel_config tnl_cfg; + char egress_iface[IFNAMSIZ]; + bool carrier_status; /* Patch Ports. */ char *peer; @@ -67,9 +70,13 @@ struct vport_class { struct netdev_class netdev_class; }; +/* Last read of the route-table's change number. */ +static uint64_t rt_change_seqno; + static int netdev_vport_construct(struct netdev *); static int get_patch_config(const struct netdev *netdev, struct smap *args); static int get_tunnel_config(const struct netdev *, struct smap *args); +static bool tunnel_check_status_change__(struct netdev_vport *); static bool is_vport_class(const struct netdev_class *class) @@ -77,6 +84,12 @@ is_vport_class(const struct netdev_class *class) return class->construct == netdev_vport_construct; } +bool +netdev_vport_is_vport_class(const struct netdev_class *class) +{ + return is_vport_class(class); +} + static const struct vport_class * vport_class_cast(const struct netdev_class *class) { @@ -164,6 +177,34 @@ netdev_vport_get_dpif_port_strdup(const struct netdev *netdev) sizeof namebuf)); } +/* Whenever the route-table change number is incremented, + * netdev_vport_route_changed() should be called to update + * the corresponding tunnel interface status. */ +static void +netdev_vport_route_changed(void) +{ + struct netdev **vports; + size_t i, n_vports; + + vports = netdev_get_vports(&n_vports); + for (i = 0; i < n_vports; i++) { + struct netdev *netdev_ = vports[i]; + struct netdev_vport *netdev = netdev_vport_cast(netdev_); + + ovs_mutex_lock(&netdev->mutex); + /* Finds all tunnel vports. */ + if (netdev->tnl_cfg.ip_dst) { + if (tunnel_check_status_change__(netdev)) { + netdev_change_seq_changed(netdev_); + } + } + netdev_close(netdev_); + ovs_mutex_unlock(&netdev->mutex); + } + + free(vports); +} + static struct netdev * netdev_vport_alloc(void) { @@ -228,29 +269,50 @@ netdev_vport_get_etheraddr(const struct netdev *netdev_, return 0; } -static int -tunnel_get_status(const struct netdev *netdev_, struct smap *smap) +/* Checks if the tunnel status has changed and returns a boolean. + * Updates the tunnel status if it has changed. */ +static bool +tunnel_check_status_change__(struct netdev_vport *netdev) + OVS_REQUIRES(netdev->mutex) { - struct netdev_vport *netdev = netdev_vport_cast(netdev_); char iface[IFNAMSIZ]; + bool status = false; ovs_be32 route; - ovs_mutex_lock(&netdev->mutex); + iface[0] = '\0'; route = netdev->tnl_cfg.ip_dst; - ovs_mutex_unlock(&netdev->mutex); - if (route_table_get_name(route, iface)) { struct netdev *egress_netdev; - smap_add(smap, "tunnel_egress_iface", iface); - if (!netdev_open(iface, "system", &egress_netdev)) { - smap_add(smap, "tunnel_egress_iface_carrier", - netdev_get_carrier(egress_netdev) ? "up" : "down"); + status = netdev_get_carrier(egress_netdev); netdev_close(egress_netdev); } } + if (strcmp(netdev->egress_iface, iface) + || netdev->carrier_status != status) { + ovs_strlcpy(netdev->egress_iface, iface, IFNAMSIZ); + netdev->carrier_status = status; + + return true; + } + + return false; +} + +static int +tunnel_get_status(const struct netdev *netdev_, struct smap *smap) +{ + struct netdev_vport *netdev = netdev_vport_cast(netdev_); + + if (netdev->egress_iface[0]) { + smap_add(smap, "tunnel_egress_iface", netdev->egress_iface); + + smap_add(smap, "tunnel_egress_iface_carrier", + netdev->carrier_status ? "up" : "down"); + } + return 0; } @@ -271,13 +333,26 @@ netdev_vport_update_flags(struct netdev *netdev OVS_UNUSED, static void netdev_vport_run(void) { + uint64_t seq; + route_table_run(); + seq = route_table_get_change_seq(); + if (rt_change_seqno != seq) { + rt_change_seqno = seq; + netdev_vport_route_changed(); + } } static void netdev_vport_wait(void) { + uint64_t seq; + route_table_wait(); + seq = route_table_get_change_seq(); + if (rt_change_seqno != seq) { + poll_immediate_wake(); + } } /* Code specific to tunnel types. */ @@ -484,6 +559,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) ovs_mutex_lock(&dev->mutex); dev->tnl_cfg = tnl_cfg; + tunnel_check_status_change__(dev); netdev_change_seq_changed(dev_); ovs_mutex_unlock(&dev->mutex); diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h index 833fee892..d79ef98ae 100644 --- a/lib/netdev-vport.h +++ b/lib/netdev-vport.h @@ -39,6 +39,7 @@ void netdev_vport_inc_rx(const struct netdev *, void netdev_vport_inc_tx(const struct netdev *, const struct dpif_flow_stats *); +bool netdev_vport_is_vport_class(const struct netdev_class *); const char *netdev_vport_class_get_dpif_port(const struct netdev_class *); #ifndef _WIN32 diff --git a/lib/netdev.c b/lib/netdev.c index 2fc183471..9aaf10757 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -1613,6 +1613,41 @@ netdev_get_devices(const struct netdev_class *netdev_class, ovs_mutex_unlock(&netdev_mutex); } +/* Extracts pointers to all 'netdev-vports' into an array 'vports' + * and returns it. Stores the size of the array into '*size'. + * + * The caller is responsible for freeing 'vports' and must close + * each 'netdev-vport' in the list. */ +struct netdev ** +netdev_get_vports(size_t *size) + OVS_EXCLUDED(netdev_mutex) +{ + struct netdev **vports; + struct shash_node *node; + size_t n = 0; + + if (!size) { + return NULL; + } + + /* Explicitly allocates big enough chunk of memory. */ + vports = xmalloc(shash_count(&netdev_shash) * sizeof *vports); + ovs_mutex_lock(&netdev_mutex); + SHASH_FOR_EACH (node, &netdev_shash) { + struct netdev *dev = node->data; + + if (netdev_vport_is_vport_class(dev->netdev_class)) { + dev->ref_cnt++; + vports[n] = dev; + n++; + } + } + ovs_mutex_unlock(&netdev_mutex); + *size = n; + + return vports; +} + const char * netdev_get_type_from_name(const char *name) { diff --git a/lib/route-table-bsd.c b/lib/route-table-bsd.c index 38cd2c94a..4cdf2acda 100644 --- a/lib/route-table-bsd.c +++ b/lib/route-table-bsd.c @@ -109,6 +109,12 @@ route_table_get_name(ovs_be32 ip, char name[IFNAMSIZ]) return false; } +uint64_t +route_table_get_change_seq(void) +{ + return 0; +} + void route_table_register(void) { diff --git a/lib/route-table-stub.c b/lib/route-table-stub.c index a4a099171..9acc81c80 100644 --- a/lib/route-table-stub.c +++ b/lib/route-table-stub.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2012 Nicira, Inc. +/* Copyright (c) 2012, 2013, 2014 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,12 @@ route_table_get_ifindex(ovs_be32 ip OVS_UNUSED, int *ifindex) return false; } +uint64_t +route_table_get_change_seq(void) +{ + return 0; +} + void route_table_register(void) { diff --git a/lib/route-table.c b/lib/route-table.c index 8f5b733ce..2986d3d9a 100644 --- a/lib/route-table.c +++ b/lib/route-table.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2012, 2013 Nicira, Inc. + * Copyright (c) 2011, 2012, 2013, 2014 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -65,6 +65,10 @@ struct name_node { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); +/* Global change number for route-table, which should be incremented + * every time route_table_reset() is called. */ +static uint64_t rt_change_seq; + static unsigned int register_count = 0; static struct nln *nln = NULL; static struct route_table_msg rtmsg; @@ -154,6 +158,12 @@ route_table_get_ifindex(ovs_be32 ip_, int *ifindex) return false; } +uint64_t +route_table_get_change_seq(void) +{ + return rt_change_seq; +} + /* Users of the route_table module should register themselves with this * function before making any other route_table function calls. */ void @@ -205,6 +215,10 @@ route_table_run(void) if (nln) { rtnetlink_link_run(); nln_run(nln); + + if (!route_table_valid) { + route_table_reset(); + } } } @@ -228,6 +242,7 @@ route_table_reset(void) route_map_clear(); route_table_valid = true; + rt_change_seq++; ofpbuf_init(&request, 0); diff --git a/lib/route-table.h b/lib/route-table.h index 6403b6df3..2ed4b9143 100644 --- a/lib/route-table.h +++ b/lib/route-table.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 Nicira, Inc. + * Copyright (c) 2011, 2012, 2013, 2014 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ bool route_table_get_ifindex(ovs_be32 ip, int *ifindex); bool route_table_get_name(ovs_be32 ip, char name[IFNAMSIZ]); +uint64_t route_table_get_change_seq(void); void route_table_register(void); void route_table_unregister(void); void route_table_run(void); -- 2.43.0