On 9/28/26 11:14 PM, Martin Kalčok via dev wrote: > On Mon, Sep 28, 2026 at 4:46 AM Bekei Park <[email protected]> wrote: > >> Hi Martin Kalčok >> > > Hi Bekei, > I'm looping in the mailing list, because I it was not on your reply and I > think that there are some valuable findings in your message. >
Hi Bekei, Martin, > >> >> I also tried the LRP option, to see whether it avoids the problem. >> On our 26.03.3 build, with one backend VM and the monitor sourced >> from the router port IP, I recreated the monitor ten times without >> this patch and five times with it: >> >> - Without the patch, the first SYN left 0.24 to 0.50 ms before the >> flow that sends the reply to ovn-controller was installed (10/10). >> The SYN-ACK reached the tap between 0.24 ms before and 0.35 ms >> after that flow's install event. All ten first probes still >> succeeded, and the monitor went online on the second probe, as it >> did with the patch. >> > > So do I understand correctly that with the current codebase, the monitor > comes online only on after the second probe? > > >> - With the patch, the first SYN left after that flow (5/5). >> >> So the ordering is the same in LRP mode, but I could not make it >> fail there. The failure we see needs the second ARP responder, that >> is, the reused IP. >> >> Given that, I understand if you do not want this for a setup that is >> outside the documented options. >> > > Far be it for me to decide what get's merged in and what not. That's really > up to maintainers, I'm just an occasional contributor :) However, IIUC and > the patch improves the health monitor responsiveness, then that's imo a > good reason to consider this patch. > Thanks for the fix! I think it's a very good fit for using the seqno module indeed. > Best regards, > Martin. > > We will keep the patch downstream, >> and I will ask the ovn-octavia-provider developers to use the LRP IP >> when the member subnet has a router port. If you still think >> sending the first probe only after its flows are installed is worth >> having, I can send a v2 with the commit message rewritten around >> that. >> >> Regards, >> Bekei >> >> 2026년 9월 27일 (일) 오후 8:07, Martin Kalčok <[email protected]>님이 작성: >> >>> Hi Bekei, >>> Sorry I didn’t do a full review, I have just one question below. >>> >>>> On 26 Sep 2026, at 00:39, Bekei Park <[email protected]> wrote: >>>> >>>> A new service monitor sends its first probe as soon as >>>> sync_svc_monitors() sees the Service_Monitor row. pinctrl_run() runs >>>> before ofctrl_put() in the same main loop iteration, so the probe >>>> leaves before the flows computed from the same SB update are in OVS. >>>> The reply depends on some of these flows. ovn-northd adds the >>>> Service_Monitor row and the ARP responder that answers for the monitor >>>> source IP with 'svc_monitor_mac' in one transaction: >>>> >>>> priority 110, "arp.tpa == <src_ip> && arp.op == 1" >>>> >>>> The backend usually has no ARP entry for the source IP yet and >>>> broadcasts a request as soon as the SYN arrives. If that request is >>>> handled before the flow above is installed, and another responder for >>>> the same IP exists, >>> >>> Wouldn’t the setup like this be a misconfiguration? The documentation [0] >>> says that the source IP of the health-check probe should be either an >>> unused IP, or an IP of a Logical Router in the network (the second option >>> was added in 26.03). Is neither of these a viable option for you? >>> >>> Even with your fix, imo, the result of re-using existing IP for health >>> checks is an unexpected behaviour from the POV of the monitored host. >>> Because now it can’t reach the true owner of the IP and the reason for it >>> is very opaque. >>> >>> Best regards, >>> Martin. >>> >>>> the backend learns the wrong MAC. With >>>> ovn-octavia-provider the source IP belongs to the "ovn-lb-hm-<subnet>" >>>> localport, which has the usual priority 50 responder for broadcast >>>> requests with its own MAC. The SYN-ACKs then go to the localport and >>>> never reach ovn-controller, the monitor goes offline after >>>> failure_count probes, and the backends of the load balancer are >>>> removed until the backend refreshes its ARP entry with a unicast >>>> request, which hits the priority 110 flow. We saw 7 to 41 seconds >>>> without a working backend after the first health check of a subnet >>>> was created. >>>> >>>> On a bare metal deployment the backend sent the ARP request 0.11 ms >>>> after the first SYN and got the localport MAC every time. On a nested >>>> test setup the request came 0.36 ms after the SYN and the priority 110 >>>> flow was installed just in time, although the SYN was sent about >>>> 0.8 ms before OVS reported that flow. >>>> >>>> Keep a new monitor in a new SVC_MON_S_WAIT_FLOWS state and request an >>>> ofctrl seqno for it in sync_svc_monitors(), which runs before >>>> ofctrl_put() of the same iteration. Move the monitor to >>>> SVC_MON_S_INIT once the seqno is acked, i.e. once the flows for these >>>> SB contents are installed. If the OpenFlow connection drops, >>>> ofctrl_seqno_flush() drops the request, so request it again. Replies >>>> are ignored while a monitor waits, and new monitors are zeroed so that >>>> the probe fields are never used uninitialized. >>>> >>>> This also covers monitors that start because the backend port was >>>> bound to this chassis or came up. >>>> >>>> There is no test that makes the race deterministic: the flows have to >>>> stay uninstalled while the main loop keeps running. The existing >>>> health check tests pass and show that the probes still start. >>>> >>>> CC: Numan Siddique <[email protected]> >>>> Fixes: 8be01f4a5329 ("Send service monitor health checks") >>>> Assisted-by: Claude Opus 5.5, Claude Code >>>> Signed-off-by: Bekei Park <[email protected]> >>>> --- ... >>>> + /* The first probe of a new monitor must wait until the flows >>> computed >>>> + * from these SB contents are installed. Otherwise the reply can >>> race >>>> + * with them, e.g. the backend resolves the probe source IP before >>> the >>>> + * ARP responder for 'svc_monitor_mac' exists and the replies >>> never reach >>>> + * ovn-controller. This is called before ofctrl_put() of the same >>>> + * iteration, so the request is acked once those flows are >>> installed. */ >>>> + bool requested = false; >>>> + LIST_FOR_EACH (svc_mon, list_node, &svc_monitors) { >>>> + if (svc_mon->state == SVC_MON_S_WAIT_FLOWS && >>> !svc_mon->flows_seqno) { >>>> + if (!requested) { >>>> + ofctrl_seqno_update_create(svc_mon_seqno_type, >>>> + ++svc_mon_seqno_requested); >>>> + requested = true; >>>> + } >>>> + svc_mon->flows_seqno = svc_mon_seqno_requested; I had to read this a couple of times, the prefix increment always gets me. But it's correct. >>>> + } >>>> + } >>>> + >>>> if (changed) { >>>> notify_pinctrl_handler(); >>>> } >>>> >>>> } Applied to main, 26.09 and 26.03. I also added Bekei to the AUTHORS.rst list. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
