The new helpers get_local_datapath() and get_patched_datapath() make code
a little shorter and easier to read. They also avoid a pitfall that was
present in at least a few of the instances: CONTAINER_OF is not safe on a
null pointer, because it does a raw pointer subtraction and will change
NULL to something else. This wasn't actually a problem in these particular
cases because the value it was subtracting was zero (although arguably it
is still undefined behavior because the compiler is allowed to assume that
a pointer on which arithmetic is performed is nonnull).
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
ovn/controller/binding.c | 4 ++--
ovn/controller/lflow.c | 9 +++------
ovn/controller/ovn-controller.c | 19 +++++++++++++++++++
ovn/controller/ovn-controller.h | 8 +++++++-
ovn/controller/patch.c | 11 +++++------
ovn/controller/physical.c | 16 +++++-----------
6 files changed, 41 insertions(+), 26 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index d3ca9c9..32fcb85 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -125,8 +125,8 @@ static void
add_local_datapath(struct hmap *local_datapaths,
const struct sbrec_port_binding *binding_rec)
{
- if (hmap_first_with_hash(local_datapaths,
- binding_rec->datapath->tunnel_key)) {
+ if (get_local_datapath(local_datapaths,
+ binding_rec->datapath->tunnel_key)) {
return;
}
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index bcad318..40bd134 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -241,15 +241,12 @@ add_logical_flows(struct controller_ctx *ctx, const
struct lport_index *lports,
* large lrouters and lswitches. This need to be studied further.
*/
- struct hmap_node *ld;
- ld = hmap_first_with_hash(local_datapaths, ldp->tunnel_key);
- if (!ld) {
+ if (!get_local_datapath(local_datapaths, ldp->tunnel_key)) {
if (!ingress) {
continue;
}
- struct hmap_node *pd;
- pd = hmap_first_with_hash(patched_datapaths, ldp->tunnel_key);
- if (!pd) {
+ if (!get_patched_datapath(patched_datapaths,
+ ldp->tunnel_key)) {
continue;
}
}
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 6027011..7c68c9d 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -62,6 +62,25 @@ OVS_NO_RETURN static void usage(void);
static char *ovs_remote;
+struct local_datapath *
+get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key)
+{
+ struct hmap_node *node = hmap_first_with_hash(local_datapaths, tunnel_key);
+ return (node
+ ? CONTAINER_OF(node, struct local_datapath, hmap_node)
+ : NULL);
+}
+
+struct patched_datapath *
+get_patched_datapath(const struct hmap *patched_datapaths, uint32_t tunnel_key)
+{
+ struct hmap_node *node = hmap_first_with_hash(patched_datapaths,
+ tunnel_key);
+ return (node
+ ? CONTAINER_OF(node, struct patched_datapath, hmap_node)
+ : NULL);
+}
+
const struct sbrec_chassis *
get_chassis(struct ovsdb_idl *ovnsb_idl, const char *chassis_id)
{
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 9955097..9af7959 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2015 Nicira, Inc.
+/* Copyright (c) 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -41,12 +41,18 @@ struct local_datapath {
const struct sbrec_port_binding *localnet_port;
};
+struct local_datapath *get_local_datapath(const struct hmap *,
+ uint32_t tunnel_key);
+
/* Contains hmap_node whose hash values are the tunnel_key of datapaths
* with at least one logical patch port binding. */
struct patched_datapath {
struct hmap_node hmap_node;
};
+struct patched_datapath *get_patched_datapath(const struct hmap *,
+ uint32_t tunnel_key);
+
const struct ovsrec_bridge *get_bridge(struct ovsdb_idl *,
const char *br_name);
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 943ac99..4808146 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2015 Nicira, Inc.
+/* Copyright (c) 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -176,10 +176,9 @@ add_bridge_mappings(struct controller_ctx *ctx,
const struct sbrec_port_binding *binding;
SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
if (!strcmp(binding->type, "localnet")) {
- struct local_datapath *ld;
- ld = CONTAINER_OF(hmap_first_with_hash(local_datapaths,
- binding->datapath->tunnel_key),
- struct local_datapath, hmap_node);
+ struct local_datapath *ld
+ = get_local_datapath(local_datapaths,
+ binding->datapath->tunnel_key);
if (!ld) {
/* This localnet port is on a datapath with no
* logical ports bound to this chassis, so there's no need
@@ -233,7 +232,7 @@ static void
add_patched_datapath(struct hmap *patched_datapaths,
const struct sbrec_port_binding *binding_rec)
{
- if (hmap_first_with_hash(patched_datapaths,
+ if (get_patched_datapath(patched_datapaths,
binding_rec->datapath->tunnel_key)) {
return;
}
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index d1a41c1..a210b39 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -138,9 +138,8 @@ put_stack(enum mf_field_id field, struct ofpact_stack
*stack)
static const struct sbrec_port_binding*
get_localnet_port(struct hmap *local_datapaths, int64_t tunnel_key)
{
- struct local_datapath *ld;
- ld = CONTAINER_OF(hmap_first_with_hash(local_datapaths, tunnel_key),
- struct local_datapath, hmap_node);
+ struct local_datapath *ld = get_local_datapath(local_datapaths,
+ tunnel_key);
return ld ? ld->localnet_port : NULL;
}
@@ -255,14 +254,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
*/
uint32_t dp_key = binding->datapath->tunnel_key;
uint32_t port_key = binding->tunnel_key;
- struct hmap_node *ld;
- ld = hmap_first_with_hash(local_datapaths, dp_key);
- if (!ld) {
- struct hmap_node *pd;
- pd = hmap_first_with_hash(patched_datapaths, dp_key);
- if (!pd) {
- continue;
- }
+ if (!get_local_datapath(local_datapaths, dp_key)
+ && !get_patched_datapath(patched_datapaths, dp_key)) {
+ continue;
}
/* Find the OpenFlow port for the logical port, as 'ofport'. This is
--
2.1.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
