Security group code is doing unnecessary work removing chains

Bug #1642770 reported by Brian Haley
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Low
Brian Haley

Bug Description

The security group code is generating a lot of these messages when trying to boot VMs:

Attempted to remove chain sg-chain which does not exist

There's also ones specific to the port. It seems to be calling remove_chain(), even when it's a new port and it's initially setting up it's filter. I dropped a print_stack() in remove_chain() and see tracebacks like this:

Prepare port filter for e8f41910-c24e-41f1-ae7f-355e9bb1d18a _apply_port_filter /opt/stack/neutron/neutron/agent/securitygroups_rpc.py:163
Preparing device (e8f41910-c24e-41f1-ae7f-355e9bb1d18a) filter prepare_port_filter /opt/stack/neutron/neutron/agent/linux/iptables_firewall.py:170
Attempted to remove chain sg-chain which does not exist remove_chain /opt/stack/neutron/neutron/agent/linux/iptables_manager.py:177
  File "/usr/local/lib/python2.7/dist-packages/eventlet/greenthread.py", line 214, in main
    result = function(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/ryu/lib/hub.py", line 54, in _launch
    return func(*args, **kwargs)
  File "/opt/stack/neutron/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_ryuapp.py", line 37, in agent_main_wrapper
    ovs_agent.main(bridge_classes)
  File "/opt/stack/neutron/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py", line 2177, in main
    agent.daemon_loop()
  File "/usr/local/lib/python2.7/dist-packages/osprofiler/profiler.py", line 154, in wrapper
    return f(*args, **kwargs)
  File "/opt/stack/neutron/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py", line 2098, in daemon_loop
    self.rpc_loop(polling_manager=pm)
  File "/usr/local/lib/python2.7/dist-packages/osprofiler/profiler.py", line 154, in wrapper
    return f(*args, **kwargs)
  File "/opt/stack/neutron/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py", line 2049, in rpc_loop
    port_info, ovs_restarted)
  File "/usr/local/lib/python2.7/dist-packages/osprofiler/profiler.py", line 154, in wrapper
    return f(*args, **kwargs)
  File "/opt/stack/neutron/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py", line 1657, in process_network_ports
    port_info.get('updated', set()))
  File "/opt/stack/neutron/neutron/agent/securitygroups_rpc.py", line 266, in setup_port_filters
    self.prepare_devices_filter(new_devices)
  File "/opt/stack/neutron/neutron/agent/securitygroups_rpc.py", line 131, in decorated_function
    *args, **kwargs)
  File "/opt/stack/neutron/neutron/agent/securitygroups_rpc.py", line 139, in prepare_devices_filter
    self._apply_port_filter(device_ids)
  File "/opt/stack/neutron/neutron/agent/securitygroups_rpc.py", line 164, in _apply_port_filter
    self.firewall.prepare_port_filter(device)
  File "/usr/lib/python2.7/contextlib.py", line 24, in __exit__
    self.gen.next()
  File "/opt/stack/neutron/neutron/agent/firewall.py", line 139, in defer_apply
    self.filter_defer_apply_off()
  File "/opt/stack/neutron/neutron/agent/linux/iptables_firewall.py", line 838, in filter_defer_apply_off
    self._pre_defer_unfiltered_ports)
  File "/opt/stack/neutron/neutron/agent/linux/iptables_firewall.py", line 248, in _remove_chains_apply
    self._remove_chain_by_name_v4v6(SG_CHAIN)
  File "/opt/stack/neutron/neutron/agent/linux/iptables_firewall.py", line 279, in _remove_chain_by_name_v4v6
    self.iptables.ipv4['filter'].remove_chain(chain_name)
  File "/opt/stack/neutron/neutron/agent/linux/iptables_manager.py", line 178, in remove_chain
    traceback.print_stack()

Looking at the code, there's a couple of things that are interesting:

1) prepare_port_filter() calls self._remove_chains() - why?
2) in the "defer" case above we always do _remove_chains_apply()/_setup_chains_apply() - is there some way to skip the remove?

This also led to us timing how long it's taking in the remove_chain() code, since that's where the message is getting printed. As the number of ports and rules grow, it's spending more time spinning through chains and rules. It looks like that can be helped with a small code change, which is just fallout from the real problem. I'll send that out since it helps a little.

More work still required.

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

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

Changed in neutron:
status: New → In Progress
Changed in neutron:
importance: Undecided → Low
Revision history for this message
rick jones (perfgeek) wrote :

The tweak to remove_chain of which Brian speaks takes the time spent in those "Attempted" blocks from about 4.2 seconds to about 3.2 seconds when one is adding the 255th, single-port instance to a compute node with 2xE5-2640 0 @ 2.50GHz processors. (Time from first "Attempted" in the log to the last).

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

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

Reviewed: https://review.openstack.org/399291
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=5ff8aecefb69ed4ea97b45725f0da29611694987
Submitter: Jenkins
Branch: master

commit 5ff8aecefb69ed4ea97b45725f0da29611694987
Author: Brian Haley <email address hidden>
Date: Thu Nov 17 18:11:37 2016 -0500

    Speed-up iptables_manager remove_chain() code

    remove_chain() has three list iterations that can be
    reduced to two, brings a small performance increase.

    Change-Id: I6f6e05b2336a983062f4787cb933d56fdf71a90a
    Partial-bug: #1642770

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (stable/newton)

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/406364

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

Reviewed: https://review.openstack.org/406364
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=49c2e146039cb5eec550e07f8e7642b591585e07
Submitter: Jenkins
Branch: stable/newton

commit 49c2e146039cb5eec550e07f8e7642b591585e07
Author: Brian Haley <email address hidden>
Date: Thu Nov 17 18:11:37 2016 -0500

    Speed-up iptables_manager remove_chain() code

    remove_chain() has three list iterations that can be
    reduced to two, brings a small performance increase.

    Change-Id: I6f6e05b2336a983062f4787cb933d56fdf71a90a
    Partial-bug: #1642770
    (cherry picked from commit 5ff8aecefb69ed4ea97b45725f0da29611694987)

tags: added: in-stable-newton
Changed in neutron:
assignee: Brian Haley (brian-haley) → Armando Migliaccio (armando-migliaccio)
Changed in neutron:
assignee: Armando Migliaccio (armando-migliaccio) → Brian Haley (brian-haley)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/400943
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=6958829471160bd13c6db8795cdbe61b81356c7d
Submitter: Jenkins
Branch: master

commit 6958829471160bd13c6db8795cdbe61b81356c7d
Author: Brian Haley <email address hidden>
Date: Tue Nov 22 12:35:50 2016 -0500

    Do not try and remove non-existent iptables chains

    When creating a new filter for a port, there is no need
    to remove any iptables chains as none should exist. This
    just leads to more work and increases the number of logging
    messages about non-existent chains.

    Change-Id: I1cecd39cf5fd046d84a1ef47c245a0a22e9323cb
    Partial-bug: #1642770

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (stable/newton)

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/434988

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

Reviewed: https://review.openstack.org/434988
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=606ec7eabb207c6f824acd2c13ed69e360c9aa38
Submitter: Jenkins
Branch: stable/newton

commit 606ec7eabb207c6f824acd2c13ed69e360c9aa38
Author: Brian Haley <email address hidden>
Date: Tue Nov 22 12:35:50 2016 -0500

    Do not try and remove non-existent iptables chains

    When creating a new filter for a port, there is no need
    to remove any iptables chains as none should exist. This
    just leads to more work and increases the number of logging
    messages about non-existent chains.

    Change-Id: I1cecd39cf5fd046d84a1ef47c245a0a22e9323cb
    Partial-bug: #1642770
    (cherry picked from commit 6958829471160bd13c6db8795cdbe61b81356c7d)

Changed in neutron:
status: In Progress → Fix Released
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.