[Xen-devel] [PATCH 2/5] hotplug/linux: Improve iptables logic

[Sylvain Munaut <s.munaut@xxxxxxxxxxxxxxxxxxxx>]

The main goal of this is to pave the way for IPv6 support, but it
also improves the rules by preventing duplicate incoming packets
rules to be added.

frob_iptables now takes a list of address to handle as parameter
and creates the rules as needed. Any 'common' rule is no longer
repeated.

Here below is a comparaison of the rules before / after.

 <*> For the case where 'ip' is empty or not given at all:

 Previous:

  ACCEPT  all  0.0.0.0/0     0.0.0.0/0  PHYSDEV match --physdev-out
 vif87.0 --physdev-is-bridged
  ACCEPT  all  0.0.0.0/0     0.0.0.0/0  PHYSDEV match --physdev-in
 vif87.0 --physdev-is-bridged

 New:

  ACCEPT  all  0.0.0.0/0     0.0.0.0/0  PHYSDEV match --physdev-out
 vif88.0 --physdev-is-bridged
  ACCEPT  all  0.0.0.0/0     0.0.0.0/0  PHYSDEV match --physdev-in
 vif88.0 --physdev-is-bridged

 <*> For the case where 'ip' is set to "192.168.0.254 192.168.0.141"
 (as an example) :

 Previous:

  ACCEPT  all  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-out
 vif86.0 --physdev-is-bridged
  ACCEPT  udp  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-in
 vif86.0 --physdev-is-bridged udp spt:68 dpt:67
  ACCEPT  all  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-out
 vif86.0 --physdev-is-bridged
  ACCEPT  all  192.168.0.141  0.0.0.0/0  PHYSDEV match --physdev-in
 vif86.0 --physdev-is-bridged
  ACCEPT  all  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-out
 vif86.0 --physdev-is-bridged
  ACCEPT  all  192.168.0.254  0.0.0.0/0  PHYSDEV match --physdev-in
 vif86.0 --physdev-is-bridged

 New:

  ACCEPT  udp  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-in
 vif89.0 --physdev-is-bridged udp spt:68 dpt:67
  ACCEPT  all  0.0.0.0/0      0.0.0.0/0  PHYSDEV match --physdev-out
 vif89.0 --physdev-is-bridged
  ACCEPT  all  192.168.0.254  0.0.0.0/0  PHYSDEV match --physdev-in
 vif89.0 --physdev-is-bridged
  ACCEPT  all  192.168.0.141  0.0.0.0/0  PHYSDEV match --physdev-in
 vif89.0 --physdev-is-bridged

Signed-off-by: Sylvain Munaut <s.munaut@xxxxxxxxxxxxxxxxxxxx>
---
 tools/hotplug/Linux/vif-common.sh |   64 ++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 15 deletions(-)

diff --git a/tools/hotplug/Linux/vif-common.sh 
b/tools/hotplug/Linux/vif-common.sh
index 28ddae5..b098630 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -123,6 +123,10 @@ ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
 
 frob_iptable()
 {
+  local has_err="no"
+  local has_any="no"
+
+  # Add or remove
   if [ "$command" == "online" -o "$command" == "add" ]
   then
     local c="-I"
@@ -130,16 +134,40 @@ frob_iptable()
     local c="-D"
   fi
 
-  iptables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in "$dev" \
-    "$@" -j ACCEPT 2>/dev/null &&
+  # Add rules for each address
+  local addr
+
+  for addr in $@; do
+    if [ "$addr" = "any" ]; then
+      addr="0.0.0.0/0"
+      has_any="yes"
+    fi
+
+    iptables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in "$dev" \
+      -s "$addr" -j ACCEPT 2>/dev/null || has_err="yes"
+  done
+
+  # Always Allow all packets _to_ the domain
   iptables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-out "$dev" \
-    -j ACCEPT 2>/dev/null
+    -j ACCEPT 2>/dev/null || has_err="yes"
 
-  if [ \( "$command" == "online" -o "$command" == "add" \) -a $? -ne 0 ]
+  # If 'any' isn't allowed, we needs to allow a few more things
+  if [ "$has_any" != "yes" ]
+  then
+
+    # Always allow the domain to talk to a DHCP server.
+    iptables "$c" FORWARD -m physdev --physdev-is-bridged --physdev-in "$dev" \
+      -p udp --sport 68 --dport 67 -j ACCEPT 2>/dev/null || has_err="yes"
+
+  fi
+
+  # Error handling
+  if [ \( "$command" == "online" -o "$command" == "add" \) -a "$has_err" == 
"yes" ]
   then
     log err "iptables setup failed. This may affect guest networking."
   fi
 }
+}
 
 
 ##
@@ -160,21 +188,27 @@ handle_iptable()
     return
   fi
 
-  claim_lock "iptables"
+  # Scan through the addresses
+  local ipv4_addrs
 
   if [ "$ip" != "" ]
   then
-      local addr
-      for addr in $ip
-      do
-        frob_iptable -s "$addr"
-      done
-
-      # Always allow the domain to talk to a DHCP server.
-      frob_iptable -p udp --sport 68 --dport 67
+    local addr
+    for addr in $ip
+    do
+        ipv4_addrs="$addr $ipv4_addrs"
+    done
   else
-      # No IP addresses have been specified, so allow anything.
-      frob_iptable
+    # No IP addresses have been specified, so allow anything.
+    ipv4_addrs="any"
+  fi
+
+  # Actually add the rules
+  claim_lock "iptables"
+
+  if [ "$ipv4_addrs" != "" ]
+  then
+    frob_iptable $ipv4_addrs
   fi
 
   release_lock "iptables"
-- 
1.7.10.4


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