On Mon, Mar 18, 2013 at 8:32 AM, Gurucharan Shetty <shet...@nicira.com> wrote:
> When we configure OVS using rhel ifupdown scripts,
> we call ifup on a bridge twice. Once while configuring the
> bridge and once while configuring the ports of the bridge.
> This looks harmless but unnecessary. This patch fixes the
> behavior.
>
> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com>
I have tested this patch on rhel6.1 and centos6.4 and it works fine for me.
Denis tried it out too and it apparently does not work for him. I am
not very sure
why that is the case. Till I get a more detailed bug report, it
probably makes sense
to review this as-is. ( I have folded Ben's changes for STP in this
patch and also removed
timeouts from ovs-vsctl).
> ---
> rhel/README.RHEL | 6 +++-
> rhel/etc_sysconfig_network-scripts_ifup-ovs | 46
> +++++++++++++++++++++------
> 2 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/rhel/README.RHEL b/rhel/README.RHEL
> index d9b68e4..ba2774a 100644
> --- a/rhel/README.RHEL
> +++ b/rhel/README.RHEL
> @@ -42,11 +42,15 @@ assignments. The following OVS-specific variable names
> are supported:
> Note
> ----
>
> -"ifdown" on a bridge will not bring individual ports on the bridge
> +* "ifdown" on a bridge will not bring individual ports on the bridge
> down. "ifup" on a bridge will not add ports to the bridge. This
> behavior should be compatible with standard bridges (with
> TYPE=Bridge).
>
> +* If 'ifup' on an interface is called multiple times, one can see
> +"RTNETLINK answers: File exists" printed on the console. This comes from
> +ifup-eth trying to add zeroconf route multiple times and is harmless.
> +
> Examples
> --------
>
> diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs
> b/rhel/etc_sysconfig_network-scripts_ifup-ovs
> index ae095a0..52e8b3c 100755
> --- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
> +++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
> @@ -34,7 +34,7 @@ if [ ! -x ${OTHERSCRIPT} ]; then
> OTHERSCRIPT="/etc/sysconfig/network-scripts/ifup-eth"
> fi
>
> -check_recursion()
> +check_recursion ()
> {
> [ -n "${UPPEDSTACK}" ] && for _r in ${UPPEDSTACK}; do
> [ "$_r" = "$1" ] && return 1
> @@ -43,6 +43,13 @@ check_recursion()
> return 0
> }
>
> +ifup_ovs_bridge ()
> +{
> + if ovs-vsctl br-exists "${OVS_BRIDGE}"; then :; else
> + /sbin/ifup "${OVS_BRIDGE}"
> + fi
> +}
> +
> if [ -z "${UPPEDSTACK}" ]; then
> UPPEDSTACK="${DEVICE}"
> fi
> @@ -57,7 +64,23 @@ done
>
> case "$TYPE" in
> OVSBridge)
> - ovs-vsctl -t ${TIMEOUT} -- --may-exist add-br "$DEVICE"
> $OVS_OPTIONS ${OVS_EXTRA+-- $OVS_EXTRA}
> + # If bridge already exists and is up, it has been configured
> through
> + # other cases like OVSPort, OVSIntPort and OVSBond. If it is
> down or
> + # it does not exist, create it. It is possible for a bridge
> to exist
> + # because it remained in the OVSDB for some reason, but it
> won't be up.
> + if check_device_down "${DEVICE}"; then
> + ovs-vsctl --may-exist add-br "$DEVICE" $OVS_OPTIONS \
> + ${OVS_EXTRA+-- $OVS_EXTRA} \
> + ${STP+-- set bridge "$DEVICE" stp_enable="${STP}"}
> + else
> + OVSBRIDGECONFIGURED="yes"
> + fi
> +
> + # When dhcp is enabled, the assumption is that there will be
> a port to
> + # attach (otherwise, we can't reach out for dhcp). So, we do
> not
> + # configure the bridge through rhel's ifup infrastructure
> unless
> + # it is being configured after the port has been configured.
> + # The "OVSINTF" is set only after the port is configured.
> if [ "${OVSBOOTPROTO}" = "dhcp" ] && [ -n "${OVSINTF}" ]; then
> case " ${OVSDHCPINTERFACES} " in
> *" ${OVSINTF} "*)
> @@ -65,29 +88,32 @@ case "$TYPE" in
> ;;
> esac
> fi
> - if [ "${OVSBOOTPROTO}" != "dhcp" ] && [ -z "${OVSINTF}" ];
> then
> +
> + # When dhcp is not enabled, it is possible that someone may
> want
> + # a standalone bridge (i.e it may not have any ports).
> Configure it.
> + if [ "${OVSBOOTPROTO}" != "dhcp" ] && [ -z "${OVSINTF}" ] && \
> + [ "${OVSBRIDGECONFIGURED}" != "yes" ]; then
> ${OTHERSCRIPT} ${CONFIG}
> fi
> - [ -n "${STP}" ] && ovs-vsctl --no-wait set bridge "${DEVICE}"
> stp_enable="${STP}"
> exit 0
> ;;
> OVSPort)
> - /sbin/ifup "$OVS_BRIDGE"
> + ifup_ovs_bridge
> ${OTHERSCRIPT} ${CONFIG} ${2}
> - ovs-vsctl -t ${TIMEOUT} -- --may-exist add-port "$OVS_BRIDGE"
> "$DEVICE" $OVS_OPTIONS ${OVS_EXTRA+-- $OVS_EXTRA}
> + ovs-vsctl --may-exist add-port "$OVS_BRIDGE" "$DEVICE"
> $OVS_OPTIONS ${OVS_EXTRA+-- $OVS_EXTRA}
> OVSINTF=${DEVICE} /sbin/ifup "$OVS_BRIDGE"
> ;;
> OVSIntPort)
> - /sbin/ifup "$OVS_BRIDGE"
> - ovs-vsctl -t ${TIMEOUT} -- --may-exist add-port "$OVS_BRIDGE"
> "$DEVICE" $OVS_OPTIONS -- set Interface "$DEVICE" type=internal
> ${OVS_EXTRA+-- $OVS_EXTRA}
> + ifup_ovs_bridge
> + ovs-vsctl --may-exist add-port "$OVS_BRIDGE" "$DEVICE"
> $OVS_OPTIONS -- set Interface "$DEVICE" type=internal ${OVS_EXTRA+--
> $OVS_EXTRA}
> ${OTHERSCRIPT} ${CONFIG} ${2}
> ;;
> OVSBond)
> - /sbin/ifup "$OVS_BRIDGE"
> + ifup_ovs_bridge
> for _iface in $BOND_IFACES; do
> /sbin/ifup ${_iface}
> done
> - ovs-vsctl -t ${TIMEOUT} -- --fake-iface add-bond
> "$OVS_BRIDGE" "$DEVICE" ${BOND_IFACES} $OVS_OPTIONS ${OVS_EXTRA+-- $OVS_EXTRA}
> + ovs-vsctl --fake-iface add-bond "$OVS_BRIDGE" "$DEVICE"
> ${BOND_IFACES} $OVS_OPTIONS ${OVS_EXTRA+-- $OVS_EXTRA}
> ${OTHERSCRIPT} ${CONFIG} ${2}
> OVSINTF=${DEVICE} /sbin/ifup "$OVS_BRIDGE"
> ;;
> --
> 1.7.9.5
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
