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 device

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] Re: [PATCH] vif-common.sh to support tap network devices in iptables FORWARD chain
From: Teck Choon Giam <giamteckchoon@xxxxxxxxx>
Date: Tue, 14 Jul 2009 12:07:33 +0800
Delivery-date: Mon, 13 Jul 2009 21:07:58 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:content-type :content-transfer-encoding; bh=MBYqfAZrfU7TMx3FtGGod2jQQ/mUaQ7Ahsezjm08prI=; b=V++oUrcfIeikvxCuvBZIpsAAq3h32GqXpm0k0shdt7VRPCmO2GCixYhVwviDpZrGGf rr5dxSLyhOJNT94B6LPMcld2BY5rIoXkrSsKVV0Q6hoVAupXQGZrHPJtnwUXHx6NVhuB e74q80oZ0lxP4w2c46Nuc0o5vm/Dq9nzJzAsQ=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type:content-transfer-encoding; b=k9LQgCpvfSqwQc/13eVCVltaVnyz2vd/qaJyFGfghjb7KqrIHBgaToW8usiPMf+MpY qRaF9gYVjcNxO2YX6ZDuhgCf9CuJvj+W9wiam22Qy8kvPLvC6I6mYMDKgUNHLhnBIPP4 pgmwtnweOA66sl1BKEXDxpizLRlwkBQoF2l0s=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20090713234533.GB10136@xxxxxxxxxxxx>
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> <20090713234533.GB10136@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Hi,

On Tue, Jul 14, 2009 at 7:45 AM, Simon Horman<horms@xxxxxxxxxxxx> wrote:
> 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.

Then there isn't a need to have such comments in the patch I submit.
I will remove the comments then if the patch is fine.

>
>> +  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?

This is just my habit to include grep -v grep and you are free to
remove it.  Some shell scripts I coded needed that if the grep result
grep itself especially for ps fauwx related.

>> +  # 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.

Ok noted.

Thanks.

Kindest regards,
Giam Teck Choon

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

<Prev in Thread] Current Thread [Next in Thread>