[ovn] enable_snat cannot be disabled once enabled

Bug #1922089 reported by Junien Fridrick
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
High
Lucas Alvares Gomes
neutron (Ubuntu)
Medium
Unassigned

Bug Description

Hi,

Using Openstack focal/ussuri - ovn version 20.03.1-0ubuntu1.2 and neutron 2:16.2.0-0ubuntu2.

If "enable_snat" is enabled on an external gateway on a router, it's not possible to disable it without completely removing said gateway from the router.

For example :
I have a subnet called subnet_axino_test - 10.0.100.0/24
I run the following :

$ openstack router create router_axino_test
$ openstack router set --disable-snat --external-gateway net_stg-external router_axino_test
$ openstack router add subnet router_axino_test subnet_axino_test

And so on OVN, I get nothing :
$ sudo ovn-nbctl list NAT |grep -B5 -A4 10.131.100.0/24

Now, I enable SNAT :
$ openstack router set --enable-snat --external-gateway net_stg-external router_axino_test

This correctly adds an OVN SNAT entry as follows :
$ sudo ovn-nbctl list NAT |grep -B5 -A4 10.131.100.0/24

_uuid : a65cc4b8-14ae-4ce4-b274-10eefdcc51dc
external_ids : {}
external_ip : "A.B.C.D"
external_mac : []
logical_ip : "10.131.100.0/24"
logical_port : []
options : {}
type : snat

Now, I remove SNAT from the router :
$ openstack router set --disable-snat --external-gateway net_stg-external router_axino_test

I confirm this :
$ openstack router show router_axino_test | grep enable_snat
| external_gateway_info | {"network_id": "4fb8304e-7adb-4cc3-bae5-deb968263eb0", "external_fixed_ips": [{"subnet_id": "60000d47-1e44-41af-8f64-dd802d5c3ddc", "ip_address": "A.B.C.D"}], "enable_snat": false} |

Above, you can see that "enable_snat" is "false". So I would expect OVN to _not_ have a NAT entry. Yet, it does :
$ sudo ovn-nbctl list NAT |grep -B5 -A4 10.131.100.0/24

_uuid : a65cc4b8-14ae-4ce4-b274-10eefdcc51dc
external_ids : {}
external_ip : "162.213.34.141"
external_mac : []
logical_ip : "10.131.100.0/24"
logical_port : []
options : {}
type : snat

The only way to remove SNAT is to completely remove the external gateway from the router, and to re-add it with SNAT disabled :
$ openstack router unset --external-gateway router_axino_test
$ openstack router set --disable-snat --external-gateway net_stg-external router_axino_test

Note that this requires removing all the floating IPs from VMs behind this router, which obviously makes them unreachable - which is less than ideal in production.

Thanks

tags: added: ovn
Revision history for this message
Bence Romsics (bence-romsics) wrote :

Sorry for the slow reaction. Thanks for the bug report. I was able to reproduce the bug by your instructions - with traffic too.

Changed in neutron:
status: New → Triaged
importance: Undecided → High
Changed in neutron:
assignee: nobody → Lucas Alvares Gomes (lucasagomes)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/788485

Changed in neutron:
status: Triaged → In Progress
Revision history for this message
Lucas Alvares Gomes (lucasagomes) wrote :

Hi,

Thanks for reporting it, I was looking into the problem and saw that we are actually checking if the snat is enabled in order to call update_nat_rules() in the update_router() method. I've fixed the problem at https://review.opendev.org/c/openstack/neutron/+/788485

I tested it locally (http://paste.openstack.org/show/804827/) but it would be good if you also could test and review the patch.

Thank you

Revision history for this message
Junien Fridrick (axino) wrote :

Sorry I don't have access to a lab where I can patch neutron, I'm not going to be able to test the patch.

Revision history for this message
Lucas Alvares Gomes (lucasagomes) wrote :

Hi,

OK, I was able to reproduce the problem locally in devstack as stated in comment #3. The patch I proposed does fix it for me, so I am somewhat confident that it will also fix for you, but wanted to be sure.

If you can take a look at the patch itself, the code is quite small.

Thanks again for reporting

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in neutron (Ubuntu):
status: New → Confirmed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/788485
Committed: https://opendev.org/openstack/neutron/commit/ddc8e625f714aebd65a454f23fedc6d9a9320e89
Submitter: "Zuul (22348)"
Branch: master

commit ddc8e625f714aebd65a454f23fedc6d9a9320e89
Author: Lucas Alvares Gomes <email address hidden>
Date: Wed Apr 28 13:41:15 2021 +0100

    [OVN] Fix: Disabling snat after it was enabled

    This patch removes a conditional check in the update_router() method
    which was verifying if snat was enabled in order to update the nat
    rules. This check does not make sense in the update method as if snat
    was disabled we should still call update_nat_rules() which will then
    remove the NAT entry from the OVN NB DB.

    Change-Id: Ice20d22365acaf33ee211b1e38b7d0bc151c1ba8
    Closes-Bug: #1922089
    Signed-off-by: Lucas Alvares Gomes <email address hidden>

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (stable/ussuri)

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/neutron/+/792536

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

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/neutron/+/792537

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

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/neutron/+/792538

Mathew Hodson (mhodson)
Changed in neutron (Ubuntu):
importance: Undecided → Medium
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (stable/victoria)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/792537
Committed: https://opendev.org/openstack/neutron/commit/749d9d6fca70aef005d518e1e4bee183ddd724b1
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 749d9d6fca70aef005d518e1e4bee183ddd724b1
Author: Lucas Alvares Gomes <email address hidden>
Date: Wed Apr 28 13:41:15 2021 +0100

    [OVN] Fix: Disabling snat after it was enabled

    This patch removes a conditional check in the update_router() method
    which was verifying if snat was enabled in order to update the nat
    rules. This check does not make sense in the update method as if snat
    was disabled we should still call update_nat_rules() which will then
    remove the NAT entry from the OVN NB DB.

    Change-Id: Ice20d22365acaf33ee211b1e38b7d0bc151c1ba8
    Closes-Bug: #1922089
    Signed-off-by: Lucas Alvares Gomes <email address hidden>
    (cherry picked from commit ddc8e625f714aebd65a454f23fedc6d9a9320e89)

tags: added: in-stable-victoria
tags: added: in-stable-ussuri
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (stable/ussuri)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/792536
Committed: https://opendev.org/openstack/neutron/commit/7e1df72c16f2e1736b068387e76f5b9070d6d5bf
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 7e1df72c16f2e1736b068387e76f5b9070d6d5bf
Author: Lucas Alvares Gomes <email address hidden>
Date: Wed Apr 28 13:41:15 2021 +0100

    [OVN] Fix: Disabling snat after it was enabled

    This patch removes a conditional check in the update_router() method
    which was verifying if snat was enabled in order to update the nat
    rules. This check does not make sense in the update method as if snat
    was disabled we should still call update_nat_rules() which will then
    remove the NAT entry from the OVN NB DB.

    Change-Id: Ice20d22365acaf33ee211b1e38b7d0bc151c1ba8
    Closes-Bug: #1922089
    Signed-off-by: Lucas Alvares Gomes <email address hidden>
    (cherry picked from commit ddc8e625f714aebd65a454f23fedc6d9a9320e89)

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

Reviewed: https://review.opendev.org/c/openstack/neutron/+/792538
Committed: https://opendev.org/openstack/neutron/commit/8c6f7745041dca8eb4568c255e2033bf54ee9fe5
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 8c6f7745041dca8eb4568c255e2033bf54ee9fe5
Author: Lucas Alvares Gomes <email address hidden>
Date: Wed Apr 28 13:41:15 2021 +0100

    [OVN] Fix: Disabling snat after it was enabled

    This patch removes a conditional check in the update_router() method
    which was verifying if snat was enabled in order to update the nat
    rules. This check does not make sense in the update method as if snat
    was disabled we should still call update_nat_rules() which will then
    remove the NAT entry from the OVN NB DB.

    Change-Id: Ice20d22365acaf33ee211b1e38b7d0bc151c1ba8
    Closes-Bug: #1922089
    Signed-off-by: Lucas Alvares Gomes <email address hidden>
    (cherry picked from commit ddc8e625f714aebd65a454f23fedc6d9a9320e89)

tags: added: in-stable-wallaby
tags: added: neutron-proactive-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 16.4.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 17.2.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 18.1.0

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

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers