Skip to content

Commit

Permalink
controller: Avoid quadratic complexity for multi-chassis ports.
Browse files Browse the repository at this point in the history 
The processing for ports had a quadratic complexity when MTU change
was involved. Avoid the unnecessary processing and do the lookup loop
only once and only in case it was actually needed. In addition to the
quadratic complexity the condition to do the flow recompute was wrong
and this resulted for the lookup in physical_handle_flows_for_lport()
to run for all ports.

The performance difference is very noticeable, see the number below
in a test with 800 LSPs:
Before:
physical_flow_output, handler for input if_status_mgr took 2072ms

After:
physical_flow_output, handler for input if_status_mgr took 4ms

Fixes: cdd8dea ("Track interface MTU in if-status-mgr")
Fixes: 7084cf4 ("Always funnel multichassis port traffic through tunnels")
Co-authored-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 16836c3)
  • Loading branch information
@almusil @numansiddique
almusil authored and numansiddique committed Sep 20, 2024
1 parent 8b3e276 commit b3e47dc
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 43 deletions.
17 changes: 1 addition & 16 deletions controller/ovn-controller.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4456,22 +4456,7 @@ pflow_output_if_status_mgr_handler(struct engine_node *node,
}
if (pb->n_additional_chassis) {
/* Update flows for all ports in datapath. */
struct sbrec_port_binding *target =
sbrec_port_binding_index_init_row(
p_ctx.sbrec_port_binding_by_datapath);
sbrec_port_binding_index_set_datapath(target, pb->datapath);

const struct sbrec_port_binding *binding;
SBREC_PORT_BINDING_FOR_EACH_EQUAL (
binding, target, p_ctx.sbrec_port_binding_by_datapath) {
bool removed = sbrec_port_binding_is_deleted(binding);
if (!physical_handle_flows_for_lport(binding, removed, &p_ctx,
&pfo->flow_table)) {
destroy_physical_ctx(&p_ctx);
return false;
}
}
sbrec_port_binding_index_destroy_row(target);
physical_multichassis_reprocess(pb, &p_ctx, &pfo->flow_table);
} else {
/* If any multichassis ports, update flows for the port. */
bool removed = sbrec_port_binding_is_deleted(pb);
Expand Down
49 changes: 22 additions & 27 deletions controller/physical.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2403,33 +2403,9 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
}
}

if (ldp) {
bool multichassis_state_changed = (
!!pb->additional_chassis ==
!!shash_find(&ldp->multichassis_ports, pb->logical_port)
);
if (multichassis_state_changed) {
if (pb->additional_chassis) {
add_local_datapath_multichassis_port(
ldp, pb->logical_port, pb);
} else {
remove_local_datapath_multichassis_port(
ldp, pb->logical_port);
}

struct sbrec_port_binding *target =
sbrec_port_binding_index_init_row(
p_ctx->sbrec_port_binding_by_datapath);
sbrec_port_binding_index_set_datapath(target, ldp->datapath);

const struct sbrec_port_binding *port;
SBREC_PORT_BINDING_FOR_EACH_EQUAL (
port, target, p_ctx->sbrec_port_binding_by_datapath) {
ofctrl_remove_flows(flow_table, &port->header_.uuid);
physical_eval_port_binding(p_ctx, port, flow_table);
}
sbrec_port_binding_index_destroy_row(target);
}
if (sbrec_port_binding_is_updated(
pb, SBREC_PORT_BINDING_COL_ADDITIONAL_CHASSIS) || removed) {
physical_multichassis_reprocess(pb, p_ctx, flow_table);
}

if (!removed) {
Expand All @@ -2446,6 +2422,25 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
return true;
}

void
physical_multichassis_reprocess(const struct sbrec_port_binding *pb,
struct physical_ctx *p_ctx,
struct ovn_desired_flow_table *flow_table)
{
struct sbrec_port_binding *target =
sbrec_port_binding_index_init_row(
p_ctx->sbrec_port_binding_by_datapath);
sbrec_port_binding_index_set_datapath(target, pb->datapath);

const struct sbrec_port_binding *port;
SBREC_PORT_BINDING_FOR_EACH_EQUAL (port, target,
p_ctx->sbrec_port_binding_by_datapath) {
ofctrl_remove_flows(flow_table, &port->header_.uuid);
physical_eval_port_binding(p_ctx, port, flow_table);
}
sbrec_port_binding_index_destroy_row(target);
}

void
physical_handle_mc_group_changes(struct physical_ctx *p_ctx,
struct ovn_desired_flow_table *flow_table)
Expand Down
3 changes: 3 additions & 0 deletions controller/physical.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,7 @@ bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
bool removed,
struct physical_ctx *,
struct ovn_desired_flow_table *);
void physical_multichassis_reprocess(const struct sbrec_port_binding *,
struct physical_ctx *,
struct ovn_desired_flow_table *);
#endif /* controller/physical.h */
67 changes: 67 additions & 0 deletions tests/ovn.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38944,3 +38944,70 @@ OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected-vif1])

AT_CLEANUP
])


OVN_FOR_EACH_NORTHD([
AT_SETUP([Multichassis port I-P processing])
ovn_start

net_add n1

sim_add hv1
as hv1
check ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.11
check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys

sim_add hv2
as hv2
check ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.12
check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys

check ovn-nbctl ls-add ls
check ovn-nbctl lsp-add ls multi
check ovn-nbctl lsp-set-options multi requested-chassis=hv1

check ovn-nbctl lsp-add ls ln
check ovn-nbctl lsp-set-type ln localnet
check ovn-nbctl lsp-set-addresses ln unknown
check ovn-nbctl lsp-set-options ln network_name=phys

check ovn-nbctl lsp-add ls lsp1 \
-- lsp-set-options lsp1 requested-chassis=hv1
as hv1 check ovs-vsctl -- add-port br-int lsp1 \
-- set Interface lsp1 external-ids:iface-id=lsp1

for hv in hv1 hv2; do
as $hv check ovs-vsctl -- add-port br-int multi \
-- set Interface multi external-ids:iface-id=multi
done

wait_for_ports_up
ovn-nbctl --wait=hv sync

OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_OUTPUT_LARGE_PKT_DETECT | grep -c check_pkt_larger) -eq 0])

check ovn-nbctl --wait=hv lsp-set-options multi requested-chassis=hv1,hv2
OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_OUTPUT_LARGE_PKT_DETECT | grep -c check_pkt_larger) -eq 4])

check ovn-nbctl --wait=hv lsp-set-options multi requested-chassis=hv1
OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_OUTPUT_LARGE_PKT_DETECT | grep -c check_pkt_larger) -eq 0])

check ovn-nbctl --wait=hv lsp-set-options multi requested-chassis=hv1,hv2
OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_OUTPUT_LARGE_PKT_DETECT | grep -c check_pkt_larger) -eq 4])

as hv2 check ovs-vsctl del-port multi
OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_OUTPUT_LARGE_PKT_DETECT | grep -c check_pkt_larger) -eq 0])

as hv2 check ovs-vsctl -- add-port br-int multi \
-- set Interface multi external-ids:iface-id=multi
OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_OUTPUT_LARGE_PKT_DETECT | grep -c check_pkt_larger) -eq 4])

check ovn-nbctl --wait=hv lsp-del multi
OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_OUTPUT_LARGE_PKT_DETECT | grep -c check_pkt_larger) -eq 0])

OVN_CLEANUP([hv1],[hv2])

AT_CLEANUP
])

0 comments on commit b3e47dc

Please sign in to comment.