[L3][QoS] FIP QoS extension cannot handle duplicated FIP addresses

[Rodolfo Alonso]

The classes "RouterFipRateLimitMaps" and "RateLimitMaps" (the child and parent classes), cannot handle the case when two FIP addresses, from different networks and routers, are the same.

Steps to reproduce:
- It is needed a compute node with access to two public networks (for example, two VLAN networks connected to the same physical network).
- Each network will have one subnet, both with the same CIDR.
- Create private networks, two routers and connect each router to a private and a public network.
- Create two FIPs, each one in a different public network. Both FIPs must have the same IP address.
- Create two QoS policies with one max-bw rule.
- Assign each QoS policy to each FIP
--> the members to store the QoS information in the related classes use the FIP address (the IP address string) to identify uniquely the FIP resources. With the explained reproducer, the second FIP will overwrite the first one.

For example, if the both QoS policies are detached from the FIPs, we'll have the following message in the L3 agent:
DEBUG neutron.agent.l3.extensions.qos.base [-] L3 QoS extension did not have information on floating IP 192.168.20.202 {{(pid=213406) _clean_by_resource /opt/stack/neutron/neutron/agent/l3/extensions/qos/base.py:129}}

This is because when the first FIP address is removed from "RouterFipRateLimitMaps.qos_policy_resources", the second one cannot be find.

A new way to identify unambiguously the FIPs in the L3 agent QoS extension is needed.

[OpenStack Infra (hudson-openstack)]

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

[Brian Haley (brian-haley)]

Hi Rodolfo, can you explain a little why having two public networks with the same subnet CIDR configured is a good idea? Just doesn't seem like a common case. Not saying this is not a bug, just trying to add more data here so I can choose a priority. Thanks.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Hello Brian:

I have no idea if this is a good idea but it is a valid configuration. Following the steps, this could be reproduced in 2 minutes. I found it by mistake when implementing [1]. Before continuing with this new feature we need to fix this error.

Regards.

[1]https://review.opendev.org/c/openstack/neutron/+/819147

[LIU Yulong (dragon889)]

https://review.opendev.org/c/openstack/neutron/+/831238/3//COMMIT_MSG#19
IMO, such resource configuration should be disabled in API layer.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Please, provide a reason to block this from the API layer.

[Lajos Katona (lajos-katona)]

I agree that this sounds strange, and mot sure what other issues we introduce if we allow such setup where 2 public network cidrs overlap.

Perhaps better to clearly document it, to avoid such setup, and list what should not work as expected, but keep the API as it is.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Those two networks doesn't need to be connected, same as the private networks routed to them. You can have two completely separate environments (private networks connected to a router with access to an external network) in the same compute. Actually we have this in our clouds for learning purposes.

Instead of documenting an artificial limitation that does not exits, we can fix what is broken in the QoS extension in a single patch https://review.opendev.org/c/openstack/neutron/+/831238.

[Lajos Katona (lajos-katona)]

Based on last weeks discussion during the drivers meeting (see: https://meetings.opendev.org/meetings/neutron_drivers/2022/neutron_drivers.2022-03-04-14.03.log.html#l-76 ) we agreed to continue with this.

[OpenStack Infra (hudson-openstack)]

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

commit 132905d81f8799f249936ecb9d56641749df8724
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Mon Feb 28 17:56:33 2022 +0000

    [L3][QoS] L3 agent QoS extension to handle duplicated FIPs

    The L3 agent QoS extension should be able to handle duplicated floating
    IP addresses from different registers. Those floating IP addresses can
    be hosted in different networks and routers but the same compute node
    and L3 agent.

    Now, instead of using the IP address as unique identifier, this patch
    stores the (ID, IP address) tuple. This tuple is unique per floating IP
    register and holds the IP address, needed by the "tc" library to set
    the QoS rules.

    Closes-Bug: #1962465
    Change-Id: I3a09718c6e79fd1f37eb75264389c4e5043076a7

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/837087
Committed: https://opendev.org/openstack/neutron/commit/0c18f80f5c3e0dde899adc45441186c08c77ea9b
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 0c18f80f5c3e0dde899adc45441186c08c77ea9b
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Mon Feb 28 17:56:33 2022 +0000

    [L3][QoS] L3 agent QoS extension to handle duplicated FIPs

    The L3 agent QoS extension should be able to handle duplicated floating
    IP addresses from different registers. Those floating IP addresses can
    be hosted in different networks and routers but the same compute node
    and L3 agent.

    Now, instead of using the IP address as unique identifier, this patch
    stores the (ID, IP address) tuple. This tuple is unique per floating IP
    register and holds the IP address, needed by the "tc" library to set
    the QoS rules.

    Closes-Bug: #1962465
    Change-Id: I3a09718c6e79fd1f37eb75264389c4e5043076a7
    (cherry picked from commit 132905d81f8799f249936ecb9d56641749df8724)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/837089
Committed: https://opendev.org/openstack/neutron/commit/21c9d40af4d874bccaaa7ed3cfde491ae8b10f97
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 21c9d40af4d874bccaaa7ed3cfde491ae8b10f97
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Mon Feb 28 17:56:33 2022 +0000

    [L3][QoS] L3 agent QoS extension to handle duplicated FIPs

    The L3 agent QoS extension should be able to handle duplicated floating
    IP addresses from different registers. Those floating IP addresses can
    be hosted in different networks and routers but the same compute node
    and L3 agent.

    Now, instead of using the IP address as unique identifier, this patch
    stores the (ID, IP address) tuple. This tuple is unique per floating IP
    register and holds the IP address, needed by the "tc" library to set
    the QoS rules.

    Closes-Bug: #1962465
    Change-Id: I3a09718c6e79fd1f37eb75264389c4e5043076a7
    (cherry picked from commit 132905d81f8799f249936ecb9d56641749df8724)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/837088
Committed: https://opendev.org/openstack/neutron/commit/3eb6aa26f7feefea43419d1625239b878dd62db0
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 3eb6aa26f7feefea43419d1625239b878dd62db0
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Mon Feb 28 17:56:33 2022 +0000

    [L3][QoS] L3 agent QoS extension to handle duplicated FIPs

    The L3 agent QoS extension should be able to handle duplicated floating
    IP addresses from different registers. Those floating IP addresses can
    be hosted in different networks and routers but the same compute node
    and L3 agent.

    Now, instead of using the IP address as unique identifier, this patch
    stores the (ID, IP address) tuple. This tuple is unique per floating IP
    register and holds the IP address, needed by the "tc" library to set
    the QoS rules.

    Closes-Bug: #1962465
    Change-Id: I3a09718c6e79fd1f37eb75264389c4e5043076a7
    (cherry picked from commit 132905d81f8799f249936ecb9d56641749df8724)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 21.0.0.0rc1 release candidate.