[dvr] bound port permanent arp entries never deleted

[Edward Hope-Morley]

[Impact]

See original bug desription but in short commit b3a42cddc5 removed all the arp management code in favour of using the arp_reponder but missed the fact that DVR floating ips don't use the arp_responder. As a result it was possible to end up with permanent arp entries in qrouter namespaces such that if you created a new port with the same IP as that of a previous port for which there is an arp entry, associating a fip with that port would never be accessible until that arp entry was manually deleted. This patch adds the reverted code back in.

[Test Plan]

  * deploy Openstack Train/Ussuri/Victoria
  * create port P1 with address A1 and create vm on node C1 with this port
  * associate floating ip with P1 and ping it
  * observe REACHABLE or PERMANENT arp entry for A1 in qrouter arp cache
  * delete vm and port
  * ensure arp entry for A1 in qrouter arp cache is deleted
  * create port P2 with address A1 and create vm on node C1 with this port
  * associate floating ip with P2 and ping it

[Where problems could occur]

No problems anticipated from re-introducing this code. Of course this code uses RPC notifications and as a result will incur some extra amqp load but is not anticipated to be a problem and it was not considered a problem when the code existed prior to removal.

--------------------------------------------------------------------------

With Openstack Ussuri using dvr-snat I do the following:

  * create port P1 with address A1 and create vm on node C1 with this port
  * associate floating ip with P1 and ping it
  * observe REACHABLE arp entry for A1 in qrouter arp cache
  * so far so good
  * restart the neutron-l3-agent
  * observe REACHABLE arp entry for A1 is now PERMANENT
  * delete vm and port
  * create port P2 with address A1 and create vm on node C1 with this port
  * vm is unreachable since arp cache contains PERMANENT entry for old port P1 mac/ip combo

If I don't restart the l3-agent, once I have deleted the port it's arp entry does REACHABLE -> STALE and will either be replaced or timeout as expected but once it is set to PERMANENT it will never disappear which means any future use of that ip address (by a port with a different mac) will not work until that entry is manually deleted.

[LIU Yulong (dragon889)]

Confirmed, I can reproduce the behavior of PERMANENT arp even the agent is not restarted.

[Edward Hope-Morley (hopem)]

I think there are multiple issues at play here.

1. if bind a port to a vm, the l3-agent does not create an arp entry for that port at all
2. if i restart the l3-agent it creates/updates arp entry for the port to permanent
3. if i unbind/delete the port the l3-agent does not perform any cleanup

[Edward Hope-Morley (hopem)]

One more thing is that this mostly impacts access via floating ips since arp entry is used post DNAT to get the dst mac ip address of the vm private ip. One quick/interim fix could be to ensure an arp update when a fip is configured to ensure the entry is correct.

[LIU Yulong (dragon889)]

The issue is the final "delete_neigh_entry" for the port IP was not executed.
So seems we have some issues on:
1. The "del_arp_entry" fanout RPC does not be processed by some l3-agents related to the test router. So the PERMANENT arp remains.
2. The port delete event does not triggered the "l3plugin.delete_arp_entry_for_dvr_service_port"

But no matter of these cases, I have a solution is to "replace the arp entry" rather than "add". For the "add" action, we maybe cloud remove the entry first (to clean the stale one), then re-add it. Then the ARP entry for the IP will have the new MAC. In such way, the bug can be killed.

[LIU Yulong (dragon889)]

Something like this:

# ip netns exec qrouter-b247f145-569a-4d5a-bdd8-31a5213641ea ip -4 neigh|grep "192.168.111.250"
192.168.111.250 dev qr-7a331d0a-1c lladdr fa:16:3e:99:6e:41 PERMANENT

Use "replace" instead of "add":
# ip netns exec qrouter-b247f145-569a-4d5a-bdd8-31a5213641ea ip -4 neigh replace 192.168.111.250 dev qr-7a331d0a-1c lladdr fa:16:3e:99:6e:42

# ip netns exec qrouter-b247f145-569a-4d5a-bdd8-31a5213641ea ip -4 neigh|grep "192.168.111.250"
192.168.111.250 dev qr-7a331d0a-1c lladdr fa:16:3e:99:6e:42 PERMANENT

[Edward Hope-Morley (hopem)]

@dragon889 the arp entry should be deleted when the port is unbound not just deleted and also we should have a way to fix existing entries.

[LIU Yulong (dragon889)]

@Edward Hope-Morley (hopem)

The issue you mentioned:
"1. if bind a port to a vm, the l3-agent does not create an arp entry for that port at all"

is similar to this
https://bugs.launchpad.net/neutron/+bug/1913621

For now, I have not reproduced the issue successfully.

For these 2 issue:
"""
2. if i restart the l3-agent it creates/updates arp entry for the port to permanent
3. if i unbind/delete the port the l3-agent does not perform any cleanup
"""
I will upload a patch based on https://bugs.launchpad.net/neutron/+bug/1916761/comments/5. Hope it will help to fix the issue.

[LIU Yulong (dragon889)]

My bad...
It is "replace" for arp entry add action:
https://github.com/openstack/neutron/blob/master/neutron/privileged/agent/linux/ip_lib.py#L470

[LIU Yulong (dragon889)]

So the arp entry is replaced to new mac based on the current code, so I need to revisit these statements:
"""
  * create port P2 with address A1 and create vm on node C1 with this port
  * vm is unreachable since arp cache contains PERMANENT entry for old port P1 mac/ip combo
"""

[Edward Hope-Morley (hopem)]

I'm definitely only seeing permanent entries after a restart in Ussuri. I will try V and W to see how they behave. Note that in my Ussuri test there is an arp entry for the port but it isn't added by the agent (so it is marked as reachable not permanent).

[Edward Hope-Morley (hopem)]

My current workaround for this problem is:

  * restart neutron-l3-agent to fix arp entries for any existing ports bound on that host
  * update all arp entries for vm ports to be REACHABLE so that when they are unbound the entry wil be allows to expire or be replaced by future arp updates.

[LIU Yulong (dragon889)]

Set the state to "new" temporarily, since I have not reproduce this again.

[Edward Hope-Morley (hopem)]

@dragon88 I have just tested with the Victoria release and can confirm I see the same behaviour. There is one caveat for Victoria and above which may be the reason why you are having trouble reproducing which is that if you create just once instance, when you delete it neutron will also tear down the qrouter ns and therefore the arp cache within such that when you create a new instance the old arp cache/entries are no longer there. So for Victoria I had to create two instances and use the second to perform my test i.e. create two vms, the second with port P1 with address A1, restart l3-agent, observe two permanent entries (one for each vm port), delete vm2 and port P1, create new vm and port with address A1 on same node and add floating ip, can no longer access fip and confirmed that arp cache contains permanent entry with old mac address.

[Edward Hope-Morley (hopem)]

The root cause of this problem is that the del_arp_entry() notifications are currently unused. I've started working on a patch to fix this at https://review.opendev.org/c/openstack/neutron/+/777616

[Edward Hope-Morley (hopem)]

This behaviour was introduced by https://review.opendev.org/q/I538aa6d68fbb5ff8431f82ba76601ee34c1bb181 which is backported as far as Train. I confirmed this by testing Stein which does not suffer from this problem. The patch in question removes the arp update code since as of Train the arp_responder is enabled by default. The problem is that floating ips dont use the arp_responder since the NAT occurs in the qrouter namespace and therefore the arp cache in the qrouter ns is used. That is why this bug breaks fip traffic but not east-west traffic. The way I see it there are a few ways to fix this; (a) revert the code that removes the arp updates (b) remove the code from the l3-agent that sets PERMANENT entries (c) use my patch as a minimal implementation of the patch that was reverted.

[Edward Hope-Morley (hopem)]

Revert is being done here - https://review.opendev.org/c/openstack/neutron/+/777903

[Corey Bryant (corey.bryant)]

New Ubuntu package versions have been uploaded to groovy and focal unapproved queues as well as trains-staging for the cloud archive.

[Launchpad Janitor (janitor)]

This bug was fixed in the package neutron - 2:17.1.0+git2021012815.0fb63f7297-0ubuntu4

---------------
neutron (2:17.1.0+git2021012815.0fb63f7297-0ubuntu4) hirsute; urgency=medium

  * d/p/revert-dvr-remove-control-plane-arp-updates.patch: Cherry-picked
    from https://review.opendev.org/c/openstack/neutron/+/777903 to prevent
    permanent arp entries that never get deleted (LP: #1916761).

 -- Corey Bryant <email address hidden> Mon, 08 Mar 2021 10:49:14 -0500

[Łukasz Zemczak (sil2100)]

Hello Edward, or anyone else affected,

Accepted neutron into groovy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/neutron/2:17.1.0-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-groovy to verification-done-groovy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-groovy. 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.

[Łukasz Zemczak (sil2100)]

Hello Edward, or anyone else affected,

Accepted neutron into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/neutron/2:16.3.0-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-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. 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.

[Corey Bryant (corey.bryant)]

Hello Edward, or anyone else affected,

Accepted neutron into victoria-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:victoria-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-victoria-needed to verification-victoria-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-victoria-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!

[Corey Bryant (corey.bryant)]

Hello Edward, or anyone else affected,

Accepted neutron into ussuri-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:ussuri-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-ussuri-needed to verification-ussuri-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-ussuri-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!

[Corey Bryant (corey.bryant)]

Hello Edward, or anyone else affected,

Accepted neutron into train-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:train-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-train-needed to verification-train-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-train-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!

[Edward Hope-Morley (hopem)]

Verified Groovy using [Test Plan] and the output is as follows:

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

# apt-cache policy neutron-common
neutron-common:
  Installed: 2:17.1.0-0ubuntu3
  Candidate: 2:17.1.0-0ubuntu3
  Version table:
 *** 2:17.1.0-0ubuntu3 500
        500 http://archive.ubuntu.com/ubuntu groovy-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     2:17.0.0-0ubuntu3 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu groovy-updates/main amd64 Packages
     2:17.0.0-0ubuntu1 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu groovy/main amd64 Packages

[Edward Hope-Morley (hopem)]

Verified Focal Victoria (UCA) using [Test Plan] and the output is as follows:

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

# apt-cache policy neutron-common
neutron-common:
  Installed: 2:17.1.0-0ubuntu3~cloud0
  Candidate: 2:17.1.0-0ubuntu3~cloud0
  Version table:
 *** 2:17.1.0-0ubuntu3~cloud0 500
        500 http://ubuntu-cloud.archive.canonical.com/ubuntu focal-proposed/victoria/main amd64 Packages
        100 /var/lib/dpkg/status
     2:16.2.0-0ubuntu3 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu focal-updates/main amd64 Packages
     2:16.0.0~b3~git2020041516.5f42488a9a-0ubuntu2 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu focal/main amd64 Packages

[Edward Hope-Morley (hopem)]

Verified Focal Ussuri (UCA) using [Test Plan] and the output is as follows:

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

# apt-cache policy neutron-common
neutron-common:
  Installed: 2:16.3.0-0ubuntu3
  Candidate: 2:16.3.0-0ubuntu3
  Version table:
 *** 2:16.3.0-0ubuntu3 500
        500 http://archive.ubuntu.com/ubuntu focal-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     2:16.2.0-0ubuntu3 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu focal-updates/main amd64 Packages
     2:16.0.0~b3~git2020041516.5f42488a9a-0ubuntu2 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu focal/main amd64 Packages

[Brian Murray (brian-murray)]

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.

[Launchpad Janitor (janitor)]

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

---------------
neutron (2:16.3.0-0ubuntu3) focal; urgency=medium

  * d/p/revert-dvr-remove-control-plane-arp-updates.patch: Cherry-picked
    from https://review.opendev.org/c/openstack/neutron/+/777903 to prevent
    permanent arp entries that never get deleted (LP: #1916761).
  * d/p/improve-get-devices-with-ip-performance.patch: Performance of
    get_devices_with_ip is improved to limit the amount of information
    to be sent and reduce the number of syscalls. (LP: #1896734).

neutron (2:16.3.0-0ubuntu2) focal; urgency=medium

  * Backport fix for dvr-snat missig rfp interfaces (LP: #1894843)
    - d/p/0001-Fix-deletion-of-rfp-interfaces-when-router-is-re-ena.patch

neutron (2:16.3.0-0ubuntu1) focal; urgency=medium

  * d/watch: Add trailing slash to Neutron URL.
  * New stable point release for OpenStack Ussuri (LP: #1915786).
  * d/p/fix-removal-of-dvr-src-mac-flows.patch,
    d/p/ovn-fix-inconsistent-igmp-configuration.patch: Removed after patch landed
    upstream.

 -- Corey Bryant <email address hidden> Mon, 08 Mar 2021 13:26:42 -0500

[Launchpad Janitor (janitor)]

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

---------------
neutron (2:17.1.0-0ubuntu3) groovy; urgency=medium

  * d/p/revert-dvr-remove-control-plane-arp-updates.patch: Cherry-picked
    from https://review.opendev.org/c/openstack/neutron/+/777903 to prevent
    permanent arp entries that never get deleted (LP: #1916761).
  * d/p/improve-get-devices-with-ip-performance.patch: Performance of
    get_devices_with_ip is improved to limit the amount of information
    to be sent and reduce the number of syscalls. (LP: #1896734).

neutron (2:17.1.0-0ubuntu2) groovy; urgency=medium

  * Backport fix for dvr-snat missig rfp interfaces (LP: #1894843)
    - d/p/0001-Fix-deletion-of-rfp-interfaces-when-router-is-re-ena.patch

neutron (2:17.1.0-0ubuntu1) groovy; urgency=medium

  * d/watch: Fix typo in watch URL, add trailing slash.
  * New stable point release for OpenStack Victoria (LP: #1915785).
  * d/p/fix-removal-of-dvr-src-mac-flows.patch,
    d/p/ovn-fix-inconsistent-igmp-configuration.patch: Removed after fix landed
    upstream.

 -- Corey Bryant <email address hidden> Mon, 08 Mar 2021 13:00:21 -0500

[Chris MacNaughton (chris.macnaughton)]

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.

[Chris MacNaughton (chris.macnaughton)]

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

 neutron (2:17.1.0-0ubuntu3~cloud0) focal-victoria; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 neutron (2:17.1.0-0ubuntu3) groovy; urgency=medium
 .
   * d/p/revert-dvr-remove-control-plane-arp-updates.patch: Cherry-picked
     from https://review.opendev.org/c/openstack/neutron/+/777903 to prevent
     permanent arp entries that never get deleted (LP: #1916761).
   * d/p/improve-get-devices-with-ip-performance.patch: Performance of
     get_devices_with_ip is improved to limit the amount of information
     to be sent and reduce the number of syscalls. (LP: #1896734).

[Corey Bryant (corey.bryant)]

This bug was fixed in the package neutron - 2:17.1.0+git2021012815.0fb63f7297-0ubuntu4~cloud0
---------------

 neutron (2:17.1.0+git2021012815.0fb63f7297-0ubuntu4~cloud0) focal-wallaby; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 neutron (2:17.1.0+git2021012815.0fb63f7297-0ubuntu4) hirsute; urgency=medium
 .
   * d/p/revert-dvr-remove-control-plane-arp-updates.patch: Cherry-picked
     from https://review.opendev.org/c/openstack/neutron/+/777903 to prevent
     permanent arp entries that never get deleted (LP: #1916761).

[Edward Hope-Morley (hopem)]

Verified Bionic Ussuri (UCA) using [Test Plan] and the output is as follows:

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

$ apt-cache policy neutron-common
neutron-common:
  Installed: 2:16.3.0-0ubuntu3~cloud0
  Candidate: 2:16.3.0-0ubuntu3~cloud0
  Version table:
 *** 2:16.3.0-0ubuntu3~cloud0 500
        500 http://ubuntu-cloud.archive.canonical.com/ubuntu bionic-proposed/ussuri/main amd64 Packages
        100 /var/lib/dpkg/status
     2:12.1.1-0ubuntu3 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu bionic-updates/main amd64 Packages
     2:12.0.1-0ubuntu1 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu bionic/main amd64 Packages

[Edward Hope-Morley (hopem)]

Verified Bionic Train (uca) using [Test Plan] and the output is as follows:

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

$ apt-cache policy neutron-common
neutron-common:
  Installed: 2:15.3.2-0ubuntu1~cloud1
  Candidate: 2:15.3.2-0ubuntu1~cloud1
  Version table:
 *** 2:15.3.2-0ubuntu1~cloud1 500
        500 http://ubuntu-cloud.archive.canonical.com/ubuntu bionic-proposed/train/main amd64 Packages
        100 /var/lib/dpkg/status
     2:12.1.1-0ubuntu3 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu bionic-updates/main amd64 Packages
     2:12.0.1-0ubuntu1 500
        500 http://nova.clouds.archive.ubuntu.com/ubuntu bionic/main amd64 Packages

[Chris MacNaughton (chris.macnaughton)]

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.

[Chris MacNaughton (chris.macnaughton)]

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

 neutron (2:16.3.0-0ubuntu3~cloud0) bionic-ussuri; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 neutron (2:16.3.0-0ubuntu3) focal; urgency=medium
 .
   * d/p/revert-dvr-remove-control-plane-arp-updates.patch: Cherry-picked
     from https://review.opendev.org/c/openstack/neutron/+/777903 to prevent
     permanent arp entries that never get deleted (LP: #1916761).
   * d/p/improve-get-devices-with-ip-performance.patch: Performance of
     get_devices_with_ip is improved to limit the amount of information
     to be sent and reduce the number of syscalls. (LP: #1896734).

[Chris MacNaughton (chris.macnaughton)]

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.

[Chris MacNaughton (chris.macnaughton)]

This bug was fixed in the package neutron - 2:15.3.2-0ubuntu1~cloud1
---------------

 neutron (2:15.3.2-0ubuntu1~cloud1) bionic-train; urgency=medium
 .
   * d/p/revert-dvr-remove-control-plane-arp-updates.patch: Cherry-picked
     from https://review.opendev.org/c/openstack/neutron/+/777903 to prevent
     permanent arp entries that never get deleted (LP: #1916761).

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/806771

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Eduardo Olivares <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/806771
Reason: In order to reproduce the bug, the neutron-l3-agent from the compute should be restarted before the first server deletion.
This is not supported by neutron-tempest-plugin, so I'm abandoning this patch and will try to create a dowsntream test to cover this bug