Flows per in_port are deleted after SG rules are applied

[Rodolfo Alonso]

During the creation of a new port in the integration bridge (br-int), first the firewall rules are applied and then all flows matching this input port are deleted:

if cur_tag != lvm.vlan:
                self.int_br.delete_flows(in_port=port.ofport)

This happens only when the port is created (or the vlan tag changes). If any firewall rule is applied using the in_port as a condition, during the initialization of the firewall for this port, this rule is deleted.

Instead of that, this security action should be moved to the previous function, "_add_port_tag_info", in order to avoid any firewall rule deletion and maintaining the same security level during the port creation; that means the ports doesn't allow any kind of traffic until the firewall rules are applied.

how to reproduce:

    Start the Neutron agent with the OVS firewall configured.
    Wait untill all ovs flows are stablished. You'll see some flows with conditions "in_port=xx". Those are set in "initialize_port_flows", in the OVS firewall.
    Stop the agent. No flow must be deleted. Make a capture of all the flows.
    Restart the agent. At this point, the VLAN tag should be different from the last one assigned by the agent.
    Now you can compare the flows in OVS to the lsit of flows in step 3.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/295155

[OpenStack Infra (hudson-openstack)]

Change abandoned by Ihar Hrachyshka (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/295155
Reason: Please repropose once master and mitaka patches land.

[IWAMOTO Toshihiro (iwamoto)]

Not directly related, but the current implementation of ovsfirewall does del-flows then add-flows for update operations. This causes a short disruption of connectivity.
If the problem can be tackled by allocating some more cookies for ovsfirewall (eg. per-port or per-SG), it may be worth considering now.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Please, review this patch: https://review.openstack.org/#/c/295154/

It's been there form weeks, waiting for review.

[IWAMOTO Toshihiro (iwamoto)]

Not directly related, but the current implementation of ovsfirewall does del-flows then add-flows for update operations. This causes a short disruption of connectivity.
If the problem can be tackled by allocating some more cookies for ovsfirewall (eg. per-port or per-SG), it may be worth considering now.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

@iwamoto I agree with your comment. But this bug tries to solve this problem. Furthermore, I'm adding cookies to the FW, so in a future modifying the flow management made by the FW could be easier.

Please, take a look at the patch (https://review.openstack.org/#/c/295154/).

Thank you for your comments!

[Ihar Hrachyshka (ihar-hrachyshka)]

Not critical for Mitaka.

[IWAMOTO Toshihiro (iwamoto)]

It seems to be one of the causes of ovsfw+tempest failure.
admin_state_up true -> false ->true transition seems to cause the cur_tag != lvm.vlan check to fire and ovsfw flows are wiped out.

http://logs.openstack.org/00/399400/5/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/41d30bc/

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

I found this error several months ago, doing manual tests. It should be merged as soon as possible. Having a patch not reviewed/accepted for 9 months is frustrating for the developer and bad for the product.

[OpenStack Infra (hudson-openstack)]

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

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

@IWAMOTO: I have a patch submited nine months ago and I'm still waiting for reviews. This patch is still active and I'm taking care of it.

It's impolite the way you moved the assignee of this patch, submiting another one with the same number.

[IWAMOTO Toshihiro (iwamoto)]

Hi Rodolfo,

I just followed your comment on
https://review.openstack.org/#/c/295154/29/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py@1664

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Another patch but not for this bug, because your solution doesn't solves the problem. Maybe it does for the tempest test you mentioned, but not this bug.

Anyway, I'll wait for other reviewers, but I won't refactor anymore this patch. This is taking too long.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/406731
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[IWAMOTO Toshihiro (iwamoto)]

I followed the sequence in the description but couldn't reproduce.

1. I started an openstack system with devstack.
2. When the system is up, there are 2 flows with in_port=xx. They are both in_port=1, which specifies int-br-ex.
3. Restarted ovs-agent. No change.

I also tried with some ports and vlans.

1'. Same as 1
2'. Devstack sets up 2 networks: private and public. Issued the following.
$ openstack network create privdeux
$ openstack subnet create --subnet-range 10.0.1.0/26 --network privdeux pri
vdeux-subnet
$ openstack --image cirros-0.3.4-x86_64-uec --flavor m1.tiny --nic net-id=p
rivate --nic net-id=privdeux hoge

3': At this point there are 29 flows with in_port=xxx. Issued the following to capture local vlan ids.

$ sudo ovs-vsctl list Port >/tmp/port.txt

4': Restarted ovs-agent. There are still 29 flows with in_port=xxx. "ovs-vsctl list Port" output was same, indicating no local vlan change.

[YAMAMOTO Takashi (yamamoto)]

Toshihiro,

it seems like the expected behaviour for the given procedure.

Rodolfo,

do you have any idea why vlan tag has been changed for you?

[IWAMOTO Toshihiro (iwamoto)]

Rodolfo, did you boot instances before restarting ovs-agent, or did you issue any neutron API while ovs-agent is down?

[OpenStack Infra (hudson-openstack)]

Change abandoned by Rodolfo Alonso Hernandez (<email address hidden>) on branch: master
Review: https://review.openstack.org/295154

[OpenStack Infra (hudson-openstack)]

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

commit c54c5791b43deada9936f0c9c5d8b681a3216893
Author: IWAMOTO Toshihiro <email address hidden>
Date: Mon Dec 5 13:28:59 2016 +0900

    ovs-agent: Clear in_port=ofport flow earlier

    This is the minimum change to pass tempest's
    test_update_instance_port_admin_state test. Alternatively, the delete_flows
    could be changed to just deal with drop_port flows, which can affect
    3rd party codes.

    Change-Id: Id15eed5f21bc6842daceb28ee9bc660f20e9b04a
    Closes-Bug: #1559920

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 11.0.0.0b2 development milestone.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/482569

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/482569
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=466ab8f3492e5fc6756a00fc2dbf6d2dfc0fa9b7
Submitter: Jenkins
Branch: stable/ocata

commit 466ab8f3492e5fc6756a00fc2dbf6d2dfc0fa9b7
Author: IWAMOTO Toshihiro <email address hidden>
Date: Mon Dec 5 13:28:59 2016 +0900

    ovs-agent: Clear in_port=ofport flow earlier

    This is the minimum change to pass tempest's
    test_update_instance_port_admin_state test. Alternatively, the delete_flows
    could be changed to just deal with drop_port flows, which can affect
    3rd party codes.

     Conflicts:
     neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py

    Change-Id: Id15eed5f21bc6842daceb28ee9bc660f20e9b04a
    Closes-Bug: #1559920
    (cherry picked from commit c54c5791b43deada9936f0c9c5d8b681a3216893)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 10.0.3 release.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/529315

[OpenStack Infra (hudson-openstack)]

Change abandoned by Max Pavlukhin (<email address hidden>) on branch: stable/ocata
Review: https://review.openstack.org/529315