Re: [Xen-devel] Re: [PATCH] vif-common.sh to support tap network devices in iptables FORWARD chain

[Simon Horman <horms@xxxxxxxxxxxx>]

Hi,

On Tue, Jul 07, 2009 at 07:49:15PM +0800, Teck Choon Giam wrote:
> Sorry, the previous patch I sent in only support xm create to add in
> iptables FORWARD chain but when you xm shutdown the tap related
> ruleset is not removed from iptables FORWARD chain.  Below is the
> patch which support xm create and xm shutdown.
> 
> --- vif-common.sh.orig  2009-07-07 19:09:39.000000000 +0800
> +++ vif-common.sh       2009-07-07 19:47:48.000000000 +0800
> @@ -73,6 +73,24 @@
>      local c="-D"
>    fi
> 
> +  # Added support for tap network devices in iptables FORWARD chain as this
> +  # is required if antispoof is enabled or otherwise all packets to/from tap
> +  # devices will be dropped.
> +  # Start adding by Giam Teck Choon.

Its not necessary to add comments that read like a changelog as
they go in the changelog which is included in the version control system.
Rather, comments in the code should just explain what the code does.

> +  local tapif=`echo $vif | sed 's/vif/tap/'`
> +  # for xm create
> +  local checktapif=`cat /proc/net/dev | grep "${tapif}:" | grep -v grep`

Why is the second grep needed?

> +  # for xm shutdown
> +  local checktapstate=`iptables -L -n | grep "state
> RELATED,ESTABLISHED PHYSDEV match --physdev-out ${tapif}"`
> +
> +  if [ -n "$checktapif" ] || [ -n "$checktapstate" ] ; then
> +    iptables "$c" FORWARD -m physdev --physdev-in "$tapif" "$@" -j ACCEPT \
> +      2>/dev/null &&
> +    iptables "$c" FORWARD -m state --state RELATED,ESTABLISHED -m physdev \
> +      --physdev-out "$tapif" -j ACCEPT 2>/dev/null
> +  fi
> +  # End adding by Giam Teck Choon.

Comments like this are not necessary either.

> +
>    iptables "$c" FORWARD -m physdev --physdev-in "$vif" "$@" -j ACCEPT \
>      2>/dev/null &&
>    iptables "$c" FORWARD -m state --state RELATED,ESTABLISHED -m physdev \
> 
> 
> Thanks.
> 
> Kindest regards,
> Giam Teck Choon
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel