On 3/19/26 8:54 AM, Dumitru Ceara wrote: > On 3/18/26 10:08 PM, Mark Michelson wrote: >> Hi Dumitru, >> > > Hi Mark, > >> The only nit I have is with the system test you have added. The test >> name is the same as the test added in ovn.at "Load balancer health > > Right, I had copy/pasted it from the ovn-northd.at test and struggled > with changing it to a better name. I'll do that in v2. > >> checks - service monitor source MAC". However, it doesn't check the >> source MAC address of the health check packets to ensure that the >> global service monitor MAC is being used. Instead, it seems to be a >> basic test to ensure that health checks work when the load balancer is >> applied to the switch and the logical router's IP address is used as >> the source IP address of the health checks. The test is useful, since > > Well, it's more than just a basic test IMO because it actually fails if > the wrong source mac is used. > >> I don't think we have coverage for this case, but I suggest that we >> add some sort of check to ensure that the source MAC of the health >> checks is what we expect. Otherwise, the test should be renamed to >> indicate it is a basic functionality test. > > As said above, this test fails without the source mac fix because OVN > will expect probe replies to be sent to $svc_monitor_mac destination but > the probe requests were actually sent from the LRP mac. > > I'll try to add an explicit check in v2 to make it more obvious. >
v2 available here: https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
