[ovs-dev] [PATCH] ovn: Discard flows for non-local ports.
Russell Bryant
russell at ovn.org
Mon Sep 18 15:25:29 UTC 2017
On Mon, Sep 18, 2017 at 11:24 AM, Russell Bryant <russell at ovn.org> wrote:
> Discard some OpenFlow flows that will never match. This includes
> flows that match on a non-local inport in the ingress pipeline or a
> non-local outport in the egress pipeline of a logical switch.
>
> This is most useful for networks with a large number of ports or ACLs
> that use large address sets.
>
> Signed-off-by: Russell Bryant <russell at ovn.org>
> Tested-by: Miguel Angel Ajo Pelayo <majopela at redhat.com>
> ---
> ovn/controller/binding.c | 29 +++++++++++++++++++++++++----
> ovn/controller/binding.h | 2 +-
> ovn/controller/lflow.c | 33 +++++++++++++++++++++++++++------
> ovn/controller/lflow.h | 3 ++-
> ovn/controller/ovn-controller.c | 9 +++++++--
> 5 files changed, 62 insertions(+), 14 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index ca1d43395..3532c6014 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -371,6 +371,17 @@ setup_qos(const char *egress_iface, struct hmap *queue_map)
> }
>
> static void
> +update_local_lport_ids(struct sset *local_lport_ids,
> + const struct sbrec_port_binding *binding_rec)
> +{
> + char buf[16];
> + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
> + binding_rec->datapath->tunnel_key,
> + binding_rec->tunnel_key);
> + sset_add(local_lport_ids, buf);
> +}
> +
> +static void
> consider_local_datapath(struct controller_ctx *ctx,
> const struct chassis_index *chassis_index,
> struct sset *active_tunnels,
> @@ -379,7 +390,8 @@ consider_local_datapath(struct controller_ctx *ctx,
> struct hmap *qos_map,
> struct hmap *local_datapaths,
> struct shash *lport_to_iface,
> - struct sset *local_lports)
> + struct sset *local_lports,
> + struct sset *local_lport_ids)
> {
> const struct ovsrec_interface *iface_rec
> = shash_find_data(lport_to_iface, binding_rec->logical_port);
> @@ -399,7 +411,7 @@ consider_local_datapath(struct controller_ctx *ctx,
> get_qos_params(binding_rec, qos_map);
> }
> /* This port is in our chassis unless it is a localport. */
> - if (strcmp(binding_rec->type, "localport")) {
> + if (strcmp(binding_rec->type, "localport")) {
> our_chassis = true;
> }
> } else if (!strcmp(binding_rec->type, "l2gateway")) {
> @@ -439,6 +451,14 @@ consider_local_datapath(struct controller_ctx *ctx,
> our_chassis = false;
> }
>
> + if (our_chassis
> + || !strcmp(binding_rec->type, "patch")
> + || !strcmp(binding_rec->type, "localport")
> + || !strcmp(binding_rec->type, "vtep")
> + || !strcmp(binding_rec->type, "localnet")) {
> + update_local_lport_ids(local_lport_ids, binding_rec);
> + }
> +
> if (ctx->ovnsb_idl_txn) {
> const char *vif_chassis = smap_get(&binding_rec->options,
> "requested-chassis");
> @@ -508,7 +528,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis *chassis_rec,
> const struct chassis_index *chassis_index,
> struct sset *active_tunnels,
> - struct hmap *local_datapaths, struct sset *local_lports)
> + struct hmap *local_datapaths, struct sset *local_lports,
> + struct sset *local_lport_ids)
> {
> if (!chassis_rec) {
> return;
> @@ -533,7 +554,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
> active_tunnels, chassis_rec, binding_rec,
> sset_is_empty(&egress_ifaces) ? NULL :
> &qos_map, local_datapaths, &lport_to_iface,
> - local_lports);
> + local_lports, local_lport_ids);
>
> }
>
> diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
> index c78f8d932..89fc2ec8f 100644
> --- a/ovn/controller/binding.h
> +++ b/ovn/controller/binding.h
> @@ -32,7 +32,7 @@ void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis *,
> const struct chassis_index *,
> struct sset *active_tunnels, struct hmap *local_datapaths,
> - struct sset *all_lports);
> + struct sset *local_lports, struct sset *local_lport_ids);
> bool binding_cleanup(struct controller_ctx *, const struct sbrec_chassis *);
>
> #endif /* ovn/binding.h */
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index 6d9f02cb2..c1f5de2ab 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -68,7 +68,8 @@ static void consider_logical_flow(struct controller_ctx *ctx,
> uint32_t *conj_id_ofs,
> const struct shash *addr_sets,
> struct hmap *flow_table,
> - struct sset *active_tunnels);
> + struct sset *active_tunnels,
> + struct sset *local_lport_ids);
>
> static bool
> lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
> @@ -145,7 +146,8 @@ add_logical_flows(struct controller_ctx *ctx,
> const struct sbrec_chassis *chassis,
> const struct shash *addr_sets,
> struct hmap *flow_table,
> - struct sset *active_tunnels)
> + struct sset *active_tunnels,
> + struct sset *local_lport_ids)
> {
> uint32_t conj_id_ofs = 1;
> const struct sbrec_logical_flow *lflow;
> @@ -170,7 +172,8 @@ add_logical_flows(struct controller_ctx *ctx,
> lflow, local_datapaths,
> group_table, chassis,
> &dhcp_opts, &dhcpv6_opts, &conj_id_ofs,
> - addr_sets, flow_table, active_tunnels);
> + addr_sets, flow_table, active_tunnels,
> + local_lport_ids);
> }
>
> dhcp_opts_destroy(&dhcp_opts);
> @@ -189,7 +192,8 @@ consider_logical_flow(struct controller_ctx *ctx,
> uint32_t *conj_id_ofs,
> const struct shash *addr_sets,
> struct hmap *flow_table,
> - struct sset *active_tunnels)
> + struct sset *active_tunnels,
> + struct sset *local_lport_ids)
> {
> /* Determine translation of logical table IDs to physical table IDs. */
> bool ingress = !strcmp(lflow->pipeline, "ingress");
> @@ -301,6 +305,22 @@ consider_logical_flow(struct controller_ctx *ctx,
> if (m->match.wc.masks.conj_id) {
> m->match.flow.conj_id += *conj_id_ofs;
> }
> + if (is_switch(ldp)) {
> + unsigned int reg_index
> + = (ingress ? MFF_LOG_INPORT : MFF_LOG_OUTPORT) - MFF_REG0;
> + int64_t port_id = m->match.flow.regs[reg_index];
> + if (port_id) {
> + int64_t dp_id = lflow->logical_datapath->tunnel_key;
> + char buf[16];
> + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id, port_id);
> + if (!sset_contains(local_lport_ids, buf)) {
> + //VLOG_INFO("Matching on port id %"PRId64" dp %"PRId64", is NOT local", port_id, dp_id);
> + continue;
> + } else {
> + //VLOG_INFO("Matching on port id %"PRId64" dp %"PRId64", is local", port_id, dp_id);
> + }
Consider these debugging lines removed ...
> + }
> + }
> if (!m->n) {
> ofctrl_add_flow(flow_table, ptable, lflow->priority,
> lflow->header_.uuid.parts[0], &m->match, &ofpacts);
> @@ -413,11 +433,12 @@ lflow_run(struct controller_ctx *ctx,
> struct group_table *group_table,
> const struct shash *addr_sets,
> struct hmap *flow_table,
> - struct sset *active_tunnels)
> + struct sset *active_tunnels,
> + struct sset *local_lport_ids)
> {
> add_logical_flows(ctx, chassis_index, local_datapaths,
> group_table, chassis, addr_sets, flow_table,
> - active_tunnels);
> + active_tunnels, local_lport_ids);
> add_neighbor_flows(ctx, flow_table);
> }
>
> diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
> index 07709fe06..bfb7415e2 100644
> --- a/ovn/controller/lflow.h
> +++ b/ovn/controller/lflow.h
> @@ -69,7 +69,8 @@ void lflow_run(struct controller_ctx *,
> struct group_table *group_table,
> const struct shash *addr_sets,
> struct hmap *flow_table,
> - struct sset *active_tunnels);
> + struct sset *active_tunnels,
> + struct sset *local_lport_ids);
> void lflow_destroy(void);
>
> #endif /* ovn/lflow.h */
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index e2c965290..a935a791c 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -670,6 +670,9 @@ main(int argc, char *argv[])
> * l2gateway ports for which options:l2gateway-chassis designates the
> * local hypervisor, and localnet ports. */
> struct sset local_lports = SSET_INITIALIZER(&local_lports);
> + /* Contains the same ports as local_lports, but in the format:
> + * <datapath-tunnel-key>_<port-tunnel-key> */
> + struct sset local_lport_ids = SSET_INITIALIZER(&local_lport_ids);
> struct sset active_tunnels = SSET_INITIALIZER(&active_tunnels);
>
> const struct ovsrec_bridge *br_int = get_br_int(&ctx);
> @@ -686,7 +689,7 @@ main(int argc, char *argv[])
> bfd_calculate_active_tunnels(br_int, &active_tunnels);
> binding_run(&ctx, br_int, chassis,
> &chassis_index, &active_tunnels, &local_datapaths,
> - &local_lports);
> + &local_lports, &local_lport_ids);
> }
> if (br_int && chassis) {
> struct shash addr_sets = SHASH_INITIALIZER(&addr_sets);
> @@ -708,7 +711,8 @@ main(int argc, char *argv[])
> struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
> lflow_run(&ctx, chassis,
> &chassis_index, &local_datapaths, &group_table,
> - &addr_sets, &flow_table, &active_tunnels);
> + &addr_sets, &flow_table, &active_tunnels,
> + &local_lport_ids);
>
> if (chassis_id) {
> bfd_run(&ctx, br_int, chassis, &local_datapaths,
> @@ -764,6 +768,7 @@ main(int argc, char *argv[])
> chassis_index_destroy(&chassis_index);
>
> sset_destroy(&local_lports);
> + sset_destroy(&local_lport_ids);
> sset_destroy(&active_tunnels);
>
> struct local_datapath *cur_node, *next_node;
> --
> 2.13.5
>
--
Russell Bryant
More information about the dev
mailing list