diff options
author | Ben Pfaff <blp@nicira.com> | 2015-03-03 15:36:28 -0800 |
---|---|---|
committer | Ben Pfaff <blp@nicira.com> | 2015-03-03 16:22:12 -0800 |
commit | 72c642e5ab553fd11eb69208b3949065f689f17d (patch) | |
tree | 8c9c0778cc39d2bc45e6976ad138a11b18357bfb | |
parent | 135706b39307f84fb0c9a5d1ffeebb9f7f556b9e (diff) | |
download | openvswitch-72c642e5ab553fd11eb69208b3949065f689f17d.tar.gz |
ovs-lldp: Use better types for ISID and VLANs.
An ISID is 24 bits, so it fits in a uint32_t. A VLAN is 12 bits, so it
fits in a uint16_t. Use these types consistently, instead of int64_t.
This removes a check in aa_mapping_unregister() that seems a little
mysterious to me: it previously checked for ISID and VLAN values >= 0. I
don't see a way that they could be < 0 in this situation though.
Signed-off-by: Ben Pfaff <blp@nicira.com>
-rw-r--r-- | lib/ovs-lldp.c | 69 | ||||
-rw-r--r-- | lib/ovs-lldp.h | 7 | ||||
-rw-r--r-- | vswitchd/bridge.c | 9 |
3 files changed, 34 insertions, 51 deletions
diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c index 220b38396..a0a76aade 100644 --- a/lib/ovs-lldp.c +++ b/lib/ovs-lldp.c @@ -78,8 +78,8 @@ enum aa_status { struct aa_mapping_internal { struct hmap_node hmap_node_isid; struct hmap_node hmap_node_aux; - int64_t isid; - int64_t vlan; + uint32_t isid; + uint16_t vlan; void *aux; enum aa_status status; }; @@ -119,14 +119,12 @@ chassisid_to_string(uint8_t *array, size_t len, char **str) /* Find an Auto Attach mapping keyed by I-SID. */ static struct aa_mapping_internal * -mapping_find_by_isid(struct lldp *lldp, const uint64_t isid) +mapping_find_by_isid(struct lldp *lldp, uint32_t isid) OVS_REQUIRES(mutex) { struct aa_mapping_internal *m; - HMAP_FOR_EACH_IN_BUCKET (m, - hmap_node_isid, - hash_bytes(&isid, sizeof isid, 0), + HMAP_FOR_EACH_IN_BUCKET (m, hmap_node_isid, hash_int(isid, 0), &lldp->mappings_by_isid) { if (isid == m->isid) { return m; @@ -323,11 +321,8 @@ aa_print_isid_status(struct ds *ds, struct lldp *lldp) OVS_REQUIRES(mutex) ds_put_format(ds, "-------- ---- ----------- --------\n"); HMAP_FOR_EACH (m, hmap_node_isid, &lldp->mappings_by_isid) { - ds_put_format(ds, "%-8ld %-4ld %-11s %-11s\n", - (long int) m->isid, - (long int) m->vlan, - "Switch", - aa_status_to_str(m->status)); + ds_put_format(ds, "%-8"PRIu32" %-4"PRIu16" %-11s %-11s\n", + m->isid, m->vlan, "Switch", aa_status_to_str(m->status)); } } @@ -520,8 +515,8 @@ aa_mapping_register(void *aux, const struct aa_mapping_settings *s) struct aa_mapping_internal *bridge_m; struct lldp *lldp; - VLOG_INFO("Adding mapping ISID=%ld, VLAN=%ld, aux=%p", (long int) s->isid, - (long int) s->vlan, aux); + VLOG_INFO("Adding mapping ISID=%"PRIu32", VLAN=%"PRIu16", aux=%p", + s->isid, s->vlan, aux); ovs_mutex_lock(&mutex); @@ -534,11 +529,9 @@ aa_mapping_register(void *aux, const struct aa_mapping_settings *s) bridge_m->vlan = s->vlan; bridge_m->aux = aux; bridge_m->status = AA_STATUS_PENDING; - hmap_insert(all_mappings, - &bridge_m->hmap_node_isid, - hash_bytes((const void *) &bridge_m->isid, - sizeof bridge_m->isid, - 0)); + hmap_insert(all_mappings, &bridge_m->hmap_node_isid, + hash_int(bridge_m->isid, 0)); + /* Update mapping on the all the LLDP instances. */ HMAP_FOR_EACH (lldp, hmap_node, all_lldps) { @@ -556,11 +549,8 @@ aa_mapping_register(void *aux, const struct aa_mapping_settings *s) m->vlan = s->vlan; m->status = AA_STATUS_PENDING; m->aux = aux; - hmap_insert(&lldp->mappings_by_isid, - &m->hmap_node_isid, - hash_bytes((const void *) &m->isid, - sizeof m->isid, - 0)); + hmap_insert(&lldp->mappings_by_isid, &m->hmap_node_isid, + hash_int(m->isid, 0)); hmap_insert(&lldp->mappings_by_aux, &m->hmap_node_aux, hash_pointer(m->aux, 0)); @@ -587,7 +577,7 @@ aa_mapping_unregister_mapping(struct lldp *lldp, &hw->h_lport.p_isid_vlan_maps) { uint32_t isid = lm->isid_vlan_data.isid; - if (isid == (uint32_t) m->isid) { + if (isid == m->isid) { VLOG_INFO("\t\t Removing lport, isid=%u, vlan=%u", isid, lm->isid_vlan_data.vlan); @@ -626,17 +616,15 @@ aa_mapping_unregister(void *aux) HMAP_FOR_EACH (lldp, hmap_node, all_lldps) { struct lldpd_hardware *hw; struct aa_mapping_internal *m = mapping_find_by_aux(lldp, aux); - int64_t isid_tmp = -1, vlan_tmp = -1; /* Remove from internal hash tables. */ if (m) { - struct aa_mapping_internal *p = - mapping_find_by_isid(lldp, m->isid); + uint32_t isid = m->isid; + uint16_t vlan = m->vlan; + struct aa_mapping_internal *p = mapping_find_by_isid(lldp, isid); - isid_tmp = m->isid; - vlan_tmp = m->vlan; - VLOG_INFO("\t Removing mapping ISID=%ld, VLAN=%ld (lldp->name=%s)", - (long int) m->isid, (long int) m->vlan, lldp->name); + VLOG_INFO("\t Removing mapping ISID=%"PRIu32", VLAN=%"PRIu16 + " (lldp->name=%s)", isid, vlan, lldp->name); if (p) { hmap_remove(&lldp->mappings_by_isid, &p->hmap_node_isid); @@ -654,13 +642,11 @@ aa_mapping_unregister(void *aux) aa_mapping_unregister_mapping(lldp, hw, m); } - if (isid_tmp >= 0 && vlan_tmp >= 0) { - /* Remove from the all_mappings */ - HMAP_FOR_EACH (m, hmap_node_isid, all_mappings) { - if (m && isid_tmp == m->isid && vlan_tmp == m->vlan) { - hmap_remove(all_mappings, &m->hmap_node_isid); - break; - } + /* Remove from the all_mappings */ + HMAP_FOR_EACH (m, hmap_node_isid, all_mappings) { + if (m && isid == m->isid && vlan == m->vlan) { + hmap_remove(all_mappings, &m->hmap_node_isid); + break; } } } @@ -878,11 +864,8 @@ lldp_create(const struct netdev *netdev, } p = xmemdup(m, sizeof *p); - hmap_insert(&lldp->mappings_by_isid, - &p->hmap_node_isid, - hash_bytes((const void *) &p->isid, - sizeof p->isid, - 0)); + hmap_insert(&lldp->mappings_by_isid, &p->hmap_node_isid, + hash_int(p->isid, 0)); hmap_insert(&lldp->mappings_by_aux, &p->hmap_node_aux, hash_pointer(p->aux, 0)); diff --git a/lib/ovs-lldp.h b/lib/ovs-lldp.h index c629aa095..51d668fba 100644 --- a/lib/ovs-lldp.h +++ b/lib/ovs-lldp.h @@ -1,4 +1,5 @@ /* + * Copyright (c) 2015 Nicira, Inc. * Copyright (c) 2014 Wind River Systems, Inc. * Copyright (c) 2015 Avaya, Inc. * @@ -64,8 +65,8 @@ struct aa_settings { /* Configuration of Auto Attach mappings. */ struct aa_mapping_settings { - int64_t isid; - int64_t vlan; + uint32_t isid; + uint16_t vlan; }; enum bridge_aa_vlan_oper { @@ -80,7 +81,7 @@ enum bridge_aa_vlan_oper { struct bridge_aa_vlan { struct ovs_list list_node; char *port_name; - uint32_t vlan; + uint16_t vlan; enum bridge_aa_vlan_oper oper; }; diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index bc785d7b1..f4ed17422 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -143,8 +143,8 @@ struct bridge { struct aa_mapping { struct hmap_node hmap_node; /* In struct bridge's "mappings" hmap. */ struct bridge *bridge; - int64_t isid; - int64_t vlan; + uint32_t isid; + uint16_t vlan; char *br_name; }; @@ -3792,9 +3792,8 @@ bridge_configure_aa(struct bridge *br) atom.integer = m->isid; if (ovsdb_datum_find_key(mc, &atom, OVSDB_TYPE_UUID) == UINT_MAX) { - VLOG_INFO("Deleting isid=%"PRId64", vlan=%"PRId64, - m->isid, - m->vlan); + VLOG_INFO("Deleting isid=%"PRIu32", vlan=%"PRIu16, + m->isid, m->vlan); bridge_aa_mapping_destroy(m); } } |