WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Teck Choon Giam <giamteckchoon@xxxxxxxxx>
Subject: Re: [Xen-devel] Re: [PATCH] vif-common.sh to support tap network devices in iptables FORWARD chain
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Tue, 14 Jul 2009 09:45:33 +1000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 13 Jul 2009 16:46:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <9b5c9bb30907070449q5cdedd24y56483999f4463b12@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <9b5c9bb30907070429m19fa021yacef5f3c9d664ec5@xxxxxxxxxxxxxx> <9b5c9bb30907070449q5cdedd24y56483999f4463b12@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
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