Skip to content

Commit

Permalink
controller: avoid extra flows if localnet_learn_fdb is disabled
Browse files Browse the repository at this point in the history 
Those extra flows are added when 1st localnet_learn_fdb is enabled.
They are however not removed if/when last localnet_learn_fdb got disabled.

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Reviewed-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
@simonartxavier @dceara
simonartxavier authored and dceara committed Nov 24, 2023
1 parent 2acf91e commit f3a1490
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 18 deletions.
12 changes: 12 additions & 0 deletions controller/binding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1887,6 +1887,18 @@ consider_localnet_lport(const struct sbrec_port_binding *pb,
if (!ld) {
return;
}

bool pb_localnet_learn_fdb = smap_get_bool(&pb->options,
"localnet_learn_fdb", false);
if (pb_localnet_learn_fdb != b_ctx_out->localnet_learn_fdb) {
b_ctx_out->localnet_learn_fdb = pb_localnet_learn_fdb;
if (b_ctx_out->tracked_dp_bindings) {
b_ctx_out->localnet_learn_fdb_changed = true;
tracked_datapath_lport_add(pb, TRACKED_RESOURCE_UPDATED,
b_ctx_out->tracked_dp_bindings);
}
}

/* 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
3 changes: 3 additions & 0 deletions controller/binding.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ struct binding_ctx_out {
struct if_status_mgr *if_mgr;

struct sset *postponed_ports;

bool localnet_learn_fdb;
bool localnet_learn_fdb_changed;
};

/* Local bindings. binding.c module binds the logical port (represented by
Expand Down
28 changes: 21 additions & 7 deletions controller/lflow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2066,7 +2066,8 @@ static void
consider_fdb_flows(const struct sbrec_fdb *fdb,
const struct hmap *local_datapaths,
struct ovn_desired_flow_table *flow_table,
struct ovsdb_idl_index *sbrec_port_binding_by_key)
struct ovsdb_idl_index *sbrec_port_binding_by_key,
bool localnet_learn_fdb)
{
struct local_datapath *ld = get_local_datapath(local_datapaths,
fdb->dp_key);
Expand Down Expand Up @@ -2108,7 +2109,7 @@ consider_fdb_flows(const struct sbrec_fdb *fdb,
fdb->header_.uuid.parts[0], &lookup_match, &ofpacts,
&fdb->header_.uuid);

if (is_vif) {
if (is_vif && localnet_learn_fdb) {
struct match lookup_match_vif = MATCH_CATCHALL_INITIALIZER;
match_set_metadata(&lookup_match_vif, htonll(fdb->dp_key));
match_set_dl_src(&lookup_match_vif, mac);
Expand All @@ -2128,12 +2129,13 @@ static void
add_fdb_flows(const struct sbrec_fdb_table *fdb_table,
const struct hmap *local_datapaths,
struct ovn_desired_flow_table *flow_table,
struct ovsdb_idl_index *sbrec_port_binding_by_key)
struct ovsdb_idl_index *sbrec_port_binding_by_key,
bool localnet_learn_fdb)
{
const struct sbrec_fdb *fdb;
SBREC_FDB_TABLE_FOR_EACH (fdb, fdb_table) {
consider_fdb_flows(fdb, local_datapaths, flow_table,
sbrec_port_binding_by_key);
sbrec_port_binding_by_key, localnet_learn_fdb);
}
}

Expand All @@ -2157,7 +2159,8 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out)
l_ctx_out->flow_table);
add_fdb_flows(l_ctx_in->fdb_table, l_ctx_in->local_datapaths,
l_ctx_out->flow_table,
l_ctx_in->sbrec_port_binding_by_key);
l_ctx_in->sbrec_port_binding_by_key,
l_ctx_in->localnet_learn_fdb);
add_port_sec_flows(l_ctx_in->binding_lports, l_ctx_in->chassis,
l_ctx_out->flow_table);
}
Expand Down Expand Up @@ -2248,7 +2251,8 @@ lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp,
l_ctx_in->sbrec_fdb_by_dp_key) {
consider_fdb_flows(fdb_row, l_ctx_in->local_datapaths,
l_ctx_out->flow_table,
l_ctx_in->sbrec_port_binding_by_key);
l_ctx_in->sbrec_port_binding_by_key,
l_ctx_in->localnet_learn_fdb);
}
sbrec_fdb_index_destroy_row(fdb_index_row);

Expand Down Expand Up @@ -2312,6 +2316,15 @@ lflow_handle_flows_for_lport(const struct sbrec_port_binding *pb,
pb->logical_port)) {
consider_port_sec_flows(pb, l_ctx_out->flow_table);
}
if (l_ctx_in->localnet_learn_fdb_changed && l_ctx_in->localnet_learn_fdb) {
const struct sbrec_fdb *fdb;
SBREC_FDB_TABLE_FOR_EACH (fdb, l_ctx_in->fdb_table) {
consider_fdb_flows(fdb, l_ctx_in->local_datapaths,
l_ctx_out->flow_table,
l_ctx_in->sbrec_port_binding_by_key,
l_ctx_in->localnet_learn_fdb);
}
}
return true;
}

Expand Down Expand Up @@ -2442,7 +2455,8 @@ lflow_handle_changed_fdbs(struct lflow_ctx_in *l_ctx_in,
UUID_ARGS(&fdb->header_.uuid));
consider_fdb_flows(fdb, l_ctx_in->local_datapaths,
l_ctx_out->flow_table,
l_ctx_in->sbrec_port_binding_by_key);
l_ctx_in->sbrec_port_binding_by_key,
l_ctx_in->localnet_learn_fdb);
}

return true;
Expand Down
2 changes: 2 additions & 0 deletions controller/lflow.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ struct lflow_ctx_in {
const struct flow_collector_ids *collector_ids;
const struct hmap *local_lbs;
bool lb_hairpin_use_ct_mark;
bool localnet_learn_fdb;
bool localnet_learn_fdb_changed;
};

struct lflow_ctx_out {
Expand Down
10 changes: 10 additions & 0 deletions controller/ovn-controller.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,8 @@ struct ed_type_runtime_data {
/* Tracked data. See below for more details and comments. */
bool tracked;
bool local_lports_changed;
bool localnet_learn_fdb;
bool localnet_learn_fdb_changed;
struct hmap tracked_dp_bindings;

struct shash local_active_ports_ipv6_pd;
Expand Down Expand Up @@ -1697,6 +1699,8 @@ init_binding_ctx(struct engine_node *node,
b_ctx_out->postponed_ports = rt_data->postponed_ports;
b_ctx_out->tracked_dp_bindings = NULL;
b_ctx_out->if_mgr = ctrl_ctx->if_mgr;
b_ctx_out->localnet_learn_fdb = rt_data->localnet_learn_fdb;
b_ctx_out->localnet_learn_fdb_changed = false;
}

static void
Expand Down Expand Up @@ -1754,6 +1758,7 @@ en_runtime_data_run(struct engine_node *node, void *data)
}

binding_run(&b_ctx_in, &b_ctx_out);
rt_data->localnet_learn_fdb = b_ctx_out.localnet_learn_fdb;

engine_set_node_state(node, EN_UPDATED);
}
Expand Down Expand Up @@ -1868,9 +1873,12 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data)
}

rt_data->local_lports_changed = b_ctx_out.local_lports_changed;
rt_data->localnet_learn_fdb = b_ctx_out.localnet_learn_fdb;
rt_data->localnet_learn_fdb_changed = b_ctx_out.localnet_learn_fdb_changed;
if (b_ctx_out.related_lports_changed ||
b_ctx_out.non_vif_ports_changed ||
b_ctx_out.local_lports_changed ||
b_ctx_out.localnet_learn_fdb_changed ||
!hmap_is_empty(b_ctx_out.tracked_dp_bindings)) {
engine_set_node_state(node, EN_UPDATED);
}
Expand Down Expand Up @@ -3915,6 +3923,8 @@ init_lflow_ctx(struct engine_node *node,
l_ctx_in->active_tunnels = &rt_data->active_tunnels;
l_ctx_in->related_lport_ids = &rt_data->related_lports.lport_ids;
l_ctx_in->binding_lports = &rt_data->lbinding_data.lports;
l_ctx_in->localnet_learn_fdb = rt_data->localnet_learn_fdb;
l_ctx_in->localnet_learn_fdb_changed = rt_data->localnet_learn_fdb_changed;
l_ctx_in->chassis_tunnels = &non_vif_data->chassis_tunnels;
l_ctx_in->lb_hairpin_use_ct_mark = n_opts->lb_hairpin_use_ct_mark;
l_ctx_in->nd_ra_opts = &fo->nd_ra_opts;
Expand Down
13 changes: 2 additions & 11 deletions tests/ovn.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31469,12 +31469,10 @@ as hv2 ovs-ofctl dump-flows br-int table=72 > hv2_offlows_table72.txt
AT_CAPTURE_FILE([hv1_offlows_table72.txt])
AT_CAPTURE_FILE([hv2_offlows_table72.txt])
AT_CHECK_UNQUOTED([cat hv1_offlows_table72.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
])

AT_CHECK_UNQUOTED([cat hv2_offlows_table72.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
])

Expand All @@ -31499,7 +31497,6 @@ priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:13 actions=load:0x$port_ke
])

AT_CHECK_UNQUOTED([cat hv3_offlows_table72.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
])

Expand Down Expand Up @@ -31550,22 +31547,16 @@ AT_CAPTURE_FILE([hv1_offlows_table72.txt])
AT_CAPTURE_FILE([hv2_offlows_table72.txt])
AT_CAPTURE_FILE([hv3_offlows_table72.txt])
AT_CHECK_UNQUOTED([cat hv1_offlows_table72.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:14 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:14 actions=load:0x1->NXM_NX_REG10[[8]]
])

AT_CHECK_UNQUOTED([cat hv2_offlows_table72.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:14 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:14 actions=load:0x1->NXM_NX_REG10[[8]]
])

AT_CHECK_UNQUOTED([cat hv3_offlows_table72.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg10=0x8000/0x8000,metadata=0x$dp_key,dl_src=50:54:00:00:00:14 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:13 actions=load:0x1->NXM_NX_REG10[[8]]
priority=100,reg14=0x$port_key,metadata=0x$dp_key,dl_src=50:54:00:00:00:14 actions=load:0x1->NXM_NX_REG10[[8]]
])
Expand Down Expand Up @@ -37196,8 +37187,8 @@ for i in 2 3; do
done
wait_column "$vif21_key" fdb port_key mac='"00:00:00:00:10:21"'

check_flow_count hv1 4
check_flow_count hv2 4
check_flow_count hv1 2
check_flow_count hv2 2

# We now enable localnet_learn_fdb
# We check how it behaves with existing vif entries in fdb
Expand Down

0 comments on commit f3a1490

Please sign in to comment.