Incorrect logic of _modify_rules() in IptablesManager

[Qin Zhao]

The logic of _modify_rules() seems not correct. For instance, assuming that we have a in-memory table like this:

:bn-chain001 - [0:0]
:chain002 - [0:0]
[0:0] -A bn-chain001 rule001
[0:0] -A chain002 rule002

and iptables-save output like this:

# Generated by zhaoqin on mars
*zhaoqin
:bn-chain001 - [0:0]
[0:0] -A bn-chain001 rule001
[0:0] -A chain002 rule002
COMMIT
# Completed on moon

The current code of _modify_rules() will generate the following result:

# Generated by zhaoqin on mars
:chain002 - [0:0]
:bn-chain001 - [0:0]
[0:0] -A bn-chain001 rule001
[0:0] -A chain002 rule002
*zhaoqin
COMMIT
# Completed on moon

The root cause is that rule '[0:0] -A chain002 rule002' is in new_filter list is removed, so that the current code will do 'rules_index -= 1'. That is an incorrect action. The correct action is to do 'rules_index -= 1', if one chain entry in new_filter list is removed, because the chain list is above the rule list.

[Qin Zhao (zhaoqin)]

Another reason why I open this bug is that I plan to propose a lot code changes to _modify_rules() for https://bugs.launchpad.net/neutron/+bug/1352826, in order to improve its performance. Before do that, I hope to add a unit test for _modify_rules() in this bug, so that we will not make it wrong in the future.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/115538

[OpenStack Infra (hudson-openstack)]

Change abandoned by Kyle Mestery (<email address hidden>) on branch: master
Review: https://review.openstack.org/115538
Reason: This change is old enough and hasn't seen any updates since August 20, 2014. Abandoning it, please revive it if you plan to work on it again.