Hi Dumitru,
Thanks a lot for your comments. I have posted patch 7 after rebasing and fixing
the unit tests. Will address the code review comments in next patch.
Thanks,
Sragdhara
From: Dumitru Ceara <dce...@redhat.com>
Date: Thursday, August 21, 2025 at 7:48 AM
To: Sragdhara Datta Chaudhuri <sragdha.chau...@nutanix.com>,
ovs-dev@openvswitch.org <ovs-dev@openvswitch.org>
Cc: Numan Siddique <num...@ovn.org>
Subject: Re: [ovs-dev] [PATCH OVN v6 5/5] northd, controller, tests: Network
Function Health monitoring.
!-------------------------------------------------------------------|
CAUTION: External Email
|-------------------------------------------------------------------!
On 8/20/25 3:25 AM, Sragdhara Datta Chaudhuri wrote:
> The LB health monitoring functionality has been extended to support NFs.
> Network_Function_Group has a list of Network_Functions, each of which has a
> reference to network_Function_Health_Check that has the monitoring config.
> There is a corresponding SB service_monitor maintaining the online/offline
> status. When status changes, northd picks one of the “online” NFs and sets in
> network_function_active field of NFG. The redirection rule in LS uses the
> ports from this NF.
>
> Ovn-controller performs the health monitoring by sending ICMP echo request
> with source IP and MAC from NB global options “svc_monitor_ip4” and
> “svc_monitor_mac”, and destination IP and MAC from new NB global options
> “svc_monitor_ip4_dst” and “svc_monitor_mac_dst”. The sequence number and id
> are randomly generated and stored in service_mon. The NF VM forwards the
> same packet out of the other port. When it comes out, ovn-controller matches
> the sequence number and id with stored values and marks online if matched.
>
> In SB Service_Monitor table three new fields have been added:
> type: to indicate “load-balancer” or “network-function”
> mac: the destination MAC address for the monitor packets
> logical_input_port: The LSP to which the probe packet would be sent
> (taken from inport of Network_Function)
>
> Co-authored-by: Naveen Yerramneni <naveen.yerramn...@nutanix.com>
> Co-authored-by: Karthik Chandrashekar <karthi...@nutanix.com>
> Signed-off-by: Naveen Yerramneni <naveen.yerramn...@nutanix.com>
> Signed-off-by: Karthik Chandrashekar <karthi...@nutanix.com>
> Signed-off-by: Sragdhara Datta Chaudhuri <sragdha.chau...@nutanix.com>
> ---
Hi Sragdhara,
Not a full review, just one thing I spotted quickly:
[...]
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 8123679b7..456ee92c1 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> "version": "21.3.0",
We need to increase the version number from x.y.z to x.(y+1).0 if there
are non cosmetic changes.
> - "cksum": "3714238857 35128",
> + "cksum": "2023557864 35455",
> "tables": {
> "SB_Global": {
> "columns": {
> @@ -509,14 +509,20 @@
> "isRoot": true},
> "Service_Monitor": {
> "columns": {
> + "type": {"type": {"key": {
> + "type": "string",
> + "enum": ["set", ["load-balancer",
> + "network-function"]]}}},
> "ip": {"type": "string"},
> + "mac": {"type": "string"},
> "protocol": {
> "type": {"key": {"type": "string",
> - "enum": ["set", ["tcp", "udp"]]},
> + "enum": ["set", ["tcp", "udp", "icmp"]]},
> "min": 0, "max": 1}},
> "port": {"type": {"key": {"type": "integer",
> "minInteger": 0,
> "maxInteger": 65535}}},
> + "logical_input_port": {"type": "string"},
> "logical_port": {"type": "string"},
> "src_mac": {"type": "string"},
> "src_ip": {"type": "string"},
I don't think I'll be able to review the rest of this patch today.
However I did have comments on the previous ones and it would be useful
to be able to test this so it would be great if you could post a v7
rebased on current main branch and ideally with the comments addressed?
Thanks,
Dumitru
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
