Skip to content

Commit

Permalink
binding.c: handle localnet port only if it is on local_datapaths.
Browse files Browse the repository at this point in the history
Today the localnet ports are added to local_lports unconditionally.
This has several side-effects:

- When ovn-monitor-all is true, all localnet ports are added to
  local_lports, including the ones on the non-local logical switches,
  which is undesirable, although there is no direct consequences yet.

- When ovn-monitor-all is false (i.e. conditional monitoring is
  enabled), only localnet ports of local datapaths are supposed to be
  monitored. However, because of the issue discussed at [0], conditional
  monitoring doesn't take effect initially, and all the localnet ports
  are monitored and added to local_lports. Later when monitor condition
  is set for port_bindings, all the localnet ports are added to the
  condition, which causes scale problem e.g. in ovn-k8s environment
  where each node has at least one localnet port, and the SB DB servers
  are overloaded when handling such huge conditions from every node.

[0] https://mail.openvswitch.org/pipermail/ovs-dev/2023-July/406201.html

Signed-off-by: Han Zhou <hzhou@ovn.org>
Acked-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
hzhou8 committed Aug 12, 2023
1 parent 4433406 commit 24da428
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions controller/binding.c
Original file line number Diff line number Diff line change
Expand Up @@ -1929,6 +1929,12 @@ consider_localnet_lport(const struct sbrec_port_binding *pb,
struct binding_ctx_in *b_ctx_in,
struct binding_ctx_out *b_ctx_out)
{
struct local_datapath *ld
= get_local_datapath(b_ctx_out->local_datapaths,
pb->datapath->tunnel_key);
if (!ld) {
return;
}
/* Add all localnet ports to local_ifaces so that we allocate ct zones
* for them. */
update_local_lports(pb->logical_port, b_ctx_out);
Expand Down Expand Up @@ -2143,7 +2149,6 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
break;

case LP_LOCALNET: {
consider_localnet_lport(pb, b_ctx_in, b_ctx_out);
struct lport *lnet_lport = xmalloc(sizeof *lnet_lport);
lnet_lport->pb = pb;
ovs_list_push_back(&localnet_lports, &lnet_lport->list_node);
Expand All @@ -2169,10 +2174,11 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
&bridge_mappings);

/* Run through each localnet lport list to see if it is a localnet port
* on local datapaths discovered from above loop, and update the
* corresponding local datapath accordingly. */
* on local datapaths discovered from above loop, and handles it
* accordingly. */
struct lport *lnet_lport;
LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) {
consider_localnet_lport(lnet_lport->pb, b_ctx_in, b_ctx_out);
update_ld_localnet_port(lnet_lport->pb, &bridge_mappings,
b_ctx_out->local_datapaths);
free(lnet_lport);
Expand Down Expand Up @@ -3209,6 +3215,7 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
b_ctx_in->sbrec_port_binding_by_datapath) {
enum en_lport_type lport_type = get_lport_type(pb);
if (lport_type == LP_LOCALNET) {
consider_localnet_lport(pb, b_ctx_in, b_ctx_out);
update_ld_localnet_port(pb, &bridge_mappings,
b_ctx_out->local_datapaths);
} else if (lport_type == LP_EXTERNAL) {
Expand Down

0 comments on commit 24da428

Please sign in to comment.