On 12/09/2015 06:38 AM, Ben Pfaff wrote:
> This was more or less implemented inside lflow.c until now, but some
> upcoming code that shouldn't be in that file needs to use it too.
>
> This also adds a second index on lports, so that lports can be looked up
> based on the logical datapath tunnel key and the logical port tunnel key.
> An upcoming commit will add a user for this new index.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> ovn/controller/automake.mk | 2 +
> ovn/controller/lflow.c | 165
> +++++++++-------------------------------
> ovn/controller/lflow.h | 7 +-
> ovn/controller/lport.c | 157 ++++++++++++++++++++++++++++++++++++++
> ovn/controller/lport.h | 69 +++++++++++++++++
> ovn/controller/ovn-controller.c | 33 ++++----
> 6 files changed, 288 insertions(+), 145 deletions(-)
> create mode 100644 ovn/controller/lport.c
> create mode 100644 ovn/controller/lport.h
>
> diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
> index cadfa9c..cf57bbd 100644
> --- a/ovn/controller/automake.mk
> +++ b/ovn/controller/automake.mk
> @@ -8,6 +8,8 @@ ovn_controller_ovn_controller_SOURCES = \
> ovn/controller/encaps.h \
> ovn/controller/lflow.c \
> ovn/controller/lflow.h \
> + ovn/controller/lport.c \
> + ovn/controller/lport.h \
> ovn/controller/ofctrl.c \
> ovn/controller/ofctrl.h \
> ovn/controller/pinctrl.c \
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index dcabe2f..2bfc9e1 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -15,12 +15,13 @@
>
> #include <config.h>
> #include "lflow.h"
> +#include "lport.h"
> #include "dynamic-string.h"
> #include "ofctrl.h"
> #include "ofp-actions.h"
> #include "ofpbuf.h"
> #include "openvswitch/vlog.h"
> -#include "ovn/controller/ovn-controller.h"
> +#include "ovn-controller.h"
> #include "ovn/lib/actions.h"
> #include "ovn/lib/expr.h"
> #include "ovn/lib/ovn-sb-idl.h"
> @@ -42,8 +43,8 @@ add_logical_register(struct shash *symtab, enum mf_field_id
> id)
> expr_symtab_add_field(symtab, name, id, NULL, false);
> }
>
> -static void
> -symtab_init(void)
> +void
> +lflow_init(void)
> {
> shash_init(&symtab);
>
> @@ -147,149 +148,48 @@ symtab_init(void)
> expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
> }
>
> -/* Logical datapaths and logical port numbers. */
> -
> -/* A logical datapath.
> - *
> - * 'ports' maps 'logical_port' names to 'tunnel_key' values in the OVN_SB
> - * Port_Binding table within the logical datapath. */
> -struct logical_datapath {
> - struct hmap_node hmap_node; /* Indexed on 'uuid'. */
> - struct uuid uuid; /* UUID from Datapath_Binding row. */
> - uint32_t tunnel_key; /* 'tunnel_key' from Datapath_Binding row. */
> - struct simap ports; /* Logical port name to port number. */
> +struct lookup_port_aux {
> + const struct lport_index *lports;
> + const struct mcgroup_index *mcgroups;
> + const struct sbrec_datapath_binding *dp;
> };
>
> -/* Contains "struct logical_datapath"s. */
> -static struct hmap logical_datapaths = HMAP_INITIALIZER(&logical_datapaths);
> -
> -/* Finds and returns the logical_datapath for 'binding', or NULL if no such
> - * logical_datapath exists. */
> -static struct logical_datapath *
> -ldp_lookup(const struct sbrec_datapath_binding *binding)
> -{
> - struct logical_datapath *ldp;
> - HMAP_FOR_EACH_IN_BUCKET (ldp, hmap_node,
> uuid_hash(&binding->header_.uuid),
> - &logical_datapaths) {
> - if (uuid_equals(&ldp->uuid, &binding->header_.uuid)) {
> - return ldp;
> - }
> - }
> - return NULL;
> -}
> -
> -/* Creates a new logical_datapath for the given 'binding'. */
> -static struct logical_datapath *
> -ldp_create(const struct sbrec_datapath_binding *binding)
> -{
> - struct logical_datapath *ldp;
> -
> - ldp = xmalloc(sizeof *ldp);
> - hmap_insert(&logical_datapaths, &ldp->hmap_node,
> - uuid_hash(&binding->header_.uuid));
> - ldp->uuid = binding->header_.uuid;
> - ldp->tunnel_key = binding->tunnel_key;
> - simap_init(&ldp->ports);
> - return ldp;
> -}
> -
> -static struct logical_datapath *
> -ldp_lookup_or_create(const struct sbrec_datapath_binding *binding)
> -{
> - struct logical_datapath *ldp = ldp_lookup(binding);
> - return ldp ? ldp : ldp_create(binding);
> -}
> -
> -static void
> -ldp_free(struct logical_datapath *ldp)
> -{
> - simap_destroy(&ldp->ports);
> - hmap_remove(&logical_datapaths, &ldp->hmap_node);
> - free(ldp);
> -}
> -
> -/* Iterates through all of the records in the Port_Binding table, updating
> the
> - * table of logical_datapaths to match the values found in active
> - * Port_Bindings. */
> -static void
> -ldp_run(struct controller_ctx *ctx)
> +static bool
> +lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
> {
> - struct logical_datapath *ldp;
> - HMAP_FOR_EACH (ldp, hmap_node, &logical_datapaths) {
> - simap_clear(&ldp->ports);
> - }
> -
> - const struct sbrec_port_binding *binding;
> - SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> - struct logical_datapath *ldp =
> ldp_lookup_or_create(binding->datapath);
> + const struct lookup_port_aux *aux = aux_;
>
> - simap_put(&ldp->ports, binding->logical_port, binding->tunnel_key);
> + const struct sbrec_port_binding *pb
> + = lport_lookup_by_name(aux->lports, port_name);
> + if (pb && pb->datapath == aux->dp) {
> + *portp = pb->tunnel_key;
> + return true;
> }
>
> - const struct sbrec_multicast_group *mc;
> - SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->ovnsb_idl) {
> - struct logical_datapath *ldp = ldp_lookup_or_create(mc->datapath);
> - simap_put(&ldp->ports, mc->name, mc->tunnel_key);
> + const struct sbrec_multicast_group *mg
> + = mcgroup_lookup_by_dp_name(aux->mcgroups, aux->dp, port_name);
> + if (mg) {
> + *portp = mg->tunnel_key;
> + return true;
> }
>
> - struct logical_datapath *next_ldp;
> - HMAP_FOR_EACH_SAFE (ldp, next_ldp, hmap_node, &logical_datapaths) {
> - if (simap_is_empty(&ldp->ports)) {
> - ldp_free(ldp);
> - }
> - }
> -}
> + VLOG_WARN("unknown lport name %s", port_name);
>
I am seeing a lot of this warning log print because 'inport = \"\"' for the
logical flows pertaining to "ARP reply for known IPs".
May be this warning print can be removed.
Thanks
Numan
> -static void
> -ldp_destroy(void)
> -{
> - struct logical_datapath *ldp, *next_ldp;
> - HMAP_FOR_EACH_SAFE (ldp, next_ldp, hmap_node, &logical_datapaths) {
> - ldp_free(ldp);
> - }
> -}
> -
> -void
> -lflow_init(void)
> -{
> - symtab_init();
> -}
> -
> -static bool
> -lookup_port_cb(const void *ldp_, const char *port_name, unsigned int *portp)
> -{
> - const struct logical_datapath *ldp = ldp_;
> - const struct simap_node *node = simap_find(&ldp->ports, port_name);
> - if (!node) {
> - return false;
> - }
> - *portp = node->data;
> - return true;
> + return false;
> }
>
> /* Translates logical flows in the Logical_Flow table in the OVN_SB database
> * into OpenFlow flows. See ovn-architecture(7) for more information. */
> void
> -lflow_run(struct controller_ctx *ctx, struct hmap *flow_table,
> - const struct simap *ct_zones)
> +lflow_run(struct controller_ctx *ctx, const struct lport_index *lports,
> + const struct mcgroup_index *mcgroups,
> + const struct simap *ct_zones, struct hmap *flow_table)
> {
> struct hmap flows = HMAP_INITIALIZER(&flows);
> uint32_t conj_id_ofs = 1;
>
> - ldp_run(ctx);
> -
> const struct sbrec_logical_flow *lflow;
> SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
> - /* Find the "struct logical_datapath" asssociated with this
> - * Logical_Flow row. If there's no such struct, that must be because
> - * no logical ports are bound to that logical datapath, so there's no
> - * point in maintaining any flows for it anyway, so skip it. */
> - const struct logical_datapath *ldp;
> - ldp = ldp_lookup(lflow->logical_datapath);
> - if (!ldp) {
> - continue;
> - }
> -
> /* Determine translation of logical table IDs to physical table IDs.
> */
> bool ingress = !strcmp(lflow->pipeline, "ingress");
> uint8_t first_ptable = (ingress
> @@ -309,10 +209,15 @@ lflow_run(struct controller_ctx *ctx, struct hmap
> *flow_table,
> char *error;
>
> ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
> + struct lookup_port_aux aux = {
> + .lports = lports,
> + .mcgroups = mcgroups,
> + .dp = lflow->logical_datapath
> + };
> struct action_params ap = {
> .symtab = &symtab,
> .lookup_port = lookup_port_cb,
> - .aux = ldp,
> + .aux = &aux,
> .ct_zones = ct_zones,
>
> .n_tables = LOG_PIPELINE_LEN,
> @@ -353,14 +258,15 @@ lflow_run(struct controller_ctx *ctx, struct hmap
> *flow_table,
>
> expr = expr_simplify(expr);
> expr = expr_normalize(expr);
> - uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, ldp,
> + uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux,
> &matches);
> expr_destroy(expr);
>
> /* Prepare the OpenFlow matches for adding to the flow table. */
> struct expr_match *m;
> HMAP_FOR_EACH (m, hmap_node, &matches) {
> - match_set_metadata(&m->match, htonll(ldp->tunnel_key));
> + match_set_metadata(&m->match,
> + htonll(lflow->logical_datapath->tunnel_key));
> if (m->match.wc.masks.conj_id) {
> m->match.flow.conj_id += conj_id_ofs;
> }
> @@ -398,5 +304,4 @@ void
> lflow_destroy(void)
> {
> expr_symtab_destroy(&symtab);
> - ldp_destroy();
> }
> diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
> index 4a4fa83..b8c53ce 100644
> --- a/ovn/controller/lflow.h
> +++ b/ovn/controller/lflow.h
> @@ -37,6 +37,8 @@
>
> struct controller_ctx;
> struct hmap;
> +struct lport_index;
> +struct mcgroup_index;
> struct simap;
> struct uuid;
>
> @@ -56,8 +58,9 @@ struct uuid;
> #define LOG_PIPELINE_LEN 16
>
> void lflow_init(void);
> -void lflow_run(struct controller_ctx *, struct hmap *flow_table,
> - const struct simap *ct_zones);
> +void lflow_run(struct controller_ctx *, const struct lport_index *,
> + const struct mcgroup_index *, const struct simap *ct_zones,
> + struct hmap *flow_table);
> void lflow_destroy(void);
>
> #endif /* ovn/lflow.h */
> diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
> new file mode 100644
> index 0000000..b9df8bd
> --- /dev/null
> +++ b/ovn/controller/lport.c
> @@ -0,0 +1,157 @@
> +/* Copyright (c) 2015 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +#include "lport.h"
> +#include "hash.h"
> +#include "openvswitch/vlog.h"
> +#include "ovn/lib/ovn-sb-idl.h"
> +
> +VLOG_DEFINE_THIS_MODULE(lport);
> +
> +/* A logical port. */
> +struct lport {
> + struct hmap_node name_node; /* Index by name. */
> + struct hmap_node key_node; /* Index by (dp->tunnel_key, tunnel_key). */
> + const struct sbrec_port_binding *sb;
> +};
> +
> +/* Finds and returns the lport with the given 'name', or NULL if no such
> lport
> + * exists. */
> +const struct sbrec_port_binding *
> +lport_lookup_by_name(const struct lport_index *lports, const char *name)
> +{
> + const struct lport *lport;
> + HMAP_FOR_EACH_WITH_HASH (lport, name_node, hash_string(name, 0),
> + &lports->by_name) {
> + if (!strcmp(lport->sb->logical_port, name)) {
> + return lport->sb;
> + }
> + }
> + return NULL;
> +}
> +
> +const struct sbrec_port_binding *
> +lport_lookup_by_key(const struct lport_index *lports,
> + uint32_t dp_key, uint16_t port_key)
> +{
> + const struct lport *lport;
> + HMAP_FOR_EACH_WITH_HASH (lport, key_node, hash_int(port_key, dp_key),
> + &lports->by_key) {
> + if (port_key == lport->sb->tunnel_key
> + && dp_key == lport->sb->datapath->tunnel_key) {
> + return lport->sb;
> + }
> + }
> + return NULL;
> +}
> +
> +void
> +lport_index_init(struct lport_index *lports, struct ovsdb_idl *ovnsb_idl)
> +{
> + hmap_init(&lports->by_name);
> + hmap_init(&lports->by_key);
> +
> + const struct sbrec_port_binding *sb;
> + SBREC_PORT_BINDING_FOR_EACH (sb, ovnsb_idl) {
> + if (lport_lookup_by_name(lports, sb->logical_port)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "duplicate logical port name '%s'",
> + sb->logical_port);
> + continue;
> + }
> +
> + struct lport *p = xmalloc(sizeof *p);
> + hmap_insert(&lports->by_name, &p->name_node,
> + hash_string(sb->logical_port, 0));
> + hmap_insert(&lports->by_key, &p->key_node,
> + hash_int(sb->tunnel_key, sb->datapath->tunnel_key));
> + p->sb = sb;
> + }
> +}
> +
> +void
> +lport_index_destroy(struct lport_index *lports)
> +{
> + /* Destroy all of the "struct lport"s.
> + *
> + * We don't have to remove the node from both indexes. */
> + struct lport *port, *next;
> + HMAP_FOR_EACH_SAFE (port, next, name_node, &lports->by_name) {
> + hmap_remove(&lports->by_name, &port->name_node);
> + free(port);
> + }
> +
> + hmap_destroy(&lports->by_name);
> + hmap_destroy(&lports->by_key);
> +}
> +
> +struct mcgroup {
> + struct hmap_node dp_name_node; /* Index by (logical datapath, name). */
> + const struct sbrec_multicast_group *sb;
> +};
> +
> +void
> +mcgroup_index_init(struct mcgroup_index *mcgroups, struct ovsdb_idl
> *ovnsb_idl)
> +{
> + hmap_init(&mcgroups->by_dp_name);
> +
> + const struct sbrec_multicast_group *sb;
> + SBREC_MULTICAST_GROUP_FOR_EACH (sb, ovnsb_idl) {
> + const struct uuid *dp_uuid = &sb->datapath->header_.uuid;
> + if (mcgroup_lookup_by_dp_name(mcgroups, sb->datapath, sb->name)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "datapath "UUID_FMT" contains duplicate "
> + "multicast group '%s'", UUID_ARGS(dp_uuid),
> sb->name);
> + continue;
> + }
> +
> + struct mcgroup *m = xmalloc(sizeof *m);
> + hmap_insert(&mcgroups->by_dp_name, &m->dp_name_node,
> + hash_string(sb->name, uuid_hash(dp_uuid)));
> + m->sb = sb;
> + }
> +}
> +
> +void
> +mcgroup_index_destroy(struct mcgroup_index *mcgroups)
> +{
> + struct mcgroup *mcgroup, *next;
> + HMAP_FOR_EACH_SAFE (mcgroup, next, dp_name_node, &mcgroups->by_dp_name) {
> + hmap_remove(&mcgroups->by_dp_name, &mcgroup->dp_name_node);
> + free(mcgroup);
> + }
> +
> + hmap_destroy(&mcgroups->by_dp_name);
> +}
> +
> +const struct sbrec_multicast_group *
> +mcgroup_lookup_by_dp_name(const struct mcgroup_index *mcgroups,
> + const struct sbrec_datapath_binding *dp,
> + const char *name)
> +{
> + const struct uuid *dp_uuid = &dp->header_.uuid;
> + const struct mcgroup *mcgroup;
> + HMAP_FOR_EACH_WITH_HASH (mcgroup, dp_name_node,
> + hash_string(name, uuid_hash(dp_uuid)),
> + &mcgroups->by_dp_name) {
> + if (uuid_equals(&mcgroup->sb->datapath->header_.uuid, dp_uuid)
> + && !strcmp(mcgroup->sb->name, name)) {
> + return mcgroup->sb;
> + }
> + }
> + return NULL;
> +}
> diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h
> new file mode 100644
> index 0000000..661638d
> --- /dev/null
> +++ b/ovn/controller/lport.h
> @@ -0,0 +1,69 @@
> +/* Copyright (c) 2015 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef OVN_LPORT_H
> +#define OVN_LPORT_H 1
> +
> +#include <stdint.h>
> +#include "hmap.h"
> +
> +struct ovsdb_idl;
> +struct sbrec_datapath_binding;
> +
> +/* Logical port and multicast group indexes
> + * ========================================
> + *
> + * This data structure holds multiple indexes over logical ports, to allow
> for
> + * efficient searching for logical ports by name or number.
> + */
> +
> +struct lport_index {
> + struct hmap by_name;
> + struct hmap by_key;
> +};
> +
> +void lport_index_init(struct lport_index *, struct ovsdb_idl *);
> +void lport_index_destroy(struct lport_index *);
> +
> +const struct sbrec_port_binding *lport_lookup_by_name(
> + const struct lport_index *, const char *name);
> +const struct sbrec_port_binding *lport_lookup_by_key(
> + const struct lport_index *, uint32_t dp_key, uint16_t port_key);
> +
> +/* Multicast group index
> + * =====================
> + *
> + * This is separate from the logical port index because of namespace issues:
> + * logical port names are globally unique, but multicast group names are only
> + * unique within the scope of a logical datapath. */
> +
> +/* A multicast group.
> + *
> + * Multicast groups could be indexed by number also, but so far the clients
> do
> + * not need this index. */
> +
> +struct mcgroup_index {
> + struct hmap by_dp_name;
> +};
> +
> +void mcgroup_index_init(struct mcgroup_index *, struct ovsdb_idl *);
> +void mcgroup_index_destroy(struct mcgroup_index *);
> +
> +const struct sbrec_multicast_group *mcgroup_lookup_by_dp_name(
> + const struct mcgroup_index *,
> + const struct sbrec_datapath_binding *,
> + const char *name);
> +
> +#endif /* ovn/lport.h */
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 02ecb3e..f5dbecc 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -23,32 +23,32 @@
> #include <stdlib.h>
> #include <string.h>
>
> +#include "binding.h"
> +#include "chassis.h"
> #include "command-line.h"
> #include "compiler.h"
> #include "daemon.h"
> #include "dirs.h"
> #include "dynamic-string.h"
> +#include "encaps.h"
> +#include "fatal-signal.h"
> +#include "lflow.h"
> +#include "lib/vswitch-idl.h"
> +#include "lport.h"
> +#include "ofctrl.h"
> #include "openvswitch/vconn.h"
> #include "openvswitch/vlog.h"
> #include "ovn/lib/ovn-sb-idl.h"
> +#include "patch.h"
> +#include "physical.h"
> +#include "pinctrl.h"
> #include "poll-loop.h"
> -#include "fatal-signal.h"
> -#include "lib/vswitch-idl.h"
> #include "smap.h"
> -#include "stream.h"
> #include "stream-ssl.h"
> +#include "stream.h"
> #include "unixctl.h"
> #include "util.h"
>
> -#include "ofctrl.h"
> -#include "pinctrl.h"
> -#include "binding.h"
> -#include "chassis.h"
> -#include "encaps.h"
> -#include "patch.h"
> -#include "physical.h"
> -#include "lflow.h"
> -
> VLOG_DEFINE_THIS_MODULE(main);
>
> static unixctl_cb_func ovn_controller_exit;
> @@ -292,18 +292,25 @@ main(int argc, char *argv[])
> }
>
> if (br_int) {
> + struct lport_index lports;
> + struct mcgroup_index mcgroups;
> + lport_index_init(&lports, ctx.ovnsb_idl);
> + mcgroup_index_init(&mcgroups, ctx.ovnsb_idl);
> +
> enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
>
> pinctrl_run(&ctx, br_int);
>
> struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
> - lflow_run(&ctx, &flow_table, &ct_zones);
> + lflow_run(&ctx, &lports, &mcgroups, &ct_zones, &flow_table);
> if (chassis_id) {
> physical_run(&ctx, mff_ovn_geneve,
> br_int, chassis_id, &ct_zones, &flow_table);
> }
> ofctrl_put(&flow_table);
> hmap_destroy(&flow_table);
> + mcgroup_index_destroy(&mcgroups);
> + lport_index_destroy(&lports);
> }
>
> unixctl_server_run(unixctl);
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
