iptables manager in quantum zero's packet and byte counts, regression from nova code

Bug #1125393 reported by Brian Haley
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Wishlist
Brian Haley

Bug Description

Back in the Folsom release I made a change to the Nova IptablesManager code to preserve packet:byte counts in iptables chains and rules (https://review.openstack.org/#/c/11300/). I just realized that some of the other iptables manager code was merged into Quantum but this wasn't. These counters can help users analyze network usage for an instance over a long period of time, so are useful for things like billing.

I will work on porting these changes over from Nova, but in the future it might be best to put them in a more central location (oslo?) so that bugs/features only have to get done in one place, not two. Any feedback on that would be appreciated.

Tags: sg-fw
Changed in quantum:
assignee: nobody → Brian Haley (brian-haley)
Revision history for this message
dan wendlandt (danwent) wrote :

HI Brian,

Adding this to quantum makes sense in general.

I don't think it makes sense to have networking specific code in Oslo. From talking with Vish, the plan has been to effectively freeze Nova networking functionality, which will nudge people over to Quantum over time. Thus, it should be very rare that people are trying to add a feature to both. Bug fixes between the two are likely to be different as well.

tags: added: sg-fw
Changed in quantum:
importance: Undecided → Wishlist
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to quantum (master)

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

Changed in quantum:
status: New → In Progress
Revision history for this message
Brian Haley (brian-haley) wrote :

So what is the best way to get someone to review this change? I didn't want to just go assign someone since we're try to get Grizzly done, but it's been a few weeks now.

Revision history for this message
Brian Haley (brian-haley) wrote :

I've also found and fixed a couple of other bugs while porting this to quantum. The one I remember is:

- The code would sometimes replace a rule that had a packet:byte count with a zero:zero one because it wasn't correctly removing duplicates by backing-up one index position the first time through the rule list.

Revision history for this message
Brian Haley (brian-haley) wrote :

Another issue I found in the iptables_firewall code is that it builds iptables rules in reverse, for example:

-j RETURN -p udp -m udp --dport 68 --sport 67 -s 10.11.12.2/32

This causes the code in iptables_manager to not match that since iptables-save outputs them in this order:

-s 10.11.12.2/32 -p udp -m udp --sport 67 --dport 68 -j RETURN

Had to tweak the code there a little.

Some of the other code needed similar tweaks, but no change in the functionality.

Also, when specifying either TCP or UDP (-p tcp), iptables-save will always put the module name on output. For example:

-p tcp -m tcp

I had to change the code to always put that '-m $proto' as well since otherwise we won't match exactly.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/24739
Committed: http://github.com/openstack/neutron/commit/62d872e4cf0186f59c7902a396e2ee7a3154cbf4
Submitter: Jenkins
Branch: master

commit 62d872e4cf0186f59c7902a396e2ee7a3154cbf4
Author: Brian Haley <email address hidden>
Date: Thu Mar 14 10:35:49 2013 -0400

    Preserve packet:byte counts in iptables_manager.

    Ported the nova iptables manager code to neutron, so that we
    use iptables-save/restore with the -c flag to save/restore
    the chains and rules with their packet:byte counts. All other
    changes were ported as well to keep the code as similar as
    possible between the two, although they will be different as
    I had to fix other bugs found during testing.

    Updated tests accordingly to account for new calls and
    input/output changes in formatting.

    Changed iptables_firewall code to add iptables rules in the same
    order that iptables-save will print them: source/dest, protocol,
    sport, dport, target; else iptables_manager won't be able
    to find them to preserve their [packet:byte] counts.
    Tweaked other rules accordingly as necessary.

    Fixed a bug introduced in an earlier version of this patch where
    _modify_rules() sometimes wouldn't match an existing rule correctly
    if not top=true.

    Fixes bug 1125393

    Change-Id: I858c552d8a7ae24f52f8e8daa05ac37026705773

Changed in neutron:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in neutron:
milestone: none → havana-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in neutron:
milestone: havana-2 → 2013.2
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.