On 1/30/26 7:25 PM, Martin Kalcok wrote:
> Load Balancer Health Checks require specifying "source IP". This IP
> has to be from the subnet of the monitored LB backend and should not
> be used by any other port in the LSP. If the "source IP" is also used
> by some other port, the host with the LB backend won't be able to communicate
> with this port due to the ARP responder rule installed by the controller.
> 
> This limitation forces CMSes to reserve an IP per LSP with LB backend, which
> is not always practical or possible.
> 
> This change allows usage of LRP IPs as the source of LB health check
> probes. It introduces following changes for such scenario:
> 
>   * service_monitor (SBDB) will use MAC address of the LRP
>   * ARP responder rule is not necessary in this case
>   * Destination IP, destination MAC, "inport" and source TCP/UDP port number
>     will be used to determine that a packet is a response to a health check
>     probe. These packets will be redirected to the controller.
> 
> Behavior is unchanged for the LB health checks that use reserved/unused
> "source IPs".
> 
> Signed-off-by: Martin Kalcok <[email protected]>
> ---
> 

Hi Martin,

> RFC -> v2
>   * Reduce number of iterations when searching for monitor source IP in ports
>     by searching only in lrp_networks of LRPs, using `lrp_find_member_ip`.
>   * Narrowed down match for health check replies by matching
>     "inport == <monitored_lb_backend_port>"
>   * ICMP unreachable messages are also forwarded to the controller to allow
>     for monitoring UDP ports and detecting when they are down.
>   * Flows that send monitor replies to the controller are still inserted into
>     LS datapath, but the logic is tied to the LR. i.e.: Whether to add the
>     flows is evaluated when LB is attached to LR.
>     It removes the necessity to add LB to the LS as well. It is enough to add
>     it to the LR.
>   * IPv6 support added
>   * System and Unit tests added
>     * existing LB and Service Monitor checks were already quite involved, so
>       I opted to add new set of tests that focus specifically on a setup
>       with LRP IPs as a source for service checks.
>   * Documentation of "ip_port_mappings" field in "Load_Balancer" was extended
>     to explicitly state requirements for the source_ip.
> 
> v2 -> v3
>   * rebased on `main`
>     * `vector_get` replaced with `sparse_array_get`
>     * `ovn_lflow_metered` replace with WITH_CTRL_METER macro
>   * Addressed review comments
>     * NEWS entry added
>     * Formatting and comments fixed
>     * Formatting of "match" rules in `build_lb_health_check_response_lflows`
>       split for ipv4 and ipv6 cases.
>     * [tests] added `check` to ovs-vsctl calls
>     * [tests] dropped unnecessary --wait=sb
>   * Bug found: Function `build_lb_health_check_response_lflows` was, on each 
>     call, iterating over backends of the same `ovn_northd_lb_vip` instance
>     (see in v2: `&lb->vips_nb->backends_nb[j++];`). This caused occasional
>     segfaults and was replaced by passing "current" `ovn_northd_lb_vip` 
> instance
>     as an argument to the function.
> 
> v3 -> v4
>   * Incremental patch included in the v3, that aimed to reduce number of LRP
>     lookups, was squashed to the main patch.
>     * `struct ovn_northd_lb_backend` has new member, `svc_mon_lrp` that stores
>       pointer to the LRP that's used as a source of health check probes.
>     * The value of `svc_mon_lrp` is set once by calling
>       `ovn_northd_lb_backend_set_mon_port`. The rest of the code uses
>       `svc_mon_lrp` value to determine if the health check is using LRP as the
>       source of the probes, or if it uses "reserved" IP address not assigned
>       to any port.
>   * Logical flows that implement this feature are now inserted in the
>     S_SWITCH_IN_L2_LKUP stage to match with the "regular" service check
>     health checks.
>   * We now also match eth.dst of the packet with MAC address of the
>     `svc_mon_lrp`.
>   * function `build_lb_health_check_response_lflows` now checks that the
>     datapath of svc_mon_lrp's peer is of type DP_SWITCH before adding logical
>     flows. This prevents accidentally adding flows to a "switch" stage
>     S_SWITCH_IN_L2_LKUP in a router.
> 

Thanks for addressing my comments, this version looks good to me!
Applied to main.

Regards,
Dumitru

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

Reply via email to