[OVS][FW] Remote SG IDs left behind when a SG is removed

Hello:

I found an "easy way" to reproduce this issue. We need first to create two SGs:
- The first one without any specific rule (the default ones), "SG1"
- The other one, "SG2", accepting a custom TCP rule (ingress) from "SG1". It is important to use custom TCP rule because the way the conj_id is generated depends on this [1].

Then we need to create two networks (with one subnet per network).

Then we need to create two VMs. VM1 in net1 and SG1, VM2 in net2 and SG2. When we delete both, because of [2], some rules are still in the OVS. This is because in [2], we use the SG ID to retrieve, from "self.conj_id_map" the conj_ids. Then we use those conj_ids to clean "self.conj_id_map".

The problem we have here: in "self.conj_id_map" we store the conj_id generated in "_conj_id_factory". This conj_id is a number divisible by 8. But in "self.conj_id_map" we store the conj_id plus the priority given [3].

Than means the "sg_removed" method [2] does not clean correctly the flows for some specific ports (and the assigned IPs). If create again VMs with ports using those IP addresses, even if those VMs/ports are not assigned to SG1, they will still have a rules like:
  cookie=0x2f9dd929399d81fa, duration=3090.772s, table=82, n_packets=0, n_bytes=0, idle_age=3090,
    priority=71,ct_state=+new-est,ip,reg6=0x3,nw_src=10.2.0.29 actions=conjunction(27,1/2)
  cookie=0x2f9dd929399d81fa, duration=2916.283s, table=82, n_packets=0, n_bytes=0, idle_age=2923,
    priority=71,ct_state=+new-est,icmp,reg5=0x16 actions=conjunction(27,2/2)

That could be use by the VM with IP address 10.2.0.29 to connect to the (IP addresses/port) represented by conj_id 27.

Regards.

[1]https://github.com/openstack/neutron/blob/31280695a26cdcf211cb964ac5f401296398a19f/neutron/agent/linux/openvswitch_firewall/rules.py#L158-L174
[2]https://github.com/openstack/neutron/blob/31280695a26cdcf211cb964ac5f401296398a19f/neutron/agent/linux/openvswitch_firewall/firewall.py#L399-L422
[3]https://github.com/openstack/neutron/blob/31280695a26cdcf211cb964ac5f401296398a19f/neutron/agent/linux/openvswitch_firewall/firewall.py#L390

Related fix proposed to branch: master
Review: https://review.opendev.org/732761

Reviewed: https://review.opendev.org/732761
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=0eebd002ccda66dc6d9f9e5a254815109225e299
Submitter: Zuul
Branch: master

commit 0eebd002ccda66dc6d9f9e5a254815109225e299
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Jun 2 17:09:07 2020 +0000

    [OVS][FW] Remote SG IDs left behind when a SG is removed

    When any port in the OVS agent is using a security groups (SG) and
    this SG is removed, is marked to be deleted. This deletion process
    is done in [1].

    The SG deletion process consists on removing any reference of this SG
    from the firewall and the SG port map. The firewall removes this SG in
    [2].

    The information of a SG is stored in:
    * ConjIPFlowManager.conj_id_map = ConjIdMap(). This class stores the
      conjunction IDS (conj_ids) in a dictionary using the following keys:

        ConjIdMap.id_map[(sg_id, remote_sg_id, direction, ethertype,
          conj_ids)] = conj_id_XXX

    * ConjIPFlowManager.conj_ids is a nested dictionary, built in the
      following way:

        self.conj_ids[vlan_tag][(direction, ethertype)][remote_sg_id] = \
          set([conj_id_1, conj_id_2, ...])

    This patch stores all conjuntion IDs generated and assigned to the
    tuple (sg_id, remote_sg_id, direction, ethertype). When a SG is
    removed, the deletion method will look for this SG in the new storage
    variable created, ConjIdMap.id_map_group, and will mark all the
    conjuntion IDs related to be removed. That will cleanup those rules
    left in the OVS matching:
      action=conjunction(conj_id, 1/2)

    [1]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L731
    [2]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L399

    Change-Id: I63e446a30cf10e7bcd34a6f0d6ba1711301efcbe
    Related-Bug: #1881157

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/744104

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/744105

Reviewed: https://review.opendev.org/744104
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=ed22f7a2ff19a874bc8521f84cb4fd1c7483a23f
Submitter: Zuul
Branch: stable/ussuri

commit ed22f7a2ff19a874bc8521f84cb4fd1c7483a23f
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Jun 2 17:09:07 2020 +0000

    [OVS][FW] Remote SG IDs left behind when a SG is removed

    When any port in the OVS agent is using a security groups (SG) and
    this SG is removed, is marked to be deleted. This deletion process
    is done in [1].

    The SG deletion process consists on removing any reference of this SG
    from the firewall and the SG port map. The firewall removes this SG in
    [2].

    The information of a SG is stored in:
    * ConjIPFlowManager.conj_id_map = ConjIdMap(). This class stores the
      conjunction IDS (conj_ids) in a dictionary using the following keys:

        ConjIdMap.id_map[(sg_id, remote_sg_id, direction, ethertype,
          conj_ids)] = conj_id_XXX

    * ConjIPFlowManager.conj_ids is a nested dictionary, built in the
      following way:

        self.conj_ids[vlan_tag][(direction, ethertype)][remote_sg_id] = \
          set([conj_id_1, conj_id_2, ...])

    This patch stores all conjuntion IDs generated and assigned to the
    tuple (sg_id, remote_sg_id, direction, ethertype). When a SG is
    removed, the deletion method will look for this SG in the new storage
    variable created, ConjIdMap.id_map_group, and will mark all the
    conjuntion IDs related to be removed. That will cleanup those rules
    left in the OVS matching:
      action=conjunction(conj_id, 1/2)

    [1]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L731
    [2]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L399

    Change-Id: I63e446a30cf10e7bcd34a6f0d6ba1711301efcbe
    Related-Bug: #1881157
    (cherry picked from commit 0eebd002ccda66dc6d9f9e5a254815109225e299)

Reviewed: https://review.opendev.org/744105
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=6615f248e25a361d988db1795a3486a58f4768cb
Submitter: Zuul
Branch: stable/train

commit 6615f248e25a361d988db1795a3486a58f4768cb
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Jun 2 17:09:07 2020 +0000

    [OVS][FW] Remote SG IDs left behind when a SG is removed

    When any port in the OVS agent is using a security groups (SG) and
    this SG is removed, is marked to be deleted. This deletion process
    is done in [1].

    The SG deletion process consists on removing any reference of this SG
    from the firewall and the SG port map. The firewall removes this SG in
    [2].

    The information of a SG is stored in:
    * ConjIPFlowManager.conj_id_map = ConjIdMap(). This class stores the
      conjunction IDS (conj_ids) in a dictionary using the following keys:

        ConjIdMap.id_map[(sg_id, remote_sg_id, direction, ethertype,
          conj_ids)] = conj_id_XXX

    * ConjIPFlowManager.conj_ids is a nested dictionary, built in the
      following way:

        self.conj_ids[vlan_tag][(direction, ethertype)][remote_sg_id] = \
          set([conj_id_1, conj_id_2, ...])

    This patch stores all conjuntion IDs generated and assigned to the
    tuple (sg_id, remote_sg_id, direction, ethertype). When a SG is
    removed, the deletion method will look for this SG in the new storage
    variable created, ConjIdMap.id_map_group, and will mark all the
    conjuntion IDs related to be removed. That will cleanup those rules
    left in the OVS matching:
      action=conjunction(conj_id, 1/2)

    [1]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L731
    [2]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L399

    Conflicts:
          neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py

    Change-Id: I63e446a30cf10e7bcd34a6f0d6ba1711301efcbe
    Related-Bug: #1881157
    (cherry picked from commit 0eebd002ccda66dc6d9f9e5a254815109225e299)
    (cherry picked from commit ed22f7a2ff19a874bc8521f84cb4fd1c7483a23f)

Related fix proposed to branch: stable/stein
Review: https://review.opendev.org/747378

Related fix proposed to branch: stable/rocky
Review: https://review.opendev.org/747379

Related fix proposed to branch: stable/queens
Review: https://review.opendev.org/747381

Reviewed: https://review.opendev.org/747378
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=aaf87aeb55949a32412ffdc46089e67c1ca24d54
Submitter: Zuul
Branch: stable/stein

commit aaf87aeb55949a32412ffdc46089e67c1ca24d54
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Jun 2 17:09:07 2020 +0000

    [OVS][FW] Remote SG IDs left behind when a SG is removed

    When any port in the OVS agent is using a security groups (SG) and
    this SG is removed, is marked to be deleted. This deletion process
    is done in [1].

    The SG deletion process consists on removing any reference of this SG
    from the firewall and the SG port map. The firewall removes this SG in
    [2].

    The information of a SG is stored in:
    * ConjIPFlowManager.conj_id_map = ConjIdMap(). This class stores the
      conjunction IDS (conj_ids) in a dictionary using the following keys:

        ConjIdMap.id_map[(sg_id, remote_sg_id, direction, ethertype,
          conj_ids)] = conj_id_XXX

    * ConjIPFlowManager.conj_ids is a nested dictionary, built in the
      following way:

        self.conj_ids[vlan_tag][(direction, ethertype)][remote_sg_id] = \
          set([conj_id_1, conj_id_2, ...])

    This patch stores all conjuntion IDs generated and assigned to the
    tuple (sg_id, remote_sg_id, direction, ethertype). When a SG is
    removed, the deletion method will look for this SG in the new storage
    variable created, ConjIdMap.id_map_group, and will mark all the
    conjuntion IDs related to be removed. That will cleanup those rules
    left in the OVS matching:
      action=conjunction(conj_id, 1/2)

    [1]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L731
    [2]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L399

    Conflicts:
          neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py

    Change-Id: I63e446a30cf10e7bcd34a6f0d6ba1711301efcbe
    Related-Bug: #1881157
    (cherry picked from commit 0eebd002ccda66dc6d9f9e5a254815109225e299)
    (cherry picked from commit ed22f7a2ff19a874bc8521f84cb4fd1c7483a23f)
    (cherry picked from commit 6615f248e25a361d988db1795a3486a58f4768cb)

Reviewed: https://review.opendev.org/747379
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=abeda5aecef31dfc190e97c770c3a3c8aa40a168
Submitter: Zuul
Branch: stable/rocky

commit abeda5aecef31dfc190e97c770c3a3c8aa40a168
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Jun 2 17:09:07 2020 +0000

    [OVS][FW] Remote SG IDs left behind when a SG is removed

    When any port in the OVS agent is using a security groups (SG) and
    this SG is removed, is marked to be deleted. This deletion process
    is done in [1].

    The SG deletion process consists on removing any reference of this SG
    from the firewall and the SG port map. The firewall removes this SG in
    [2].

    The information of a SG is stored in:
    * ConjIPFlowManager.conj_id_map = ConjIdMap(). This class stores the
      conjunction IDS (conj_ids) in a dictionary using the following keys:

        ConjIdMap.id_map[(sg_id, remote_sg_id, direction, ethertype,
          conj_ids)] = conj_id_XXX

    * ConjIPFlowManager.conj_ids is a nested dictionary, built in the
      following way:

        self.conj_ids[vlan_tag][(direction, ethertype)][remote_sg_id] = \
          set([conj_id_1, conj_id_2, ...])

    This patch stores all conjuntion IDs generated and assigned to the
    tuple (sg_id, remote_sg_id, direction, ethertype). When a SG is
    removed, the deletion method will look for this SG in the new storage
    variable created, ConjIdMap.id_map_group, and will mark all the
    conjuntion IDs related to be removed. That will cleanup those rules
    left in the OVS matching:
      action=conjunction(conj_id, 1/2)

    [1]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L731
    [2]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L399

    Conflicts:
          neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py

    Change-Id: I63e446a30cf10e7bcd34a6f0d6ba1711301efcbe
    Related-Bug: #1881157
    (cherry picked from commit 0eebd002ccda66dc6d9f9e5a254815109225e299)
    (cherry picked from commit ed22f7a2ff19a874bc8521f84cb4fd1c7483a23f)
    (cherry picked from commit 6615f248e25a361d988db1795a3486a58f4768cb)

Reviewed: https://review.opendev.org/747381
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=d0b696f076a94830b040ffcb2dec28e2eabe2e04
Submitter: Zuul
Branch: stable/queens

commit d0b696f076a94830b040ffcb2dec28e2eabe2e04
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Jun 2 17:09:07 2020 +0000

    [OVS][FW] Remote SG IDs left behind when a SG is removed

    When any port in the OVS agent is using a security groups (SG) and
    this SG is removed, is marked to be deleted. This deletion process
    is done in [1].

    The SG deletion process consists on removing any reference of this SG
    from the firewall and the SG port map. The firewall removes this SG in
    [2].

    The information of a SG is stored in:
    * ConjIPFlowManager.conj_id_map = ConjIdMap(). This class stores the
      conjunction IDS (conj_ids) in a dictionary using the following keys:

        ConjIdMap.id_map[(sg_id, remote_sg_id, direction, ethertype,
          conj_ids)] = conj_id_XXX

    * ConjIPFlowManager.conj_ids is a nested dictionary, built in the
      following way:

        self.conj_ids[vlan_tag][(direction, ethertype)][remote_sg_id] = \
          set([conj_id_1, conj_id_2, ...])

    This patch stores all conjuntion IDs generated and assigned to the
    tuple (sg_id, remote_sg_id, direction, ethertype). When a SG is
    removed, the deletion method will look for this SG in the new storage
    variable created, ConjIdMap.id_map_group, and will mark all the
    conjuntion IDs related to be removed. That will cleanup those rules
    left in the OVS matching:
      action=conjunction(conj_id, 1/2)

    [1]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L731
    [2]https://github.com/openstack/neutron/blob/118930f03d31f157f8c7a9e6c57122ecea8982b9/neutron/agent/linux/openvswitch_firewall/firewall.py#L399

    Conflicts:
        neutron/agent/linux/openvswitch_firewall/firewall.py
        neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py

    Change-Id: I63e446a30cf10e7bcd34a6f0d6ba1711301efcbe
    Related-Bug: #1881157
    (cherry picked from commit 0eebd002ccda66dc6d9f9e5a254815109225e299)
    (cherry picked from commit ed22f7a2ff19a874bc8521f84cb4fd1c7483a23f)
    (cherry picked from commit 6615f248e25a361d988db1795a3486a58f4768cb)

As things stand, this is already available in Ussuri uca (16.2.0), and will be available in the upcoming 15.2.0 Train uca point release. Stein and Queens have no existing upstream tag that contains the fix so will require an SRU.

(for Ubuntu ^^)

Sorry I meant Ussuri 16.0.1 above

The attachment "lp1881157-stein.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

The stein patch is already in stein-proposed via https://bugs.launchpad.net/cloud-archive/+bug/1904563

This change has now landed into stein-updates via https://bugs.launchpad.net/cloud-archive/+bug/1904563

A new version of neutron has been uploaded to the bionic unapproved queue:
https://launchpad.net/ubuntu/bionic/+queue?queue_state=1&queue_text=neutron

Hello Rodolfo, or anyone else affected,

Accepted neutron into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/neutron/2:12.1.1-0ubuntu3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Hello Rodolfo, or anyone else affected,

Accepted neutron into queens-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:queens-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-queens-needed to verification-queens-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-queens-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

If anyone has cycles to test this on bionic-proposed and queens-proposed it would be greatly appreciated!

Verified bionic-proposed using [Test Case] and the output is here - https://pastebin.ubuntu.com/p/NrGdcJb9fx/

Verified xenial-queens-proposed using [Test Case] and the output is here -

https://pastebin.ubuntu.com/p/gVvDNrYnwY/

NOTE: that I hit an issue due to the python-oslo.messaging sru from bug 1881157 also in proposed. I had to downgrade to the version from xenial-queens/updates to get it working again.

Typo in PB above, should have this for post-upgrade (copy and paste error):

# dpkg -l| grep neutron-common
ii neutron-common 2:12.1.1-0ubuntu3~cloud0 all Neutron is a virtual network service for Openstack - common

The verification of the Stable Release Update for neutron has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package neutron - 2:12.1.1-0ubuntu3

---------------
neutron (2:12.1.1-0ubuntu3) bionic; urgency=medium

  [ Chris MacNaughton ]
  * d/control: Update VCS paths for move to lp:~ubuntu-openstack-dev.

  [ Corey Bryant ]
  * d/p/ovs-fw-remote-sg-ids-left-behind.patch: Cherry-picked from upstream
    stable/queens to ensure proper cleanup of remote security group IDs when
    a security group is removed (LP: #1881157).

 -- Corey Bryant <email address hidden> Tue, 24 Nov 2020 10:33:03 -0500

The verification of the Stable Release Update for neutron has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package neutron - 2:12.1.1-0ubuntu3~cloud0
---------------

 neutron (2:12.1.1-0ubuntu3~cloud0) xenial-queens; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 neutron (2:12.1.1-0ubuntu3) bionic; urgency=medium
 .
   [ Chris MacNaughton ]
   * d/control: Update VCS paths for move to lp:~ubuntu-openstack-dev.
 .
   [ Corey Bryant ]
   * d/p/ovs-fw-remote-sg-ids-left-behind.patch: Cherry-picked from upstream
     stable/queens to ensure proper cleanup of remote security group IDs when
     a security group is removed (LP: #1881157).