From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60955) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XioQO-0006ta-Oy for qemu-devel@nongnu.org; Mon, 27 Oct 2014 13:49:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XioQK-0006lm-2l for qemu-devel@nongnu.org; Mon, 27 Oct 2014 13:49:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53682) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XioQJ-0006lf-Ky for qemu-devel@nongnu.org; Mon, 27 Oct 2014 13:49:44 -0400 Date: Mon, 27 Oct 2014 17:49:20 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20141027174920.GI5021@work-vm> References: <1411464235-5653-1-git-send-email-yanghy@cn.fujitsu.com> <1411464235-5653-22-git-send-email-yanghy@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <1411464235-5653-22-git-send-email-yanghy@cn.fujitsu.com> Subject: Re: [Qemu-devel] [RFC PATCH v2 21/23] COLO nic: implement colo nic device interface configure() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yang Hongyang Cc: GuiJianfeng@cn.fujitsu.com, yunhong.jiang@intel.com, eddie.dong@intel.com, qemu-devel@nongnu.org, mrhines@linux.vnet.ibm.com * Yang Hongyang (yanghy@cn.fujitsu.com) wrote: > implement colo nic device interface configure() > add a script to configure nic devices: > ${QEMU_SCRIPT_DIR}/network-colo >=20 > Script for configuring the network of Master & Slaver. You might also like to think about making this possible via libvirt; it does a lot of network setup for VMs already. > Usage: > network-colo (master|slaver) (install|uninstall) vif pif [ifb1 ifb2] Can you just describe what the four interfaces are; I'm assuming 'vif' is the virtual interface for the VM, and 'pif' is a physical interface the packets from that VM are going to go to/come from. Then I was expecting one more interface where you copy the outgoing packets =66rom the slave so that they go to the primary VM for comparison; why are = there two? > Signed-off-by: Yang Hongyang > --- > include/net/net.h | 1 + > net/colo-nic.c | 106 +++++++++++++++++++++++++++++ > network-colo | 194 ++++++++++++++++++++++++++++++++++++++++++++++++= ++++++ > 3 files changed, 301 insertions(+) > create mode 100755 network-colo >=20 > diff --git a/include/net/net.h b/include/net/net.h > index 62050c5..9cc9b5c 100644 > --- a/include/net/net.h > +++ b/include/net/net.h > @@ -88,6 +88,7 @@ struct NetClientState { > char colo_script[1024]; > char colo_nicname[128]; > char ifname[128]; > + char ifb[2][128]; It's a shame that these are all magic 128 character lengths. > unsigned receive_disabled : 1; > NetClientDestructor *destructor; > unsigned int queue_index; > diff --git a/net/colo-nic.c b/net/colo-nic.c > index 7255a48..d661d8b 100644 > --- a/net/colo-nic.c > +++ b/net/colo-nic.c > @@ -10,6 +10,7 @@ > */ > #include "net/net.h" > #include "net/colo-nic.h" > +#include "qemu/error-report.h" > =20 > typedef struct nic_device { > NetClientState *nc; > @@ -26,11 +27,116 @@ static bool nic_support_colo(NetClientState *nc) > return nc && nc->colo_script[0] && nc->colo_nicname[0]; > } > =20 > +#define STDOUT_BUF_LEN 1024 > +static char stdout_buf[STDOUT_BUF_LEN]; > + > +static int launch_colo_script(char *argv[]) > +{ > + int pid, status; > + char *script =3D argv[0]; > + int fds[2]; > + > + bzero(stdout_buf, sizeof(stdout_buf)); > + > + if (pipe(fds) < 0) { > + return -1; > + } > + /* try to launch network script */ > + pid =3D fork(); > + if (pid =3D=3D 0) { > + close(fds[0]); > + dup2(fds[1], STDOUT_FILENO); > + execv(script, argv); > + _exit(1); > + } else if (pid > 0) { > + FILE *stream; > + int n; > + close(fds[1]); > + stream =3D fdopen(fds[0], "r"); > + n =3D fread(stdout_buf, 1, STDOUT_BUF_LEN - 1, stream); > + stdout_buf[n] =3D '\0'; > + close(fds[0]); > + > + while (waitpid(pid, &status, 0) !=3D pid) { > + /* loop */ > + } > + > + if (WIFEXITED(status) && WEXITSTATUS(status) =3D=3D 0) { > + return 0; > + } > + } > + fprintf(stderr, "%s\n", stdout_buf); > + fprintf(stderr, "%s: could not launch network script\n", script); > + return -1; > +} > + > +static void store_ifbname(NetClientState *nc) > +{ > + char *str_b =3D NULL, *str_e =3D NULL; > + > + str_b =3D strstr(stdout_buf, "ifb0=3D"); > + if (str_b) { > + str_e =3D strstr(str_b, "\n"); > + } > + if (str_e) { > + snprintf(nc->ifb[0], str_e - str_b - 5 + 1, "%s", str_b + 5); > + } > + > + str_b =3D str_e =3D NULL; > + str_b =3D strstr(stdout_buf, "ifb1=3D"); > + if (str_b) { > + str_e =3D strstr(str_b, "\n"); > + } > + if (str_e) { > + snprintf(nc->ifb[1], str_e - str_b - 5 + 1, "%s", str_b + 5); > + } > +} I don't think these protect against a bad string that's longer than the 128 character ifb[] length. I think it would also be good to return sometype of status from this function to know if it worked. > +static int nic_configure(NetClientState *nc, bool up, bool is_slave) > +{ > + char *argv[8]; > + char **parg; > + int ret =3D -1, i; > + int argc =3D (!is_slave && !up) ? 7 : 5; > + > + if (!nc) { > + error_report("Can not parse colo_script or colo_nicname"); > + return ret; > + } > + > + parg =3D argv; > + *parg++ =3D nc->colo_script; > + *parg++ =3D (char *)(is_slave ? "slaver" : "master"); > + *parg++ =3D (char *)(up ? "install" : "uninstall"); > + *parg++ =3D nc->ifname; > + *parg++ =3D nc->colo_nicname; > + if (!is_slave && !up) { > + *parg++ =3D nc->ifb[0]; > + *parg++ =3D nc->ifb[1]; > + } > + *parg =3D NULL; > + > + for (i =3D 0; i < argc; i++) { > + if (!argv[i][0]) { > + error_report("Can not get colo_script argument"); > + return ret; > + } > + } > + > + ret =3D launch_colo_script(argv); > + if (!is_slave && up && ret =3D=3D 0) { > + store_ifbname(nc); > + } > + > + return ret; > +} > + > void colo_add_nic_devices(NetClientState *nc) > { > struct nic_device *nic =3D g_malloc0(sizeof(*nic)); > =20 > nic->support_colo =3D nic_support_colo; > + nic->configure =3D nic_configure; > =20 > /* > * TODO > diff --git a/network-colo b/network-colo > new file mode 100755 > index 0000000..9112888 > --- /dev/null > +++ b/network-colo > @@ -0,0 +1,194 @@ > +#! /bin/bash > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > +# ${QEMU_SCRIPT_DIR}/network-colo > +# > +# Script for configuring the network of Master & Slaver. > +# > +# Usage: > +# network-colo (master|slaver) (install|uninstall) vif pif [ifb1 ifb2] > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > + > +sides=3D$1 > +op=3D$2 > +vif=3D$3 > +pif=3D$4 > +ifb1=3D$5 > +ifb2=3D$6 > +BR=3Dbr1 > + > +qlen=3D40960 > +module=3D"HA_compare" > +device=3D"HA_compare" > + > +# start_master ifbx > +function start_master() { > + > + # In colo mode, we don't use gso, gro... > + ip link set dev $vif qlen $qlen > + > + # copy and foward input packets to $pif > + tc qdisc add dev $vif root handle 1: prio > + tc filter add dev $vif parent 1: protocol ip prio 10 u32 match u32 0= 0 flowid 1:2 action mirred egress mirror dev $pif > + tc filter add dev $vif parent 1: protocol arp prio 11 u32 match u32 = 0 0 flowid 1:2 action mirred egress mirror dev $pif > + > + # foward output packets to ifbx > + tc qdisc add dev $vif ingress > + tc filter add dev $vif parent ffff: protocol ip prio 10 u32 match u3= 2 0 0 flowid 1:2 action mirred egress redirect dev $1 > + tc filter add dev $vif parent ffff: protocol arp prio 11 u32 match u= 32 0 0 flowid 1:2 action mirred egress redirect dev $1 > +} > + > +function stop_master() { > + # don't copy and foward input packets to $pif > + tc filter del dev $vif parent 1: protocol ip prio 10 u32 > + tc filter del dev $vif parent 1: protocol arp prio 11 u32 > + tc qdisc del dev $vif root handle 1: prio > + > + # don't foward output packets to ifbx > + tc filter del dev $vif parent ffff: protocol ip prio 10 u32 > + tc filter del dev $vif parent ffff: protocol arp prio 11 u32 > + tc qdisc del dev $vif ingress > +} > + > +# load_module module parameter > +function load_module() > +{ > + local module=3D$1 > + shift > + > + lsmod | grep -q "$module" That's perhaps not selective enough, especially since 'HA_compare' would match in the 'HA_compare_icmp' line. > + if [[ $? -eq 0 ]]; then > + # The module has been loaded > + return > + fi > + > + modprobe $module "$@" > +} > + > +function select_ifb() > +{ > + local -i index > + > + for (( index =3D 0; index < 100; index++)); do > + state=3D$(ip link show dev ifb$index | sed -n -e 's/.*state \([a= -zA-Z]*\) .*/\1/p') > + if [[ $state =3D=3D "DOWN" ]]; then > + return $index > + fi > + done The fixed sized loop seems weird; maybe something like: ip link show |awk -F' ' ' { if ( $9 =3D=3D "DOWN" ) { print $2; }; }' or even : grep down /sys/class/net/ifb*/operstate > + > + return 100 > +} > + > + > +function install_master() { > + load_module sch_colo > + load_module HA_compare > + load_module HA_compare_icmp > + > + if [[ ! -e "/dev/$device" ]]; then > + major=3D$(awk "\$2=3D=3D\"$module\" {print \$1}" /proc/devices) > + mknod /dev/$device c $major 0 > + fi So I think module at that point is HA_compare ? > + load_module ifb numifbs=3D100 > + > + select_ifb > + index1=3D$? > + if [[ $index1 -eq 100 ]]; then > + echo "index1 $index1 overflow" >>/root/network-colo.log > + exit 1 > + fi > + ip link set ifb$index1 up > + ip link set ifb$index1 qlen $qlen > + > + select_ifb > + index2=3D$? > + if [[ $index2 -eq 100 ]]; then > + echo "index1 $index1 overflow" >>/root/network-colo.log > + exit 1 > + fi > + ip link set ifb$index2 up > + ip link set ifb$index2 qlen $qlen > + colo_tc qdisc add dev ifb$index1 root handle 1: colo dev ifb$index2 = master > + colo_tc qdisc add dev ifb$index2 root handle 1: colo dev ifb$index1 = slaver > + > + ifconfig $pif promisc > + ip link set $pif qlen $qlen > + > + # forward packets from $pif to ifb$index2 > + tc qdisc add dev $pif ingress > + tc filter add dev $pif parent ffff: protocol ip prio 10 u32 match u3= 2 0 0 flowid 1:2 action mirred egress redirect dev ifb$index2 > + tc filter add dev $pif parent ffff: protocol arp prio 11 u32 match u= 32 0 0 flowid 1:2 action mirred egress redirect dev ifb$index2 > + > + start_master ifb$index1 > +} > + > +function uninstall_master() { > + stop_master > + > + # shutdown $ifb1 > + tc qdisc del dev $ifb1 root handle 1: colo > + ip link set $ifb1 down > + > + # don't forward packets from $pif to $ifb2 > + tc filter del dev $pif parent ffff: protocol ip prio 10 u32 > + tc qdisc del dev $pif ingress > + > + # shutdown $ifb2 > + tc qdisc del dev $ifb2 root handle 1: colo > + ip link set $ifb2 down > + > + ifconfig $pif -promisc > +} > + > +function install_slaver() > +{ > + ifconfig $pif promisc > + ip link set $pif qlen $qlen > + > + # forward packets from $pif to $vif > + tc qdisc add dev $pif ingress > + tc filter add dev $pif parent ffff: protocol ip prio 10 u32 match u3= 2 0 0 flowid 1:2 action mirred egress redirect dev $vif > + tc filter add dev $pif parent ffff: protocol arp prio 11 u32 match u= 32 0 0 flowid 1:2 action mirred egress redirect dev $vif > + > + ip link set $vif qlen $qlen > + # forward packets from $vif to $pif > + tc qdisc add dev $vif ingress > + tc filter add dev $vif parent ffff: protocol ip prio 10 u32 match u3= 2 0 0 flowid 1:2 action mirred egress redirect dev $pif > + tc filter add dev $vif parent ffff: protocol arp prio 11 u32 match u= 32 0 0 flowid 1:2 action mirred egress redirect dev $pif > + > + brctl delif $BR $vif > +} > + > +function uninstall_slaver() > +{ > + # don't forward packets from $pif to $vif > + tc filter del dev $pif parent ffff: protocol ip prio 10 u32 > + tc filter del dev $pif parent ffff: protocol arp prio 11 u32 > + tc qdisc del dev $pif ingress > + > + # don't forward packets from $vif to $pif > + tc filter del dev $vif parent ffff: protocol ip prio 10 u32 > + tc filter del dev $vif parent ffff: protocol arp prio 11 u32 > + tc qdisc del dev $vif ingress > + > + ifconfig $pif -promisc > + > + brctl addif $BR $vif > +} > + > +echo "$@" >/root/network-colo.log > +if [[ $1 !=3D "master" && $1 !=3D "slaver" ]]; then > + echo "$1 !=3D master/slaver" >>/root/network-colo.log > + exit 1 > +fi > + > +if [[ $2 !=3D "install" && $2 !=3D "uninstall" ]]; then > + echo "$2 !=3D install/uninstall" >>/root/network-colo.log > + exit 1 > +fi > + > +${op}_$sides 1>>/root/network-colo.log 2>&1 > +if [[ $1 =3D=3D "master" && $2 =3D=3D "install" ]]; then > + echo ifb0=3Difb$index1 > + echo ifb1=3Difb$index2 > +fi > --=20 > 1.9.1 >=20 -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK