[dvr] misleading fip rule priority not found error message

Bug #1929821 reported by Edward Hope-Morley
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Low
Rodolfo Alonso

Bug Description

The fix for bug 1891673 added an error log like "Rule priority not found for floating ip a.b.c.d" such that if the ip rule priority information needed to configure a floating ip could not be found (in the fip-priorities file) the error message is logged. This error message can be misleading since not all floating ips will have or need a priority allocation since only those configured in qrouter namespaces need them but not fips for unbound ports that are configured in the snat ns (dvr). We should gate retrieving that information on whether the fixed_ip associated with fip is bound or not to avoid having this misleading error messages.

tags: added: l3-dvr-backlog low-hanging-fruit
Changed in neutron:
importance: Undecided → Low
Revision history for this message
Rodolfo Alonso (rodolfo-alonso-hernandez) wrote :

Hello:

Actually reviewing patch [1] I think we can improve a couple of things:

1) How we load the FIP information from the FipRulePriorityAllocator file (the subject of this bug).
In case of not having one of the FIP priorities in the state file, we should try to find it directly on the "ip rule" namespace table, searching for the fixed IP address (if present, as Slawek said). All FIP rules are stored in table FIP_RT_TBL = 16. This is the output of ip_lib.list_ip_rules: http://paste.openstack.org/show/806327/
This is just a matter of finding the IP address and assigning the priority already given. Problem: we cannot allocate a FIP in "self._rule_priorities" with a specific priority. We'll need to remove the rule in the "ip rules" table, allocate it again and write the rule with the new priority.
That will make [2] impossible to happen (I don't image when this could happen).

2) Once we finished reading the FIP list, we'll have "self._rule_priorities" updated. With this information, we can remove any rule present in table FIP_RT_TBL = 16 that is not in "self._rule_priorities". That will clean up any unnecessary "ip rule" present in this table.

Regards.

[1]https://review.opendev.org/q/If656a703c996ccc7719b1b09d793c5bbdfd6f3c1
[2]https://github.com/openstack/neutron/blob/63d8788d0f30aeedd54447e5c7ae7380973bf21a/neutron/agent/l3/dvr_local_router.py#L185-L188

Changed in neutron:
status: New → In Progress
Revision history for this message
Edward Hope-Morley (hopem) wrote :

@rodolfo-alonso-hernandez that sounds like a plan. Just remember that not all FIPs will have or need an entry in the fip-priorities file e.g. floating ips associated with unbound ports that are e.g. stored in the snat namespace, so we will still need a way to differentiate/filter those in order to completely fix this.

Revision history for this message
Rodolfo Alonso (rodolfo-alonso-hernandez) wrote :
Changed in neutron:
assignee: nobody → Rodolfo Alonso (rodolfo-alonso-hernandez)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

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

commit a03c240ef4ea1d4b874b618dbd0163a3a2f7024c
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Jun 3 14:49:45 2021 +0000

    Populate self.floating_ips_dict using "ip rule" information

    When the L3 agent starts, reads the floating IP rule priority from
    a state file created by "FipRulePriorityAllocator". In case of not
    having all floating IPs registers in this file, the method:
    - Creates a new priority for this floating IP.
    - Creates the "ip rule" in the namespace.
    - Adds a new entry in "self.floating_ips_dict".

    All "ip rules" present in the namespace that do not match the
    registered fixed IP address ("from") and the priority assigned
    are deleted.

    Closes-Bug: #1891673
    Closes-Bug: #1929821

    Change-Id: Ia3fbde3304ab5f3c309dc62dbf58274afbcf4614

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 19.0.0.0rc1

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

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/+/810393

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/+/810394

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/+/810425

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

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

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

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

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

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

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

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

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

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

commit 7eaa84a0cd48adb2ecd7653640d32aea8096e786
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Jun 3 14:49:45 2021 +0000

    Populate self.floating_ips_dict using "ip rule" information

    When the L3 agent starts, reads the floating IP rule priority from
    a state file created by "FipRulePriorityAllocator". In case of not
    having all floating IPs registers in this file, the method:
    - Creates a new priority for this floating IP.
    - Creates the "ip rule" in the namespace.
    - Adds a new entry in "self.floating_ips_dict".

    All "ip rules" present in the namespace that do not match the
    registered fixed IP address ("from") and the priority assigned
    are deleted.

    Closes-Bug: #1891673
    Closes-Bug: #1929821

    Conflicts:
        neutron/tests/unit/agent/l3/test_dvr_local_router.py

    Change-Id: Ia3fbde3304ab5f3c309dc62dbf58274afbcf4614
    (cherry picked from commit a03c240ef4ea1d4b874b618dbd0163a3a2f7024c)
    (cherry picked from commit b4ad1a2775d00cd6d14bd4766a0a1c5c41332d89)

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

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

commit f2f7602de7e1b067ae28f55b2b2dbbde29e7bcb1
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Jun 3 14:49:45 2021 +0000

    Populate self.floating_ips_dict using "ip rule" information

    When the L3 agent starts, reads the floating IP rule priority from
    a state file created by "FipRulePriorityAllocator". In case of not
    having all floating IPs registers in this file, the method:
    - Creates a new priority for this floating IP.
    - Creates the "ip rule" in the namespace.
    - Adds a new entry in "self.floating_ips_dict".

    All "ip rules" present in the namespace that do not match the
    registered fixed IP address ("from") and the priority assigned
    are deleted.

    Closes-Bug: #1891673
    Closes-Bug: #1929821

    Change-Id: Ia3fbde3304ab5f3c309dc62dbf58274afbcf4614
    (cherry picked from commit a03c240ef4ea1d4b874b618dbd0163a3a2f7024c)

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

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

commit b4ad1a2775d00cd6d14bd4766a0a1c5c41332d89
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Jun 3 14:49:45 2021 +0000

    Populate self.floating_ips_dict using "ip rule" information

    When the L3 agent starts, reads the floating IP rule priority from
    a state file created by "FipRulePriorityAllocator". In case of not
    having all floating IPs registers in this file, the method:
    - Creates a new priority for this floating IP.
    - Creates the "ip rule" in the namespace.
    - Adds a new entry in "self.floating_ips_dict".

    All "ip rules" present in the namespace that do not match the
    registered fixed IP address ("from") and the priority assigned
    are deleted.

    Closes-Bug: #1891673
    Closes-Bug: #1929821

    Change-Id: Ia3fbde3304ab5f3c309dc62dbf58274afbcf4614
    (cherry picked from commit a03c240ef4ea1d4b874b618dbd0163a3a2f7024c)

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

Reviewed: https://review.opendev.org/c/openstack/neutron/+/810395
Committed: https://opendev.org/openstack/neutron/commit/2cde9609c972e4fc4dd11a5c4caa342a3c1a3272
Submitter: "Zuul (22348)"
Branch: stable/train

commit 2cde9609c972e4fc4dd11a5c4caa342a3c1a3272
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Jun 3 14:49:45 2021 +0000

    Populate self.floating_ips_dict using "ip rule" information

    When the L3 agent starts, reads the floating IP rule priority from
    a state file created by "FipRulePriorityAllocator". In case of not
    having all floating IPs registers in this file, the method:
    - Creates a new priority for this floating IP.
    - Creates the "ip rule" in the namespace.
    - Adds a new entry in "self.floating_ips_dict".

    All "ip rules" present in the namespace that do not match the
    registered fixed IP address ("from") and the priority assigned
    are deleted.

    Closes-Bug: #1891673
    Closes-Bug: #1929821

    Conflicts:
        neutron/tests/unit/agent/l3/test_dvr_local_router.py

    Change-Id: Ia3fbde3304ab5f3c309dc62dbf58274afbcf4614
    (cherry picked from commit a03c240ef4ea1d4b874b618dbd0163a3a2f7024c)
    (cherry picked from commit b4ad1a2775d00cd6d14bd4766a0a1c5c41332d89)

tags: added: in-stable-train
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers