[PATCH 1/2] dracut: add team device support

[PATCH 1/2] dracut: add team device support

[Cong Wang]

This patch adds the initial support for team device.
A new cmdline team= is introduced for it.

Note, currently we don't support stacked devices
on/under team, it is tricky and can be added on request.

Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
Cc: Harald Hoyer <harald-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Cong Wang <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 modules.d/40network/ifup.sh         |   20 +++++++++++++++
 modules.d/40network/module-setup.sh |    4 ++-
 modules.d/40network/net-genrules.sh |    5 ++++
 modules.d/40network/parse-team.sh   |   45 +++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 1 deletions(-)
 create mode 100755 modules.d/40network/parse-team.sh

diff --git a/modules.d/40network/ifup.sh b/modules.d/40network/ifup.sh
index db233de..d5ec4e0 100755
--- a/modules.d/40network/ifup.sh
+++ b/modules.d/40network/ifup.sh
@@ -28,6 +28,15 @@ if [ -e /tmp/bond.info ]; then
     done
 fi
 
+if [ -e /tmp/team.info ]; then
+    . /tmp/team.info
+    for slave in $teamslaves ; do
+        if [ "$netif" = "$slave" ] ; then
+            netif=$teammaster
+        fi
+    done
+fi
+
 # bridge this interface?
 if [ -e /tmp/bridge.info ]; then
     . /tmp/bridge.info
@@ -167,6 +176,17 @@ if [ -e /tmp/bond.info ]; then
     fi
 fi
 
+if [ -e /tmp/team.info ]; then
+    . /tmp/team.info
+    if [ "$netif" = "$teammaster" ] && [ ! -e /tmp/net.$teammaster.up ] ; then
+        # team needs slaves to be down
+        for slave in $teamslaves ; do
+            ip link set $slave down
+        done
+        teamd -d -f /etc/teamd/$teammaster.conf
+        ip link set $teammaster up
+    fi
+fi
 
 # XXX need error handling like dhclient-script
 
diff --git a/modules.d/40network/module-setup.sh b/modules.d/40network/module-setup.sh
index da0f447..f2a8f73 100755
--- a/modules.d/40network/module-setup.sh
+++ b/modules.d/40network/module-setup.sh
@@ -64,7 +64,7 @@ installkernel() {
     { find_kernel_modules_by_path drivers/net; if [ "$_arch" = "s390" -o "$_arch" = "s390x" ]; then find_kernel_modules_by_path drivers/s390/net; fi; } \
         | net_module_filter | instmods
 
-    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding 8021q af_packet virtio_net
+    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding 8021q af_packet virtio_net =drivers/net/team
 }
 
 install() {
@@ -72,6 +72,7 @@ install() {
     dracut_install ip arping dhclient sed
     dracut_install -o ping ping6
     dracut_install -o brctl
+    dracut_install -o teamd teamdctl teamnl
     inst_script "$moddir/ifup.sh" "/sbin/ifup"
     inst_script "$moddir/netroot.sh" "/sbin/netroot"
     inst_script "$moddir/dhclient-script.sh" "/sbin/dhclient-script"
@@ -82,6 +83,7 @@ install() {
     inst_hook cmdline 91 "$moddir/dhcp-root.sh"
     inst_hook cmdline 95 "$moddir/parse-vlan.sh"
     inst_hook cmdline 96 "$moddir/parse-bond.sh"
+    inst_hook cmdline 96 "$moddir/parse-team.sh"
     inst_hook cmdline 97 "$moddir/parse-bridge.sh"
     inst_hook cmdline 98 "$moddir/parse-ip-opts.sh"
     inst_hook cmdline 99 "$moddir/parse-ifname.sh"
diff --git a/modules.d/40network/net-genrules.sh b/modules.d/40network/net-genrules.sh
index 6a58ab5..f5cd200 100755
--- a/modules.d/40network/net-genrules.sh
+++ b/modules.d/40network/net-genrules.sh
@@ -34,6 +34,11 @@ fi
         IFACES+=" ${bondslaves%% *}"
     fi
 
+    if [ -e /tmp/team.info ]; then
+        . /tmp/team.info
+        IFACES+=" ${teamslaves}"
+    fi
+
     if [ -e /tmp/vlan.info ]; then
         . /tmp/vlan.info
         IFACES+=" $phydevice"
diff --git a/modules.d/40network/parse-team.sh b/modules.d/40network/parse-team.sh
new file mode 100755
index 0000000..182a5b7
--- /dev/null
+++ b/modules.d/40network/parse-team.sh
@@ -0,0 +1,45 @@
+#!/bin/sh
+#
+# Format:
+#       team=<teammaster>:<teamslaves>
+#
+#       teamslaves is a comma-separated list of physical (ethernet) interfaces
+#
+
+# return if team already parsed
+[ -n "$teammaster" ] && return
+
+# Check if team parameter is valid
+if getarg team= >/dev/null ; then
+    :
+fi
+
+# We translate list of slaves to space-separated here to mwke it easier to loop over them in ifup
+parseteam() {
+    local v=${1}:
+    set --
+    while [ -n "$v" ]; do
+        set -- "$@" "${v%%:*}"
+        v=${v#*:}
+    done
+
+    unset teammaster teamslaves
+    case $# in
+    2)  teammaster=$1; teamslaves=$(str_replace "$2" "," " ") ;;
+    *)  die "team= requires two parameters" ;;
+    esac
+}
+
+unset teammaster teamslaves
+
+if getarg team>/dev/null; then
+    # Read team= parameters if they exist
+    team="$(getarg team=)"
+    if [ ! "$team" = "team" ]; then
+        parseteam "$(getarg team=)"
+    fi
+    # Make it suitable for initscripts export
+    echo "teammaster=$teammaster" > /tmp/team.info
+    echo "teamslaves=\"$teamslaves\"" >> /tmp/team.info
+    return
+fi
-- 
1.7.7.6

[PATCH 2/2] dracut: document team cmdline

[Cong Wang]

Document team= in dracut.cmdline.

Cc: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
Cc: Harald Hoyer <harald-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Cong Wang <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 dracut.cmdline.7.asc |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/dracut.cmdline.7.asc b/dracut.cmdline.7.asc
index 2a74604..c2eec5e 100644
--- a/dracut.cmdline.7.asc
+++ b/dracut.cmdline.7.asc
@@ -411,6 +411,10 @@ interface name. Better name it "bootnet" or "bluesocket".
     then its values should be separated by semicolon.
     Bond without parameters assumes bond=bond0:eth0,eth1:mode=balance-rr
 
+**team =_<teammaster>_:_<teamslaves>_**::
+    Setup team device <teammaster> on top of <teamslaves>.
+    <teamslaves> is a comma-separated list of physical (ethernet) interfaces.
+
 **bridge=_<bridgename>_:_<ethnames>_**::
     Setup bridge <bridgename> with <ethnames>. <ethnames> is a comma-separated
     list of physical (ethernet) interfaces. Bridge without parameters assumes bridge=br0:eth0
-- 
1.7.7.6

Re: [PATCH 1/2] dracut: add team device support

[Cong Wang]

Ping...

Any review? Harald?

On Thu, 2012-11-15 at 22:58 +0800, Cong Wang wrote:
> This patch adds the initial support for team device.
> A new cmdline team= is introduced for it.
> 
> Note, currently we don't support stacked devices
> on/under team, it is tricky and can be added on request.
> 
> Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
> Cc: Harald Hoyer <harald-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Cong Wang <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  modules.d/40network/ifup.sh         |   20 +++++++++++++++
>  modules.d/40network/module-setup.sh |    4 ++-
>  modules.d/40network/net-genrules.sh |    5 ++++
>  modules.d/40network/parse-team.sh   |   45 +++++++++++++++++++++++++++++++++++
>  4 files changed, 73 insertions(+), 1 deletions(-)
>  create mode 100755 modules.d/40network/parse-team.sh
> 
> diff --git a/modules.d/40network/ifup.sh b/modules.d/40network/ifup.sh
> index db233de..d5ec4e0 100755
> --- a/modules.d/40network/ifup.sh
> +++ b/modules.d/40network/ifup.sh
> @@ -28,6 +28,15 @@ if [ -e /tmp/bond.info ]; then
>      done
>  fi
>  
> +if [ -e /tmp/team.info ]; then
> +    . /tmp/team.info
> +    for slave in $teamslaves ; do
> +        if [ "$netif" = "$slave" ] ; then
> +            netif=$teammaster
> +        fi
> +    done
> +fi
> +
>  # bridge this interface?
>  if [ -e /tmp/bridge.info ]; then
>      . /tmp/bridge.info
> @@ -167,6 +176,17 @@ if [ -e /tmp/bond.info ]; then
>      fi
>  fi
>  
> +if [ -e /tmp/team.info ]; then
> +    . /tmp/team.info
> +    if [ "$netif" = "$teammaster" ] && [ ! -e /tmp/net.$teammaster.up ] ; then
> +        # team needs slaves to be down
> +        for slave in $teamslaves ; do
> +            ip link set $slave down
> +        done
> +        teamd -d -f /etc/teamd/$teammaster.conf
> +        ip link set $teammaster up
> +    fi
> +fi
>  
>  # XXX need error handling like dhclient-script
>  
> diff --git a/modules.d/40network/module-setup.sh b/modules.d/40network/module-setup.sh
> index da0f447..f2a8f73 100755
> --- a/modules.d/40network/module-setup.sh
> +++ b/modules.d/40network/module-setup.sh
> @@ -64,7 +64,7 @@ installkernel() {
>      { find_kernel_modules_by_path drivers/net; if [ "$_arch" = "s390" -o "$_arch" = "s390x" ]; then find_kernel_modules_by_path drivers/s390/net; fi; } \
>          | net_module_filter | instmods
>  
> -    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding 8021q af_packet virtio_net
> +    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding 8021q af_packet virtio_net =drivers/net/team
>  }
>  
>  install() {
> @@ -72,6 +72,7 @@ install() {
>      dracut_install ip arping dhclient sed
>      dracut_install -o ping ping6
>      dracut_install -o brctl
> +    dracut_install -o teamd teamdctl teamnl
>      inst_script "$moddir/ifup.sh" "/sbin/ifup"
>      inst_script "$moddir/netroot.sh" "/sbin/netroot"
>      inst_script "$moddir/dhclient-script.sh" "/sbin/dhclient-script"
> @@ -82,6 +83,7 @@ install() {
>      inst_hook cmdline 91 "$moddir/dhcp-root.sh"
>      inst_hook cmdline 95 "$moddir/parse-vlan.sh"
>      inst_hook cmdline 96 "$moddir/parse-bond.sh"
> +    inst_hook cmdline 96 "$moddir/parse-team.sh"
>      inst_hook cmdline 97 "$moddir/parse-bridge.sh"
>      inst_hook cmdline 98 "$moddir/parse-ip-opts.sh"
>      inst_hook cmdline 99 "$moddir/parse-ifname.sh"
> diff --git a/modules.d/40network/net-genrules.sh b/modules.d/40network/net-genrules.sh
> index 6a58ab5..f5cd200 100755
> --- a/modules.d/40network/net-genrules.sh
> +++ b/modules.d/40network/net-genrules.sh
> @@ -34,6 +34,11 @@ fi
>          IFACES+=" ${bondslaves%% *}"
>      fi
>  
> +    if [ -e /tmp/team.info ]; then
> +        . /tmp/team.info
> +        IFACES+=" ${teamslaves}"
> +    fi
> +
>      if [ -e /tmp/vlan.info ]; then
>          . /tmp/vlan.info
>          IFACES+=" $phydevice"
> diff --git a/modules.d/40network/parse-team.sh b/modules.d/40network/parse-team.sh
> new file mode 100755
> index 0000000..182a5b7
> --- /dev/null
> +++ b/modules.d/40network/parse-team.sh
> @@ -0,0 +1,45 @@
> +#!/bin/sh
> +#
> +# Format:
> +#       team=<teammaster>:<teamslaves>
> +#
> +#       teamslaves is a comma-separated list of physical (ethernet) interfaces
> +#
> +
> +# return if team already parsed
> +[ -n "$teammaster" ] && return
> +
> +# Check if team parameter is valid
> +if getarg team= >/dev/null ; then
> +    :
> +fi
> +
> +# We translate list of slaves to space-separated here to mwke it easier to loop over them in ifup
> +parseteam() {
> +    local v=${1}:
> +    set --
> +    while [ -n "$v" ]; do
> +        set -- "$@" "${v%%:*}"
> +        v=${v#*:}
> +    done
> +
> +    unset teammaster teamslaves
> +    case $# in
> +    2)  teammaster=$1; teamslaves=$(str_replace "$2" "," " ") ;;
> +    *)  die "team= requires two parameters" ;;
> +    esac
> +}
> +
> +unset teammaster teamslaves
> +
> +if getarg team>/dev/null; then
> +    # Read team= parameters if they exist
> +    team="$(getarg team=)"
> +    if [ ! "$team" = "team" ]; then
> +        parseteam "$(getarg team=)"
> +    fi
> +    # Make it suitable for initscripts export
> +    echo "teammaster=$teammaster" > /tmp/team.info
> +    echo "teamslaves=\"$teamslaves\"" >> /tmp/team.info
> +    return
> +fi

Re: [PATCH 1/2] dracut: add team device support

[Dave Young]

On 11/15/2012 10:58 PM, Cong Wang wrote:

> This patch adds the initial support for team device.
> A new cmdline team= is introduced for it.
> 
> Note, currently we don't support stacked devices
> on/under team, it is tricky and can be added on request.
> 
> Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
> Cc: Harald Hoyer <harald-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Cong Wang <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  modules.d/40network/ifup.sh         |   20 +++++++++++++++
>  modules.d/40network/module-setup.sh |    4 ++-
>  modules.d/40network/net-genrules.sh |    5 ++++
>  modules.d/40network/parse-team.sh   |   45 +++++++++++++++++++++++++++++++++++
>  4 files changed, 73 insertions(+), 1 deletions(-)
>  create mode 100755 modules.d/40network/parse-team.sh
> 
> diff --git a/modules.d/40network/ifup.sh b/modules.d/40network/ifup.sh
> index db233de..d5ec4e0 100755
> --- a/modules.d/40network/ifup.sh
> +++ b/modules.d/40network/ifup.sh
> @@ -28,6 +28,15 @@ if [ -e /tmp/bond.info ]; then
>      done
>  fi
>  
> +if [ -e /tmp/team.info ]; then
> +    . /tmp/team.info
> +    for slave in $teamslaves ; do
> +        if [ "$netif" = "$slave" ] ; then
> +            netif=$teammaster
> +        fi
> +    done
> +fi
> +
>  # bridge this interface?
>  if [ -e /tmp/bridge.info ]; then
>      . /tmp/bridge.info
> @@ -167,6 +176,17 @@ if [ -e /tmp/bond.info ]; then
>      fi
>  fi
>  
> +if [ -e /tmp/team.info ]; then
> +    . /tmp/team.info
> +    if [ "$netif" = "$teammaster" ] && [ ! -e /tmp/net.$teammaster.up ] ; then
> +        # team needs slaves to be down
> +        for slave in $teamslaves ; do
> +            ip link set $slave down
> +        done
> +        teamd -d -f /etc/teamd/$teammaster.conf
> +        ip link set $teammaster up
> +    fi
> +fi
>  
>  # XXX need error handling like dhclient-script
>  
> diff --git a/modules.d/40network/module-setup.sh b/modules.d/40network/module-setup.sh
> index da0f447..f2a8f73 100755
> --- a/modules.d/40network/module-setup.sh
> +++ b/modules.d/40network/module-setup.sh
> @@ -64,7 +64,7 @@ installkernel() {
>      { find_kernel_modules_by_path drivers/net; if [ "$_arch" = "s390" -o "$_arch" = "s390x" ]; then find_kernel_modules_by_path drivers/s390/net; fi; } \
>          | net_module_filter | instmods
>  
> -    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding 8021q af_packet virtio_net
> +    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding 8021q af_packet virtio_net =drivers/net/team



40network installs all related kernel modules. This will create a big
initramfs though some of them are not used at all.

I'm thinking it's better to conditionally install them. In the long run
maybe network module should be splited to to multiple modules.

And for now I think it's better to install them only if there's team
related user space tools in running system.


>  }
>  
>  install() {
> @@ -72,6 +72,7 @@ install() {
>      dracut_install ip arping dhclient sed
>      dracut_install -o ping ping6
>      dracut_install -o brctl
> +    dracut_install -o teamd teamdctl teamnl
>      inst_script "$moddir/ifup.sh" "/sbin/ifup"
>      inst_script "$moddir/netroot.sh" "/sbin/netroot"
>      inst_script "$moddir/dhclient-script.sh" "/sbin/dhclient-script"
> @@ -82,6 +83,7 @@ install() {
>      inst_hook cmdline 91 "$moddir/dhcp-root.sh"
>      inst_hook cmdline 95 "$moddir/parse-vlan.sh"
>      inst_hook cmdline 96 "$moddir/parse-bond.sh"
> +    inst_hook cmdline 96 "$moddir/parse-team.sh"
>      inst_hook cmdline 97 "$moddir/parse-bridge.sh"
>      inst_hook cmdline 98 "$moddir/parse-ip-opts.sh"
>      inst_hook cmdline 99 "$moddir/parse-ifname.sh"
> diff --git a/modules.d/40network/net-genrules.sh b/modules.d/40network/net-genrules.sh
> index 6a58ab5..f5cd200 100755
> --- a/modules.d/40network/net-genrules.sh
> +++ b/modules.d/40network/net-genrules.sh
> @@ -34,6 +34,11 @@ fi
>          IFACES+=" ${bondslaves%% *}"
>      fi
>  
> +    if [ -e /tmp/team.info ]; then
> +        . /tmp/team.info
> +        IFACES+=" ${teamslaves}"
> +    fi
> +
>      if [ -e /tmp/vlan.info ]; then
>          . /tmp/vlan.info
>          IFACES+=" $phydevice"
> diff --git a/modules.d/40network/parse-team.sh b/modules.d/40network/parse-team.sh
> new file mode 100755
> index 0000000..182a5b7
> --- /dev/null
> +++ b/modules.d/40network/parse-team.sh
> @@ -0,0 +1,45 @@
> +#!/bin/sh
> +#
> +# Format:
> +#       team=<teammaster>:<teamslaves>
> +#
> +#       teamslaves is a comma-separated list of physical (ethernet) interfaces
> +#
> +
> +# return if team already parsed
> +[ -n "$teammaster" ] && return
> +
> +# Check if team parameter is valid
> +if getarg team= >/dev/null ; then
> +    :
> +fi


I do not understand why there's above checking, looks like it's useless.
At the bottom of this file it's also checked.

Is it intend to do some checking about the user space tools existence in
if ... fi?

> +
> +# We translate list of slaves to space-separated here to mwke it easier to loop over them in ifup
> +parseteam() {
> +    local v=${1}:
> +    set --
> +    while [ -n "$v" ]; do
> +        set -- "$@" "${v%%:*}"
> +        v=${v#*:}
> +    done
> +
> +    unset teammaster teamslaves
> +    case $# in
> +    2)  teammaster=$1; teamslaves=$(str_replace "$2" "," " ") ;;
> +    *)  die "team= requires two parameters" ;;
> +    esac
> +}
> +
> +unset teammaster teamslaves
> +
> +if getarg team>/dev/null; then
> +    # Read team= parameters if they exist
> +    team="$(getarg team=)"
> +    if [ ! "$team" = "team" ]; then
> +        parseteam "$(getarg team=)"
> +    fi
> +    # Make it suitable for initscripts export
> +    echo "teammaster=$teammaster" > /tmp/team.info
> +    echo "teamslaves=\"$teamslaves\"" >> /tmp/team.info
> +    return
> +fi



-- 
Thanks
Dave

Re: [PATCH 1/2] dracut: add team device support

[Cong Wang]

On Mon, 2012-12-03 at 11:11 +0800, Dave Young wrote:
> > -    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
> 8021q af_packet virtio_net
> > +    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
> 8021q af_packet virtio_net =drivers/net/team
> 
> 
> 
> 40network installs all related kernel modules. This will create a big
> initramfs though some of them are not used at all.
> 
> I'm thinking it's better to conditionally install them. In the long
> run
> maybe network module should be splited to to multiple modules.
> 
> And for now I think it's better to install them only if there's team
> related user space tools in running system. 

It is not as easy as what you think, users may specify their network
config at _run time_ via cmdline, so it is not easy to know which kernel
module will be used until we get all the cmdline.

For kdump, one solution is removing all unnecessary stuffs including
kernel modules and user-space utitlies at setup time, because kdump
parses the network config at setup time. Obviously this only applies for
kdump, for others, e.g. anaconda, this problem still exists, but they
probably don't care about the memory usage as much as kdump.

However, I don't think this is relevant here, we should do this in a
different patch. It is unfair to only blame team driver here, there are
already some other modules in the same line anyway...

Re: [PATCH 1/2] dracut: add team device support

[Dave Young]

On 12/03/2012 11:27 AM, Cong Wang wrote:

> On Mon, 2012-12-03 at 11:11 +0800, Dave Young wrote:
>>> -    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
>> 8021q af_packet virtio_net
>>> +    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
>> 8021q af_packet virtio_net =drivers/net/team
>>
>>
>>
>> 40network installs all related kernel modules. This will create a big
>> initramfs though some of them are not used at all.
>>
>> I'm thinking it's better to conditionally install them. In the long
>> run
>> maybe network module should be splited to to multiple modules.
>>
>> And for now I think it's better to install them only if there's team
>> related user space tools in running system. 
> 
> It is not as easy as what you think, users may specify their network
> config at _run time_ via cmdline, so it is not easy to know which kernel
> module will be used until we get all the cmdline.


I agree it's not easy. But for team support if there's no team user
space utility the 2nd kernel setup will fail, isn't it?

> 
> For kdump, one solution is removing all unnecessary stuffs including
> kernel modules and user-space utitlies at setup time, because kdump
> parses the network config at setup time. Obviously this only applies for
> kdump, for others, e.g. anaconda, this problem still exists, but they
> probably don't care about the memory usage as much as kdump.


I will thinking more about the kdump solution. But even for anaconda,
smaller initramfs size is also good to have.

> 
> However, I don't think this is relevant here, we should do this in a
> different patch. It is unfair to only blame team driver here, there are
> already some other modules in the same line anyway...


This is not a blame for team code, just begin to consider the impact to
initramfs size. It's good to start do it once we notice it.
.

-- 
Thanks
Dave

Re: [PATCH 1/2] dracut: add team device support

[Cong Wang]

On Mon, 2012-12-03 at 13:27 +0800, Dave Young wrote:
> On 12/03/2012 11:27 AM, Cong Wang wrote:
> 
> > On Mon, 2012-12-03 at 11:11 +0800, Dave Young wrote:
> >>> -    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
> >> 8021q af_packet virtio_net
> >>> +    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
> >> 8021q af_packet virtio_net =drivers/net/team
> >>
> >>
> >>
> >> 40network installs all related kernel modules. This will create a big
> >> initramfs though some of them are not used at all.
> >>
> >> I'm thinking it's better to conditionally install them. In the long
> >> run
> >> maybe network module should be splited to to multiple modules.
> >>
> >> And for now I think it's better to install them only if there's team
> >> related user space tools in running system. 
> > 
> > It is not as easy as what you think, users may specify their network
> > config at _run time_ via cmdline, so it is not easy to know which kernel
> > module will be used until we get all the cmdline.
> 
> 
> I agree it's not easy. But for team support if there's no team user
> space utility the 2nd kernel setup will fail, isn't it?

Yes, bridge too.

> 
> > 
> > For kdump, one solution is removing all unnecessary stuffs including
> > kernel modules and user-space utitlies at setup time, because kdump
> > parses the network config at setup time. Obviously this only applies for
> > kdump, for others, e.g. anaconda, this problem still exists, but they
> > probably don't care about the memory usage as much as kdump.
> 
> 
> I will thinking more about the kdump solution. But even for anaconda,
> smaller initramfs size is also good to have.

It saves too little, as nowadays we all have 4G+ memory even a KVM
guest.

% ls -lh /lib/modules/3.6.7-4.fc16.x86_64/kernel/drivers/net/team/team*
-rwxr--r--. 1 root root  42K Nov 21
04:44 /lib/modules/3.6.7-4.fc16.x86_64/kernel/drivers/net/team/team.ko
-rwxr--r--. 1 root root 6.3K Nov 21
04:44 /lib/modules/3.6.7-4.fc16.x86_64/kernel/drivers/net/team/team_mode_activebackup.ko
-rwxr--r--. 1 root root 5.8K Nov 21
04:44 /lib/modules/3.6.7-4.fc16.x86_64/kernel/drivers/net/team/team_mode_broadcast.ko
-rwxr--r--. 1 root root  15K Nov 21
04:44 /lib/modules/3.6.7-4.fc16.x86_64/kernel/drivers/net/team/team_mode_loadbalance.ko
-rwxr--r--. 1 root root 5.5K Nov 21
04:44 /lib/modules/3.6.7-4.fc16.x86_64/kernel/drivers/net/team/team_mode_roundrobin.ko


> 
> > 
> > However, I don't think this is relevant here, we should do this in a
> > different patch. It is unfair to only blame team driver here, there are
> > already some other modules in the same line anyway...
> 
> 
> This is not a blame for team code, just begin to consider the impact to
> initramfs size. It's good to start do it once we notice it.
> .

On my machine, bridge.ko is 146K, bonding.ko is 198K, both are more than
the total size of all team modules. You shall start even earlier. :)

Re: [PATCH 1/2] dracut: add team device support

[Jiri Pirko]

Mon, Dec 03, 2012 at 04:27:03AM CET, amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
>On Mon, 2012-12-03 at 11:11 +0800, Dave Young wrote:
>> > -    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
>> 8021q af_packet virtio_net
>> > +    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
>> 8021q af_packet virtio_net =drivers/net/team
>> 
>> 
>> 
>> 40network installs all related kernel modules. This will create a big
>> initramfs though some of them are not used at all.
>> 
>> I'm thinking it's better to conditionally install them. In the long
>> run
>> maybe network module should be splited to to multiple modules.
>> 
>> And for now I think it's better to install them only if there's team
>> related user space tools in running system. 
>
>It is not as easy as what you think, users may specify their network
>config at _run time_ via cmdline, so it is not easy to know which kernel
>module will be used until we get all the cmdline.
>
>For kdump, one solution is removing all unnecessary stuffs including
>kernel modules and user-space utitlies at setup time, because kdump
>parses the network config at setup time. Obviously this only applies for
>kdump, for others, e.g. anaconda, this problem still exists, but they
>probably don't care about the memory usage as much as kdump.
>
>However, I don't think this is relevant here, we should do this in a
>different patch. It is unfair to only blame team driver here, there are
>already some other modules in the same line anyway...

+ team modules are thin.

>
>

Re: [PATCH 1/2] dracut: add team device support

[Vivek Goyal]

On Mon, Dec 03, 2012 at 03:24:52PM +0100, Jiri Pirko wrote:
> Mon, Dec 03, 2012 at 04:27:03AM CET, amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
> >On Mon, 2012-12-03 at 11:11 +0800, Dave Young wrote:
> >> > -    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
> >> 8021q af_packet virtio_net
> >> > +    instmods =drivers/net/phy ecb arc4 bridge stp llc ipv6 bonding
> >> 8021q af_packet virtio_net =drivers/net/team
> >> 
> >> 
> >> 
> >> 40network installs all related kernel modules. This will create a big
> >> initramfs though some of them are not used at all.
> >> 
> >> I'm thinking it's better to conditionally install them. In the long
> >> run
> >> maybe network module should be splited to to multiple modules.
> >> 
> >> And for now I think it's better to install them only if there's team
> >> related user space tools in running system. 
> >
> >It is not as easy as what you think, users may specify their network
> >config at _run time_ via cmdline, so it is not easy to know which kernel
> >module will be used until we get all the cmdline.
> >
> >For kdump, one solution is removing all unnecessary stuffs including
> >kernel modules and user-space utitlies at setup time, because kdump
> >parses the network config at setup time. Obviously this only applies for
> >kdump, for others, e.g. anaconda, this problem still exists, but they
> >probably don't care about the memory usage as much as kdump.
> >
> >However, I don't think this is relevant here, we should do this in a
> >different patch. It is unfair to only blame team driver here, there are
> >already some other modules in the same line anyway...
> 
> + team modules are thin.

Will they be included in host only mode too (-H, --host-only) ? We use 
hostonly option for building kdump initramfs. So if root is not on team
device, will it still be included.

If no, it is fine but if yes, then we need to fix it. We try very hard to
keep kdump initramfs size small.

Thanks
Vivek

Re: [PATCH 1/2] dracut: add team device support

[Cong Wang]

On Tue, 2012-12-04 at 17:18 -0500, Vivek Goyal wrote:
> 
> Will they be included in host only mode too (-H, --host-only) ? We use 
> hostonly option for building kdump initramfs. So if root is not on team
> device, will it still be included.
> 
> If no, it is fine but if yes, then we need to fix it. We try very hard to
> keep kdump initramfs size small.
> 

% git grep hostonly modules.d/40network/

The problem is the _whole_ network module doesn't respect hostonly, we
should fix that first. It deserves another patch.

Re: [PATCH 1/2] dracut: add team device support

[Cong Wang]

On Tue, 2012-12-04 at 17:18 -0500, Vivek Goyal wrote:
> 
> Will they be included in host only mode too (-H, --host-only) ? We use 
> hostonly option for building kdump initramfs. So if root is not on team
> device, will it still be included.
> 
> If no, it is fine but if yes, then we need to fix it. We try very hard to
> keep kdump initramfs size small.

Actually, instmods() checks hostonly flag and calls
module_is_host_only() to check if the module is loaded by the first
kernel. So, we don't need to worry about it at all. Same for
`dracut_install -o`

I am going to fix the active-backup mode, but it seems the latest teamd
is still not ready for this, it at least should not error out when any
port in the config file disappears. I am still checking this with Jiri.

Good news is that teamdctl now can connect to teamd via unix socket.

Thanks!