On 3/10/25 9:14 PM, Rosemarie O'Riorden wrote:
> When a gateway router has a load balancer configured, the option
> lb_force_snat_ip=router_ip can be set so that OVN SNATs load balanced
> packets to the logical router's egress interface's IP address, that is, the
> port chosen as "outport" in the lr_in_ip_routing stage.
> 
> However, this was only designed to work when one network was configured
> on the logical router outport. When multiple networks were configured,
> OVN's behavior was to simply choose the lexicographically first IP address for
> SNAT. This often lead to an incorrect address being used for SNAT.
> 
> To fix this, two main components have been added:
>  1. A new flag, flags.network_id. It is 4 bits and stores an index.
>  2. A new stage in the router ingress pipeline, lr_in_network_id.
> 
> Now in the stage lr_in_network_id, OVN generates flows that assign
> flags.network_id with an index, which is chosen by looping through the 
> networks
> on the port, and assigning flags.network_id = i. flags.network_id is then
> matched on later in lr_out_snat and the network at that index will be chosen
> for SNAT.
> 
> However, if there are more than 16 networks, flags.network_id will be 0
> for networks 17 and up. Then, for those networks, the first
> lexicographical network will be chosen for SNAT.
> 
> There is also a lower priority flow with the old behavior to make upgrades
> smooth.
> 
> Two tests have been added to verify that:
>  1. The correct network is chosen for SNAT.
>  2. The new and updated flows with flags.network_id are correct.
> 
> And tests that were broken by this new behavior have been updated.
> 
> Reported-at: https://issues.redhat.com/browse/FDP-871
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417717.html
> Signed-off-by: Rosemarie O'Riorden <[email protected]>
> ---

Hi Rosemarie,

Thanks for this new version!

> v3:
> - Fix "16 bits" typo, flags.network_id 4 bits (fits 16 networks)
> - Replace inconsistent comments (networks "17 and up" get network_id=0)
> - Define OVN_MAX_NETWORK_ID macro, use instead of hardcoding 15 or 16
> - Use OVN_MAX_NETWORK_ID while defining MLF_NETWORK_ID
> - Fix incorrect expression resulting in wrong hex value
> - Remove extra quotations around port name, resulting in double quotes and 
> thus
>   invalid flows
> - Make priority-100 flow (with old behavior) 105 instead

I'm guessing some tests didn't get updated after this change.  Please
see the test run:

https://github.com/ovsrobot/ovn/actions/runs/13774011391/job/38519208720#step:11:5664

-  table=??(lr_out_snat        ), priority=100  ,
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"),
action=(ct_snat(172.168.0.100);)
+  table=??(lr_out_snat        ), priority=105  ,
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"),
action=(ct_snat(172.168.0.100);)

> - Remove "flags.force_snat_for_lb == 1" from match in new stage, set 
> network_id
>   for all IP packets instead
> - Change function name for new stage since it's no longer restricted to "force
>   snat for lb" case
> - Update affected test flows
> - Fix comment that was too long, improve wording
> - Replace "routerip" with "router_ip"
> v2:
> - Improve wording of commit message and add missing explanation
> - Add Reported-at tag
> - Correct hex value for MLF_NETWORK_ID in include/ovn/logical-fields.h
> - In northd/northd.c:
>   - Change behavior to set flags.network_id=0 for networks 16 and up
>   - Add flow with old behavior for consistency during upgrades
>   - Fix redundant loop conditions
>   - Remove code I accidentally included in v1
>   - Fix indentation in multiple spots
>   - Remove incorrect changes in build_lrouter_nat_defrag_and_lb()
> - Fix documentation, add new stage
> - Move end-to-end test from ovn-northd.at to ovn.at
> - Add check before all ovn and ovs commands in test
> - Remove unnecessary section included in test
> - Updated all flows in tests that were affected by new changes
> - Added more IP addresses to test to test behavior with >16 networks

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to