On 2/26/26 3:48 PM, Ales Musil wrote: > On Thu, Feb 26, 2026 at 3:26 PM Dumitru Ceara <[email protected]> wrote: > >> On 2/25/26 9:39 AM, Ales Musil via dev wrote: >>> The LSP port security was parsed just to be used as check if there >>> is any valid port security. The full parsing isn't necessary we can >>> stop the parsing when we find at least one valid entry as a >>> indication for the rest of the processing. This also ensure that we >>> don't produce log when there is "VRRPv3" specified as it was just >>> noise and didn't really affect the functionality. >>> >>> Fixes: d9e2393e9840 ("controller: Add option to make port security >> compliant with RFC 9568.") >>> Reported-at: https://issues.redhat.com/browse/FDP-3245 >>> Signed-off-by: Ales Musil <[email protected]> >>> --- >> >> Hi Ales, >> >> Thanks for the patch! >> > > Hi Dumitru, > > thank you for the review. > > >> >>> northd/northd.c | 28 ++++++++-------------------- >>> northd/northd.h | 4 ++-- >>> tests/ovn.at | 2 ++ >>> 3 files changed, 12 insertions(+), 22 deletions(-) >>> >>> diff --git a/northd/northd.c b/northd/northd.c >>> index 983975dac..9401f41ab 100644 >>> --- a/northd/northd.c >>> +++ b/northd/northd.c >>> @@ -1116,13 +1116,6 @@ ovn_port_cleanup(struct ovn_port *port) >>> port->peer->peer = NULL; >>> } >>> >>> - for (int i = 0; i < port->n_ps_addrs; i++) { >>> - destroy_lport_addresses(&port->ps_addrs[i]); >>> - } >>> - free(port->ps_addrs); >>> - port->ps_addrs = NULL; >>> - port->n_ps_addrs = 0; >>> - >> >> Now you use port->lsp_has_port_sec but it's not cleared here. There are >> cases when ovn_port_cleanup(port) is called and then 'port' is >> reinitialized, e.g., in ls_port_reinit(). There we call >> ls_port_init(port) after cleanup. That calls parse_lsp_addrs() where we >> only at most set port->lsp_has_port_sec = true. >> >> But if it used to be "true" and now it should be "false", we'd have an >> issue. In theory. In practice that can't happen because if addresses >> change lsp_can_be_inc_processed(port) returns false. >> >>> destroy_lport_addresses(&port->lrp_networks); >>> destroy_lport_addresses(&port->proxy_arp_addrs); >>> } >>> @@ -1512,19 +1505,14 @@ parse_lsp_addrs(struct ovn_port *op) >> >> So, I think I'd set port->lsp_has_port_sec = false in the beginning of >> parse_lsp_addrs(), somewhere above this point. >> >> What do you think? >> > > Yeah that makes sense. We can probably do that above the for loop. >
OK, if that's the only change in this patch I think there's no need for v2. I'm OK if you just amend your current commit with it and: Acked-by: Dumitru Ceara <[email protected]> Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
