Re: [Xen-devel] [PATCH 2 of 5 V2] tools/hotplug: Remus network buffering setup scripts

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-08-29 at 15:16 -0700, Shriram Rajagopalan wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@xxxxxxxxx>
> # Date 1377812185 25200
> # Node ID 79b21d778550f74e550af791eca41d4ca152492a
> # Parent  cc2b88cba1c67cc0e0a00c7e63a3ba97e14609cb
> tools/hotplug: Remus network buffering setup scripts
> 
> This patch introduces remus-netbuf-setup hotplug script responsible for
> setting up and tearing down the necessary infrastructure required for
> network output buffering in Remus.  This script is intended to be invoked
> by libxl for each guest interface, when starting or stopping Remus.
> 
> Apart from returning success/failure indication via the usual hotplug
> entires in xenstore, this script also writes to xenstore, the name of

"entries"

Also, if you haven't already in a later patch please can you update
docs/misc/xenstore-paths.markdown at some point in this series.

> the IFB device to be used to control the vif's network output.
> 
> The script relies on libnl3 command line utilities to perform various
> setup/teardown functions. The script is confined to Linux platforms only
> since NetBSD does not seem to have libnl3.
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>

> +
> +# We only check for modules. We dont load them.

"don't" (sorry, pedantic I know)

> +# User/Admin is supposed to load ifb during boot time,
> +# ensuring that there are enough free ifbs in the system.
> +# Other modules will be loaded automatically by tc commands.

Is there (going to be) a howto on the wiki or somewhere in this series?

> +check_modules() {
> +    for m in ifb sch_plug sch_ingress act_mirred cls_u32
> +    do
> +     if ! modinfo $m > /dev/null 2>&1; then
> +         fatal "Unable to find $m kernel module"
> +     fi
> +    done
> +}
> +
> +setup_ifb() {
> +
> +    for ifb in `ifconfig -a -s|egrep ^ifb|cut -d ' ' -f1`
> +    do
> +     local installed=`nl-qdisc-list -d $ifb`
> +     [ -n "$installed" ] && continue
> +     IFB="$ifb"
> +     break
> +    done

This is a bit unfortunate, but I guess it is hard to do much else from
shell though I suppose.

Is there any locking in the script to avoid two instances racing in this
function? I didn't see it but maybe it's in common.sh or something.

> +
> +case "$command" in
> +    setup)
> +     check_libnl_tools
> +     check_modules
> +     setup_ifb
> +     redirect_vif_traffic "$vifname" "$IFB"
> +     add_plug_qdisc "$vifname" "$IFB"
> +     #not using xenstore_write that automatically exits on error
> +     # because we need to cleanup
> +     _xenstore_write "$XENBUS_PATH/ifb" "$IFB" || xs_write_failed "$vifname" 
> "$IFB"
> +        ;;

Some tab vs space confusion here.

> +    teardown)
> +        teardown_netbuf "$vifname" "$IFB"
> +        ;;
> +esac
> +
> +log debug "Successful remus-netbuf-setup $command for $vifname, ifb $IFB."
> +
> +if [ "$command" = "setup" ]
> +then
> +  success
> +fi



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel