From 9311294d9c7faf1caf91c27a8211370555ba49a2 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 9 Feb 2016 19:22:14 +0000 Subject: [PATCH] "refactor" veth creation Invoke the new create_veth from a few places, eliminating code duplication and inconsistencies. Semantic differences: - init_bridged_fastdp now skips the veth creation/config if the veth already exists. This eliminates routing blips. Though in reality something very strange must have happened to end up in this code with an existing veth. - init_bridged_fastdp no longer deletes the $BRIDGE and $DATAPATH when veth creation/configuration fails. This is consistent with init_bridge, which does not delete $BRIDGE when something goes wrong. And it is generally desirable since the init_* functions are invoked on potentially existing bridges - e.g. following 'weave stop'. - attach_bridge and setup_router_iface_bridge used to mark one veth end 'up', then invoke add_iface_* for it, and then mark the other end 'up'. create_veth marks both ends 'up' first. This makes no practical difference in these cases. notable other differences: - we detect veth presence by checking whether *both* ends are present, rather than just one. We deliberately continue to (and fail in) the veth creation if one end already exists. This can only happen when we've stumbled across a different interface, since a veth end cannot exist independently. So it is right to treat this as an error. - init_bridged_fastdp invokes add_iface_bridge instead of duplicating what that does. Also makes for nice coherence with the add_iface_fastdp it is invoking too. --- weave | 69 +++++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/weave b/weave index 7ef866ca..f877a78b 100755 --- a/weave +++ b/weave @@ -576,6 +576,24 @@ EOF configure_arp_cache $BRIDGE } +# create veth with ends $1-$2, and then invoke $3..., removing the +# veth on failure. No-op of veth already exists. +create_veth() { + VETHL=$1 + VETHR=$2 + shift 2 + + ip link show $VETHL >/dev/null 2>&1 && ip link show $VETHR >/dev/null 2>&1 && return 0 + + ip link add name $VETHL mtu $MTU type veth peer name $VETHR mtu $MTU + + if ! ip link set $VETHL up || ! ip link set $VETHR up || ! "$@" ; then + ip link del $VETHL >/dev/null 2>&1 || true + ip link del $VETHR >/dev/null 2>&1 || true + return 1 + fi +} + init_fastdp() { # GCE has the lowest underlay network MTU we're likely to encounter on # a local network, at 1460 bytes. To get the overlay MTU from that we @@ -624,32 +642,21 @@ init_bridged_fastdp() { # Initialise the datapath as normal. NB sets MTU for use below init_fastdp - # Create linking veth pair. We do this before initialising the bridge - # so that `ip link show` displays the datapath, linking veths and - # the bridge in natural order - ip link del $DATAPATH_IFNAME >/dev/null 2>&1 || true - ip link del $BRIDGE_IFNAME >/dev/null 2>&1 || true - ip link add name $BRIDGE_IFNAME mtu $MTU type veth peer name $DATAPATH_IFNAME mtu $MTU || return 1 - # Initialise the bridge using fast datapath MTU init_bridge - # Link intermediary bridge and datapath - if ! ip link set $DATAPATH_IFNAME up || - ! ip link set $BRIDGE_IFNAME up || - ! add_iface_fastdp $DATAPATH_IFNAME || ! ip link set $BRIDGE_IFNAME master $BRIDGE ; then - # Failed to link bridge and datapath - clean up - ip link del $BRIDGE >/dev/null 2>&1 || true - ip link del $DATAPATH_IFNAME >/dev/null 2>&1 || true - ip link del $BRIDGE_IFNAME >/dev/null 2>&1 || true - util_op delete-datapath $DATAPATH >/dev/null 2>&1 || true - return 1 - fi + # Create linking veth pair + create_veth $BRIDGE_IFNAME $DATAPATH_IFNAME configure_veth_bridged_fastdp # Finally, bring the datapath up ip link set dev $DATAPATH up } +configure_veth_bridged_fastdp() { + add_iface_fastdp $DATAPATH_IFNAME + add_iface_bridge $BRIDGE_IFNAME +} + ethtool_tx_off_fastdp() { true } @@ -802,16 +809,12 @@ attach_bridge() { LOCAL_IFNAME=v${CONTAINER_IFNAME}bl$bridge GUEST_IFNAME=v${CONTAINER_IFNAME}bg$bridge - ip link show $GUEST_IFNAME >/dev/null 2>&1 && return 0 - if ! ip link add name $LOCAL_IFNAME mtu $MTU type veth peer name $GUEST_IFNAME mtu $MTU || - ! ip link set $LOCAL_IFNAME up || - ! add_iface_$BRIDGE_TYPE $LOCAL_IFNAME || - ! ip link set $GUEST_IFNAME up - ! ip link set $GUEST_IFNAME master $bridge ; then - ip link del $GUEST_IFNAME >/dev/null 2>&1 || true - ip link del $LOCAL_IFNAME >/dev/null 2>&1 || true - return 1 - fi + create_veth $LOCAL_IFNAME $GUEST_IFNAME configure_veth_attached_bridge +} + +configure_veth_attached_bridge() { + add_iface_$BRIDGE_TYPE $LOCAL_IFNAME + ip link set $GUEST_IFNAME master $bridge } router_opts_fastdp() { @@ -844,15 +847,7 @@ setup_router_iface_fastdp() { } setup_router_iface_bridge() { - ip link show $PCAP_IFNAME >/dev/null 2>&1 && return 0 - if ! ip link add name $BRIDGE_IFNAME mtu $MTU type veth peer name $PCAP_IFNAME mtu $MTU || - ! ip link set $BRIDGE_IFNAME up || - ! add_iface_bridge $BRIDGE_IFNAME || - ! ip link set $PCAP_IFNAME up ; then - ip link del $PCAP_IFNAME >/dev/null 2>&1 || true - ip link del $BRIDGE_IFNAME >/dev/null 2>&1 || true - return 1 - fi + create_veth $BRIDGE_IFNAME $PCAP_IFNAME add_iface_bridge $BRIDGE_IFNAME } setup_router_iface_bridged_fastdp() {