race in vif-common.sh

race in vif-common.sh

[Andreas Kinzler]

Hello

in /etc/xen/scripts/vif-common.sh there is a function handle_iptable. At its start there is a check for a working iptables implementation. This check is outside the iptables lock section (claim_lock "iptables") and even if it is only a read-only operation the underlying iptables operation still accesses the xtables lock. I debugged a malfunction (=race) with multiple vif-interfaces down to the following iptables error message in the check section above:

iptables -L -n
Another app is currently holding the xtables lock. Perhaps you want to use the -w option?

So this check needs to be inside the lock or removed at all (iptables should be working).

Regards Andreas

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

Re: race in vif-common.sh

[Wei Liu]

CC George (author of recent change) and Ian

On Mon, Jul 03, 2017 at 01:30:09PM +0200, Andreas Kinzler wrote:
> Hello
> 
> in /etc/xen/scripts/vif-common.sh there is a function handle_iptable.
> At its start there is a check for a working iptables implementation.
> This check is outside the iptables lock section (claim_lock
> "iptables") and even if it is only a read-only operation the
> underlying iptables operation still accesses the xtables lock. I
> debugged a malfunction (=race) with multiple vif-interfaces down to
> the following iptables error message in the check section above:
> 
> iptables -L -n Another app is currently holding the xtables lock.
> Perhaps you want to use the -w option?
> 
> So this check needs to be inside the lock or removed at all (iptables
> should be working).
> 
> Regards Andreas
> 
> _______________________________________________ Xen-devel mailing list
> Xen-devel@lists.xen.org https://lists.xen.org/xen-devel

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

Re: race in vif-common.sh

[George Dunlap]

On 07/03/2017 02:07 PM, Wei Liu wrote:
> CC George (author of recent change) and Ian
> 
> On Mon, Jul 03, 2017 at 01:30:09PM +0200, Andreas Kinzler wrote:
>> Hello
>>
>> in /etc/xen/scripts/vif-common.sh there is a function handle_iptable.
>> At its start there is a check for a working iptables implementation.
>> This check is outside the iptables lock section (claim_lock
>> "iptables") and even if it is only a read-only operation the
>> underlying iptables operation still accesses the xtables lock. I
>> debugged a malfunction (=race) with multiple vif-interfaces down to
>> the following iptables error message in the check section above:
>>
>> iptables -L -n Another app is currently holding the xtables lock.
>> Perhaps you want to use the -w option?
>>
>> So this check needs to be inside the lock or removed at all (iptables
>> should be working).

Andreas,

What version of the script are you looking at?

For 4.9 we checked in a fix to this problem that would specifically
attempt to use the -w option if it was available; see c/s 3d2010f9ff.

From your description it sounds like you are using an older version, is
that correct?  Can you try cherry-picking that changeset?

 -George

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

Re: race in vif-common.sh

[Andreas Kinzler]

On Thu, 27 Jul 2017 12:55:14 +0200, George Dunlap  
<george.dunlap@citrix.com> wrote:
> For 4.9 we checked in a fix to this problem that would specifically
> attempt to use the -w option if it was available; see c/s 3d2010f9ff.

Sorry, I think that this patch is just far to complicated. If you really  
want to keep the "iptables is working check" (lines 1-7 of function  
handle_iptable) then you should just move it inside the claim_lock  
"iptables" section and you won't need any -w option and no iptables_w()  
check.

Regards Andreas

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

Re: race in vif-common.sh

[George Dunlap]

> On Jul 27, 2017, at 5:12 PM, Andreas Kinzler <ml-ak@posteo.de> wrote:
> 
> On Thu, 27 Jul 2017 12:55:14 +0200, George Dunlap <george.dunlap@citrix.com> wrote:
>> For 4.9 we checked in a fix to this problem that would specifically
>> attempt to use the -w option if it was available; see c/s 3d2010f9ff.
> 
> Sorry, I think that this patch is just far to complicated. If you really want to keep the "iptables is working check" (lines 1-7 of function handle_iptable) then you should just move it inside the claim_lock "iptables" section and you won't need any -w option and no iptables_w() check.

That assumes that vif-common.sh is the only thing on the system that ever calls iptables (since even simply querying the tables wants to grab the lock).  I’m afraid that’s not a very good assumption to make.

 -George

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

Re: race in vif-common.sh

[Andreas Kinzler]

On Thu, 27 Jul 2017 18:49:47 +0200, George Dunlap  
<George.Dunlap@citrix.com> wrote:
>> Sorry, I think that this patch is just far to complicated. If you  
>> really want to keep the "iptables is working check" (lines 1-7 of  
>> function handle_iptable) then you should just move it inside the  
>> claim_lock "iptables" section and you won't need any -w option and no  
>> iptables_w() check.
> That assumes that vif-common.sh is the only thing on the system that  
> ever calls iptables (since even simply querying the tables wants to grab  
> the lock).  I’m afraid that’s not a very good assumption to make.

Hmm, I see your point but that boils down to a total different question  
that has nothing to do with Xen: should iptables have "-w" as a default?  
Somehow the current state (-w is not a default) seems to work for most  
people/cases.

Regards Andreas

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

Re: race in vif-common.sh

[George Dunlap]

> On Jul 27, 2017, at 6:11 PM, Andreas Kinzler <ml-ak@posteo.de> wrote:
> 
> On Thu, 27 Jul 2017 18:49:47 +0200, George Dunlap <George.Dunlap@citrix.com> wrote:
>>> Sorry, I think that this patch is just far to complicated. If you really want to keep the "iptables is working check" (lines 1-7 of function handle_iptable) then you should just move it inside the claim_lock "iptables" section and you won't need any -w option and no iptables_w() check.
>> That assumes that vif-common.sh is the only thing on the system that ever calls iptables (since even simply querying the tables wants to grab the lock).  I’m afraid that’s not a very good assumption to make.
> 
> Hmm, I see your point but that boils down to a total different question that has nothing to do with Xen: should iptables have "-w" as a default? Somehow the current state (-w is not a default) seems to work for most people/cases.

Well it works for most people / cases entirely by accident.  Search for your error message and you’ll find hundreds of people have problems because -w is not the default.

If you want to submit a patch to iptables to make ‘-w’ the default, that would be great.  But our script will have to deal with the current behavior until we can be sure that none of our users are using the old version of iptables.

 -George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

race in vif-common.sh

[Andreas Kinzler]

Hello

in /etc/xen/scripts/vif-common.sh there is a function handle_iptable. At its start there is a check for a working iptables implementation. This check is outside the iptables lock section (claim_lock "iptables") and even if it is only a read-only operation the underlying iptables operation still accesses the xtables lock. I debugged a malfunction (=race) with multiple vif-interfaces down to the following iptables error message in the check section above:

iptables -L -n
Another app is currently holding the xtables lock. Perhaps you want to use the -w option?

So this check needs to be inside the lock or removed at all (iptables should be working).

Regards Andreas



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