Hi Jacob, Thanks for your review. Em ter., 11 de ago. de 2026 às 13:23, Jacob Tanenbaum <[email protected]> escreveu:
> Thanks for this patch series and I will reviewing the rest of the series > as well. > > > On Fri, Jul 24, 2026 at 12:41 PM Lucas Vargas Dias > <[email protected]> wrote: > >> Commit b337750e45be ("northd: Incremental processing of VIF changes in >> 'northd' node.") skipped non-VIF ports from processing, and 860d5e4138ed >> ("northd: Enable incremental processing for remote ports.") later >> observed that "remote" ports are almost identical to VIF ports. The >> same holds for ports of type "localport": every flow whose content >> differs from a plain VIF (most notably the ARP/ND responder flows, which >> a localport gets regardless of its "up" state) is owned by the port's >> own lflow_ref and is regenerated by the existing per-port incremental >> path, and a localport mutates no aggregate od->* state that a VIF would >> not. Enable that processing path for localport ports. >> >> Add a test covering incremental create/update/delete of a localport, >> including that its ARP responder reply flow is present regardless of the >> "up" state and removed on deletion. >> >> Assisted-by: Claude Opus 4.8, Claude Code >> Signed-off-by: Lucas Vargas Dias <[email protected]> >> --- >> northd/northd.c | 10 ++++++++-- >> tests/ovn-northd.at | 31 +++++++++++++++++++++++++++++++ >> 2 files changed, 39 insertions(+), 2 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index 4ca9820d4..7d0cd4f14 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -1284,6 +1284,12 @@ lsp_is_localnet(const struct >> nbrec_logical_switch_port *nbsp) >> return !strcmp(nbsp->type, "localnet"); >> } >> >> +static bool >> +lsp_is_localport(const struct nbrec_logical_switch_port *nbsp) >> +{ >> + return !strcmp(nbsp->type, "localport"); >> +} >> + >> > > nit: there is another location that also uses strcmp() for localport > in build_lswitch_arp_nd_responder_known_ips() could you replace that with > your helper function > I agree > > >> static bool >> lsp_is_vtep(const struct nbrec_logical_switch_port *nbsp) >> { >> @@ -4677,8 +4683,8 @@ destroy_northd_tracked_data(struct northd_data *nd) >> static bool >> lsp_can_be_inc_processed(const struct nbrec_logical_switch_port *nbsp) >> { >> - /* Support only normal VIF and remote ports for now. */ >> - if (nbsp->type[0] && !lsp_is_remote(nbsp)) { >> + /* Support only normal VIF, remote and localport ports for now. */ >> + if (nbsp->type[0] && !lsp_is_remote(nbsp) && >> !lsp_is_localport(nbsp)) { >> return false; >> } >> >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index 810e9f2d7..2dcd0f8e0 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -12030,6 +12030,37 @@ check_recompute_counter 0 0 0 0 0 0 >> >> CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) >> >> +dnl Localports should be incrementally processed >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-add ls0 lp0 \ >> + -- lsp-set-type lp0 localport \ >> + -- lsp-set-addresses lp0 "aa:aa:aa:00:00:30 192.168.0.30" >> +check_recompute_counter 0 0 0 0 0 0 >> + >> +# A localport gets an ARP responder reply flow regardless of its "up" >> state. >> +AT_CHECK([ovn-sbctl dump-flows ls0 | grep ls_in_arp_rsp | \ >> + grep 'arp.tpa == 192.168.0.30' | grep -c 'arp.op = 2'], [0], [1 >> +]) >> + >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb \ >> + lsp-set-addresses lp0 "aa:aa:aa:00:00:31 192.168.0.31" >> +check_recompute_counter 0 0 0 0 0 0 >> + >> +AT_CHECK([ovn-sbctl dump-flows ls0 | grep ls_in_arp_rsp | \ >> + grep 'arp.tpa == 192.168.0.31' | grep -c 'arp.op = 2'], [0], [1 >> +]) >> + >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-del lp0 >> +check_recompute_counter 0 0 0 0 0 0 >> + >> +AT_CHECK([ovn-sbctl dump-flows ls0 | grep ls_in_arp_rsp | \ >> + grep -c '192.168.0.3'], [1], [0 >> > > nit: could you explicitly check for 192.168.0.30 and 192.168.0.31 > > I agree Regards, Lucas +]) >> + >> +CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) >> + >> check ovn-nbctl --wait=hv ls-del ls0 >> >> OVN_CLEANUP([hv1]) >> -- >> 2.43.0 >> >> >> -- >> >> >> >> >> _'Esta mensagem é direcionada apenas para os endereços constantes no >> cabeçalho inicial. Se você não está listado nos endereços constantes no >> cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa >> mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas >> estão >> imediatamente anuladas e proibidas'._ >> >> >> * **'Apesar do Magazine Luiza tomar >> todas as precauções razoáveis para assegurar que nenhum vírus esteja >> presente nesse e-mail, a empresa não poderá aceitar a responsabilidade >> por >> quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* >> >> >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
