bonding: Major rewrite of the hook

The bonding code now uses ip instead of writing to /sys
and the hook has been cleaned up, improved, tested and
received minor fixes.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>

diff --git a/src/functions/functions.bonding b/src/functions/functions.bonding
index 7939c9624995053327d597d52f567860fdac2d87..f81a8371744457208ee0e945d4f358529bde8a98 100644 (file)
--- a/src/functions/functions.bonding
+++ b/src/functions/functions.bonding
@@ -22,6 +22,7 @@
 BONDING_ALLOWED_MODES="balance-rr active-backup balance-xor broadcast 802.3ad \
        balance-tlb balance-alb"
 BONDING_MASTERS="/sys/class/net/bonding_masters"
+BONDING_PORT_PATTERN="bN"
 
 bonding_init() {
        module_load "bonding"
@@ -68,24 +69,15 @@ bonding_create() {
        # when we need it.
        bonding_init
 
-       # Create the bonding device.
-       fappend "${BONDING_MASTERS}" "+${device}"
-       local ret=$?
-
-       if [ ${ret} -eq ${EXIT_OK} ]; then
+       # Create the bonding device
+       if cmd ip link add "${device}" address "${address}" \
+                       type bond mode "${mode}"; then
                log DEBUG "Successfully created bonding device '${device}'"
        else
                log ERROR "Could not create bonding device '${device}'"
                return ${EXIT_ERROR}
        fi
 
-       # Set the mode of the bonding device.
-       bonding_set_mode "${device}" "${mode}"
-
-       if isset address; then
-               device_set_address "${device}" "${address}"
-       fi
-
        return ${EXIT_OK}
 }
 
@@ -93,18 +85,8 @@ bonding_remove() {
        local device=${1}
        assert isset device
 
-       if ! device_exists ${device}; then
-               return ${EXIT_ERROR}
-       fi
-
-       # Set device down if not already done.
-       device_set_down ${device}
-
        # Remove the device.
-       fappend "${BONDING_MASTERS}" "-${device}"
-       local ret=$?
-
-       if [ ${ret} -eq ${EXIT_OK} ]; then
+       if device_delete "${device}"; then
                log DEBUG "Successfully removed bonding device '${device}'"
        else
                log ERROR "Could not remove bonding device '${device}'"
@@ -148,16 +130,20 @@ bonding_enslave_device() {
 
        shift 2
 
-       # Slave must be down to be enslaved.
-       device_set_down ${device}
+       local slaves="$(bonding_get_slaves "${device}")"
+       if list_match "${slave}" ${slaves}; then
+               log DEBUG "${slave} is already enslaved in ${device}"
+               return ${EXIT_OK}
+       fi
 
-       if device_is_up ${device}; then
+       # Slave must be down to be enslaved.
+       if ! device_set_down "${slave}"; then
                log ERROR "Cannot enslave '${slave}' because it cannot be set down."
                return ${EXIT_ERROR}
        fi
 
-       # Add it.
-       fappend "${SYS_CLASS_NET}/${device}/bonding/slaves" "+${slave}"
+       # Add it
+       cmd ip link set "${slave}" master "${device}"
        local ret=$?
 
        if [ ${ret} -eq ${EXIT_OK} ]; then

diff --git a/src/header-port b/src/header-port
index 50e26d218d5863dcffb473cd7be659f2a1f6b11e..8756e24065ccf7e5014a57236dfbb06a192407ca 100644 (file)
--- a/src/header-port
+++ b/src/header-port
@@ -44,7 +44,7 @@ hook_add() {
        cmd_not_implemented
 }
 
-hook_edit() {
+hook_default_edit() {
        local port=${1}
        assert isset port
        shift
@@ -57,7 +57,11 @@ hook_edit() {
 
        port_settings_write "${port}" ${HOOK_SETTINGS}
 
-       exit ${EXIT_OK}
+       return ${EXIT_OK}
+}
+
+hook_edit() {
+       hook_default_edit $@
 }
 
 hook_info() {

diff --git a/src/hooks/ports/bonding b/src/hooks/ports/bonding
index 6e3f62eeb606dbddb4213eb1a04656c0cd90c8d1..f4accc076fa4894ea220203aa5d4d745e1d2f6c5 100644 (file)
--- a/src/hooks/ports/bonding
+++ b/src/hooks/ports/bonding
@@ -23,7 +23,6 @@
 
 HOOK_SETTINGS="HOOK ADDRESS MIIMON MODE SLAVES"
 
-ADDRESS=$(mac_generate)
 SLAVES=""
 MIIMON=100
 MODE="balance-rr"
@@ -36,15 +35,7 @@ hook_check_settings() {
        assert isinteger MIIMON
 }
 
-hook_new() {
-       hook_edit $@
-}
-
-hook_edit() {
-       local port=${1}
-       assert isset port
-       shift
-
+hook_parse_cmdline() {
        while [ $# -gt 0 ]; do
                case "${1}" in
                        --address=*)
@@ -56,9 +47,24 @@ hook_edit() {
                        --mode=*)
                                MODE=$(cli_get_val ${1})
                                ;;
-                       --slave=*)
-                               slave=$(cli_get_val ${1})
-                               SLAVES="${SLAVES} ${slave}"
+                       +*)
+                               local slave=$(cli_get_val ${1:1})
+
+                               if port_exists "${slave}"; then
+                                       if list_match "${slave}" ${SLAVES}; then
+                                               warning "Port ${slave} is already enslaved"
+                                       else
+                                               list_append SLAVES "${slave}"
+                                       fi
+                               else
+                                       warning "Port ${slave} does not exist"
+                               fi
+                               ;;
+                       -*)
+                               local slave=$(cli_get_val ${1:1})
+                               if ! list_remove SLAVES "${slave}"; then
+                                       warning "Port ${slave} is not a slave of this bonding device"
+                               fi
                                ;;
                        *)
                                warning "Unknown argument '${1}'"
@@ -67,33 +73,69 @@ hook_edit() {
                shift
        done
 
-       DEVICE=${port}
+       if isset ADDRESS; then
+               if ! ismac ADDRESS; then
+                       error "The given MAC address is invalid: ${ADDRESS}"
+                       return ${EXIT_ERROR}
+               fi
+       else
+               ADDRESS=$(mac_generate)
+       fi
+}
 
-       # XXX think this must move to _check()
-       if ! isset DEVICE; then
-               error "You must set a device name."
-               exit ${EXIT_ERROR}
+hook_new() {
+       if ! hook_parse_cmdline $@; then
+               return ${EXIT_ERROR}
        fi
 
-       if ! isset SLAVES; then
-               error "You need to specify at least one slave port (e.g. --slave=port0)."
-               exit ${EXIT_ERROR}
+       # Find a new name
+       local port=$(port_find_free ${BONDING_PORT_PATTERN})
+       assert isset port
+
+       # Save configuration
+       if port_settings_write "${port}" ${HOOK_SETTINGS}; then
+               log INFO "New port ${port} has been created"
+       else
+               error "Could not save configuration for ${port}"
+               return ${EXIT_ERROR}
        fi
 
-       local slave
-       for slave in $(unquote ${SLAVES}); do
-               if ! device_is_ethernet_compatible ${slave}; then
-                       error "The slave device '${slave}' is not able to transport ethernet frames"
-                       exit ${EXIT_ERROR}
+       return ${EXIT_OK}
+}
+
+hook_edit() {
+       local port=${1}
+
+       if ! hook_default_edit $@; then
+               return ${EXIT_ERROR}
+       fi
+
+       # If the master device is up, make sure that all
+       # slaves are up, too
+       if device_exists "${port}"; then
+               # Setting the mode requires us to destroy the device
+               # and to recreate it again.
+               local mode=$(bonding_get_mode "${port}")
+               if [ "${mode}" != "${MODE}" ]; then
+                       port_remove "${port}" && port_create "${port}"
+                       return ${EXIT_OK}
                fi
-       done
 
-       # Remove any whitespace
-       SLAVES=$(echo ${SLAVES})
+               # Set address
+               device_set_address "${port}" "${ADDRESS}"
 
-       port_settings_write "${port}" ${HOOK_SETTINGS}
+               # Set miimon
+               bonding_set_miimon "${port}" "${MIIMON}"
 
-       exit ${EXIT_OK}
+               local slave
+               for slave in ${SLAVES}; do
+                       if device_exists "${slave}"; then
+                               bonding_enslave_device "${port}" "${slave}"
+                       else
+                               port_create "${slave}"
+                       fi
+               done
+       fi
 }
 
 hook_create() {