| Message ID | 20230227200344.1308604-4-i.maximets@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | northd: Optimize parsing of LSP addresses. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Mon, Feb 27, 2023 at 09:03:44PM +0100, Ilya Maximets wrote: > At the point of IPAM configuration all the addresses are already parsed. > No need to waste time parsing them again. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Mark Michelson <mmichels@redhat.com> On 2/27/23 15:03, Ilya Maximets wrote: > At the point of IPAM configuration all the addresses are already parsed. > No need to waste time parsing them again. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > northd/northd.c | 54 ++++++++++++++++--------------------------------- > 1 file changed, 17 insertions(+), 37 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 6d37ea713..868a8c3b8 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -1497,7 +1497,10 @@ struct ovn_port { > const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */ > > struct lport_addresses *lsp_addrs; /* Logical switch port addresses. */ > - unsigned int n_lsp_addrs; > + unsigned int n_lsp_addrs; /* Total length of lsp_addrs. */ > + unsigned int n_lsp_non_router_addrs; /* Number of elements from the > + * beginning of 'lsp_addrs' extracted > + * directly from LSP 'addresses'. */ > > struct lport_addresses *ps_addrs; /* Port security addresses. */ > unsigned int n_ps_addrs; > @@ -1817,35 +1820,21 @@ ipam_insert_ip_for_datapath(struct ovn_datapath *od, uint32_t ip) > } > > static void > -ipam_insert_lsp_addresses(struct ovn_datapath *od, struct ovn_port *op, > - char *address) > +ipam_insert_lsp_addresses(struct ovn_datapath *od, > + struct lport_addresses *laddrs) > { > - if (!od || !op || !address || !strcmp(address, "unknown") > - || !strcmp(address, "router") || is_dynamic_lsp_address(address)) { > - return; > - } > - > - struct lport_addresses laddrs; > - if (!extract_lsp_addresses(address, &laddrs)) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "Extract addresses failed."); > - return; > - } > - ipam_insert_mac(&laddrs.ea, true); > + ipam_insert_mac(&laddrs->ea, true); > > /* IP is only added to IPAM if the switch's subnet option > * is set, whereas MAC is always added to MACAM. */ > if (!od->ipam_info.allocated_ipv4s) { > - destroy_lport_addresses(&laddrs); > return; > } > > - for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) { > - uint32_t ip = ntohl(laddrs.ipv4_addrs[j].addr); > + for (size_t j = 0; j < laddrs->n_ipv4_addrs; j++) { > + uint32_t ip = ntohl(laddrs->ipv4_addrs[j].addr); > ipam_insert_ip_for_datapath(od, ip); > } > - > - destroy_lport_addresses(&laddrs); > } > > static void > @@ -1855,29 +1844,21 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op) > return; > } > > - if (op->nbsp) { > + if (op->n_lsp_non_router_addrs) { > /* Add all the port's addresses to address data structures. */ > - for (size_t i = 0; i < op->nbsp->n_addresses; i++) { > - ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]); > - } > - } else if (op->nbrp) { > - struct lport_addresses lrp_networks; > - if (!extract_lrp_networks(op->nbrp, &lrp_networks)) { > - static struct vlog_rate_limit rl > - = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "Extract addresses failed."); > - return; > + for (size_t i = 0; i < op->n_lsp_non_router_addrs; i++) { > + ipam_insert_lsp_addresses(od, &op->lsp_addrs[i]); > } > - ipam_insert_mac(&lrp_networks.ea, true); > + } else if (op->lrp_networks.ea_s[0]) { > + ipam_insert_mac(&op->lrp_networks.ea, true); > > if (!op->peer || !op->peer->nbsp || !op->peer->od || !op->peer->od->nbs > || !smap_get(&op->peer->od->nbs->other_config, "subnet")) { > - destroy_lport_addresses(&lrp_networks); > return; > } > > - for (size_t i = 0; i < lrp_networks.n_ipv4_addrs; i++) { > - uint32_t ip = ntohl(lrp_networks.ipv4_addrs[i].addr); > + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { > + uint32_t ip = ntohl(op->lrp_networks.ipv4_addrs[i].addr); > /* If the router has the first IP address of the subnet, don't add > * it to IPAM. We already added this when we initialized IPAM for > * the datapath. This will just result in an erroneous message > @@ -1887,8 +1868,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op) > ipam_insert_ip_for_datapath(op->peer->od, ip); > } > } > - > - destroy_lport_addresses(&lrp_networks); > } > } > > @@ -2573,6 +2552,7 @@ join_logical_ports(struct northd_input *input_data, > } > op->n_lsp_addrs++; > } > + op->n_lsp_non_router_addrs = op->n_lsp_addrs; > > op->ps_addrs > = xmalloc(sizeof *op->ps_addrs * nbsp->n_port_security);
diff --git a/northd/northd.c b/northd/northd.c index 6d37ea713..868a8c3b8 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -1497,7 +1497,10 @@ struct ovn_port { const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */ struct lport_addresses *lsp_addrs; /* Logical switch port addresses. */ - unsigned int n_lsp_addrs; + unsigned int n_lsp_addrs; /* Total length of lsp_addrs. */ + unsigned int n_lsp_non_router_addrs; /* Number of elements from the + * beginning of 'lsp_addrs' extracted + * directly from LSP 'addresses'. */ struct lport_addresses *ps_addrs; /* Port security addresses. */ unsigned int n_ps_addrs; @@ -1817,35 +1820,21 @@ ipam_insert_ip_for_datapath(struct ovn_datapath *od, uint32_t ip) } static void -ipam_insert_lsp_addresses(struct ovn_datapath *od, struct ovn_port *op, - char *address) +ipam_insert_lsp_addresses(struct ovn_datapath *od, + struct lport_addresses *laddrs) { - if (!od || !op || !address || !strcmp(address, "unknown") - || !strcmp(address, "router") || is_dynamic_lsp_address(address)) { - return; - } - - struct lport_addresses laddrs; - if (!extract_lsp_addresses(address, &laddrs)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "Extract addresses failed."); - return; - } - ipam_insert_mac(&laddrs.ea, true); + ipam_insert_mac(&laddrs->ea, true); /* IP is only added to IPAM if the switch's subnet option * is set, whereas MAC is always added to MACAM. */ if (!od->ipam_info.allocated_ipv4s) { - destroy_lport_addresses(&laddrs); return; } - for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) { - uint32_t ip = ntohl(laddrs.ipv4_addrs[j].addr); + for (size_t j = 0; j < laddrs->n_ipv4_addrs; j++) { + uint32_t ip = ntohl(laddrs->ipv4_addrs[j].addr); ipam_insert_ip_for_datapath(od, ip); } - - destroy_lport_addresses(&laddrs); } static void @@ -1855,29 +1844,21 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op) return; } - if (op->nbsp) { + if (op->n_lsp_non_router_addrs) { /* Add all the port's addresses to address data structures. */ - for (size_t i = 0; i < op->nbsp->n_addresses; i++) { - ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]); - } - } else if (op->nbrp) { - struct lport_addresses lrp_networks; - if (!extract_lrp_networks(op->nbrp, &lrp_networks)) { - static struct vlog_rate_limit rl - = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "Extract addresses failed."); - return; + for (size_t i = 0; i < op->n_lsp_non_router_addrs; i++) { + ipam_insert_lsp_addresses(od, &op->lsp_addrs[i]); } - ipam_insert_mac(&lrp_networks.ea, true); + } else if (op->lrp_networks.ea_s[0]) { + ipam_insert_mac(&op->lrp_networks.ea, true); if (!op->peer || !op->peer->nbsp || !op->peer->od || !op->peer->od->nbs || !smap_get(&op->peer->od->nbs->other_config, "subnet")) { - destroy_lport_addresses(&lrp_networks); return; } - for (size_t i = 0; i < lrp_networks.n_ipv4_addrs; i++) { - uint32_t ip = ntohl(lrp_networks.ipv4_addrs[i].addr); + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { + uint32_t ip = ntohl(op->lrp_networks.ipv4_addrs[i].addr); /* If the router has the first IP address of the subnet, don't add * it to IPAM. We already added this when we initialized IPAM for * the datapath. This will just result in an erroneous message @@ -1887,8 +1868,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op) ipam_insert_ip_for_datapath(op->peer->od, ip); } } - - destroy_lport_addresses(&lrp_networks); } } @@ -2573,6 +2552,7 @@ join_logical_ports(struct northd_input *input_data, } op->n_lsp_addrs++; } + op->n_lsp_non_router_addrs = op->n_lsp_addrs; op->ps_addrs = xmalloc(sizeof *op->ps_addrs * nbsp->n_port_security);
At the point of IPAM configuration all the addresses are already parsed. No need to waste time parsing them again. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- northd/northd.c | 54 ++++++++++++++++--------------------------------- 1 file changed, 17 insertions(+), 37 deletions(-)