Hi Ales,
Sorry for the long silence on this one.
> no worries that can happen. For this approach shouldn't we also
> include UDP, AFAIR we support UDP too.
Yes, we should. And the UDP side is arguably the worse half of the
original bug, which I had not appreciated when I wrote that mail.
svc_monitor_type_allows_proto() lets SVC_MON_TYPE_LB use TCP and UDP,
and the two protocols fail in opposite directions when the reply lflow
never fires:
- TCP: svc_monitors_run() hits the SVC_MON_S_WAITING timeout,
n_failures++ and the monitor goes OFFLINE. A working baremetal
member is reported down - noisy, but at least visible.
- UDP: the same timeout takes the other branch, n_success++ and the
monitor goes ONLINE. The only thing that ever moves a UDP monitor
to OFFLINE is the ICMP port-unreachable path at the end of
pinctrl_handle_svc_check() (ICMP4_DST_UNREACH code 3 / ICMPv6 type 1
code 4), and that is reached only through handle_svc_check(). So a
dead UDP member stays ONLINE forever and the LB keeps sending
traffic to it. Silent, and worse than the TCP case.
That ICMP error is exactly what the "icmp4.type == 3" / "icmp6.type == 1"
branch of build_lb_health_check_response_lflows() already matches, so
including UDP does not need a new match shape - it is the same
existing per-protocol branch. Only the inport part changes.
Concretely, for a backend whose LSP is type=external on a switch with
localnet ports, per localnet port lp:
match: inport == <lp> && ip4.dst == <svc_mon_src_ip> &&
ip4.src == <backend ip> && eth.dst == <lrp mac> && <l4>
action: inport = "<external lsp>"; handle_svc_check(inport);
with <l4> being "tcp.src == <backend port>" for TCP and
"icmp4.type == 3" for UDP (icmp6.type == 1 for v6) - i.e. reusing the
branch that is already there. Same for the IPv6 leg.
Two changes from what I proposed last time:
- I dropped "eth.src == <backend mac>". It is redundant next to
"ip4.src == <backend ip>", which the existing lflow already carries,
and after FDP-4157 I would rather not key anything on a bare MAC
again. If you would prefer to keep it as belt and braces I have no
objection.
- The rewrite is scoped to this one lflow. handle_svc_check() takes a
field, not a port name, so the assignment is still needed - but the
action list ends there, with no next;, so the modified MFF_LOG_INPORT
never reaches the flood stages or any later table. That is the
substantive difference from the reverted patch, which rewrote inport
at ls_in_check_port_sec and let the packet carry it through the rest
of the pipeline.
On your regression test: it stays green. The unknown unicast, the
MLDv2 report and the IGMPv3 report all fail "eth.dst == <lrp mac>",
so none of them can reach this lflow, and nothing gets re-injected.
@Dumitru - you mentioned you had seen a version of the alternative fix
posted. I think that was the lb_force_snat_ip patch I sent around the
same time; the alternative HM fix has not been posted yet. Nothing is
waiting on you in this thread.
v2 will cover TCP and UDP over v4 and v6, re-add the ovn-northd.at
coverage with both a TCP and a UDP load balancer, and keep your test
untouched. It will also carry:
Assisted-by: Claude Opus 5, Claude Code
since Dumitru asked about that on the other patch.
Regards,
JayGue
On Fri, Jul 31, 2026 at 4:12 PM Ales Musil <[email protected]> wrote:
>
>
> On Thu, Jul 30, 2026 at 2:16 AM jay <[email protected]> wrote:
>
>> Hi Ales,
>>
>> My bad - I missed that matching on eth.src alone catches all
>> traffic from that MAC, including IGMP/MLD reports re-injected by
>> pinctrl. The revert makes sense.
>>
>> Acked-by: JayGue Lee <[email protected]>
>>
>> I'd still like to fix the original issue, this time without
>> touching table 0. Instead of the early inport rewrite, add the
>> localnet inport and backend MAC to the per-backend HM reply lflow
>> at ls_in_l2_lkup:
>>
>> inport == <localnet port> && eth.src == <backend MAC> &&
>> ip4.src == <backend IP> && tcp.src == <port> &&
>> eth.dst == <router MAC>
>> actions: inport = <external LSP>; handle_svc_check(inport);
>>
>> The rewrite only fires on unicast TCP replies to the router MAC,
>> so multicast and unknown unicast are unaffected and your new test
>> keeps passing.
>>
>> Does this sound reasonable? If so I'll send a v2.
>>
>> Regards,
>> JayGue Lee
>>
>
> Hi JayGue,
>
> no worries that can happen. For this approach shouldn't we also
> include UDP, AFAIR we support UDP too.
>
> Regards,
> Ales
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
