lldp: Change representation of ISID to uint32_t.

It's a lot more straightforward to deal with integer values as integers instead of arrays of bytes. Signed-off-by: Ben Pfaff <[email protected]>
garryling123 · Mar 4, 2015 · 02c842e · 02c842e
1 parent 24885e3
commit 02c842e
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 63 deletions.
diff --git a/lib/lldp/aa-structs.h b/lib/lldp/aa-structs.h
@@ -1,7 +1,8 @@
 /* aa-structs.h */
 /* contains tlv structures for various auto attach functionality */
 
-/* Copyright (c) 2014 Avaya, Inc
+/* Copyright (c) 2015 Nicira, Inc.
+ * Copyright (c) 2014 Avaya, Inc
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -38,7 +39,7 @@ struct lldpd_aa_element_tlv {
 struct lldpd_aa_isid_vlan_map_data {
     uint16_t status;
     uint16_t vlan;
-    uint8_t  isid[3];
+    uint32_t isid;
 };
 
 struct lldpd_aa_isid_vlan_maps_tlv {

diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
@@ -126,6 +126,15 @@ lldp_tlv_put_u32(struct dp_packet *p, uint32_t x)
     dp_packet_put(p, &nx, sizeof nx);
 }
 
+static void
+lldp_tlv_put_isid(struct dp_packet *p, uint32_t isid)
+{
+    uint8_t *data = dp_packet_put_uninit(p, 3);
+    data[0] = isid >> 16;
+    data[1] = isid >> 8;
+    data[2] = isid;
+}
+
 static void
 lldp_tlv_start(struct dp_packet *p, uint8_t tlv, unsigned int *start)
 {
@@ -288,8 +297,7 @@ lldp_send(struct lldpd *global OVS_UNUSED,
                 vlan_isid_map->isid_vlan_data.vlan;
 
             lldp_tlv_put_u16(p, status_vlan_word);
-            dp_packet_put(p, &vlan_isid_map->isid_vlan_data.isid,
-                          sizeof vlan_isid_map->isid_vlan_data.isid);
+            lldp_tlv_put_isid(p, vlan_isid_map->isid_vlan_data.isid);
         }
 
         lldp_tlv_end(p, start);
@@ -540,6 +548,8 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int s,
 
                     num_mappings /= 5; /* Each mapping is 5 Bytes */
                     for(; num_mappings > 0; num_mappings--) {
+                        uint8_t isid[3];
+
                         isid_vlan_map = xzalloc(sizeof *isid_vlan_map);
                         aa_status_vlan_word = PEEK_UINT16;
 
@@ -550,8 +560,9 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int s,
                         /* Vlan is last 12 bits */
                         isid_vlan_map->isid_vlan_data.vlan =
                             aa_status_vlan_word & 0x0FFF;
-                        PEEK_BYTES(&isid_vlan_map->isid_vlan_data.isid,
-                            sizeof isid_vlan_map->isid_vlan_data.isid);
+                        PEEK_BYTES(isid, 3);
+                        isid_vlan_map->isid_vlan_data.isid =
+                            (isid[0] << 16) | (isid[1] << 8) | isid[2];
                         list_push_back(
                             (struct ovs_list *) &port->p_isid_vlan_maps,
                             (struct ovs_list *) isid_vlan_map);

diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
@@ -101,38 +101,6 @@ static struct hmap *const all_mappings OVS_GUARDED_BY(mutex) = &all_mappings__;
 
 static struct lldp_aa_element_system_id system_id_null;
 
-/* Convert an array to an integer.  I-SID are stored in an array of bytes
- * in the LLDP hardware structrure.
- */
-static uint32_t
-array_to_int(uint8_t *array, size_t len)
-{
-    uint32_t res = 0;
-    unsigned int i = 0;
-
-    ovs_assert(len <= sizeof(uint32_t));
-
-    for (i = 0; i < len; i++) {
-        res = res | (array[len - i - 1] << (i * 8));
-    }
-
-    return res;
-}
-
-/* Convert an integer to an array of byte.
- */
-static void
-int_to_array(uint8_t *array, size_t len, uint32_t value)
-{
-    unsigned int i;
-
-    ovs_assert(len <= sizeof(uint32_t));
-
-    for (i = 0; i < len; i++) {
-        array[len - i - 1] = value >> (8 * i);
-    }
-}
-
 /* Convert an LLDP chassis ID to a string.
  */
 static void
@@ -298,8 +266,7 @@ aa_print_isid_status_port_isid(struct lldp *lldp, struct lldpd_port *port)
     }
 
     LIST_FOR_EACH (mapping, m_entries, &port->p_isid_vlan_maps.m_entries) {
-        uint32_t isid = array_to_int(mapping->isid_vlan_data.isid,
-            sizeof mapping->isid_vlan_data.isid);
+        uint32_t isid = mapping->isid_vlan_data.isid;
         struct aa_mapping_internal *m = mapping_find_by_isid(lldp, isid);
 
         VLOG_INFO("h_rport: isid=%u, vlan=%u, status=%d",
@@ -435,9 +402,7 @@ update_mapping_on_lldp(struct lldp *lldp, struct lldpd_hardware *hardware,
         VLOG_INFO("\t\t hardware->h_ifname=%s", hardware->h_ifname);
     }
 
-    int_to_array(lm->isid_vlan_data.isid,
-                 ARRAY_SIZE(lm->isid_vlan_data.isid),
-                 (uint32_t) m->isid);
+    lm->isid_vlan_data.isid = m->isid;
     lm->isid_vlan_data.vlan = m->vlan;
 
     list_push_back(&hardware->h_lport.p_isid_vlan_maps.m_entries,
@@ -623,8 +588,7 @@ aa_mapping_unregister_mapping(struct lldp *lldp,
                         lm_next,
                         m_entries,
                         &hw->h_lport.p_isid_vlan_maps.m_entries) {
-        uint32_t isid = array_to_int(lm->isid_vlan_data.isid,
-                                     sizeof lm->isid_vlan_data.isid);
+        uint32_t isid = lm->isid_vlan_data.isid;
 
         if (isid == (uint32_t) m->isid) {
             VLOG_INFO("\t\t Removing lport, isid=%u, vlan=%u",

diff --git a/tests/test-aa.c b/tests/test-aa.c
@@ -117,12 +117,8 @@ check_received_aa(struct lldpd_port *sport,
                received_map->isid_vlan_data.status);
         assert(smap[i].isid_vlan_data.vlan ==
                received_map->isid_vlan_data.vlan);
-        assert(smap[i].isid_vlan_data.isid[0] ==
-               received_map->isid_vlan_data.isid[0]);
-        assert(smap[i].isid_vlan_data.isid[1] ==
-               received_map->isid_vlan_data.isid[1]);
-        assert(smap[i].isid_vlan_data.isid[2] ==
-               received_map->isid_vlan_data.isid[2]);
+        assert(smap[i].isid_vlan_data.isid ==
+               received_map->isid_vlan_data.isid);
 
         /* Next mapping sent */
         i++;
@@ -191,15 +187,11 @@ test_aa_send(void)
     /* ISID/VLAN mappings */
     map_init[0].isid_vlan_data.status  = 0xC;
     map_init[0].isid_vlan_data.vlan    = 0x64;
-    map_init[0].isid_vlan_data.isid[0] = 1;
-    map_init[0].isid_vlan_data.isid[1] = 2;
-    map_init[0].isid_vlan_data.isid[2] = 3;
+    map_init[0].isid_vlan_data.isid    = 0x010203;
 
     map_init[1].isid_vlan_data.status  = 0xD;
     map_init[1].isid_vlan_data.vlan    = 0xF;
-    map_init[1].isid_vlan_data.isid[0] = 4;
-    map_init[1].isid_vlan_data.isid[1] = 5;
-    map_init[1].isid_vlan_data.isid[2] = 6;
+    map_init[1].isid_vlan_data.isid    = 0x040506;
 
     /* Prepare an empty packet buffer */
     dp_packet_use_stub(&packet, stub, sizeof stub);
@@ -262,15 +254,11 @@ test_aa_send(void)
     /* Populate instance with two auto attach isid/vlan mappings */
     map[0].isid_vlan_data.status  = map_init[0].isid_vlan_data.status;
     map[0].isid_vlan_data.vlan    = map_init[0].isid_vlan_data.vlan;
-    map[0].isid_vlan_data.isid[0] = map_init[0].isid_vlan_data.isid[0];
-    map[0].isid_vlan_data.isid[1] = map_init[0].isid_vlan_data.isid[1];
-    map[0].isid_vlan_data.isid[2] = map_init[0].isid_vlan_data.isid[2];
+    map[0].isid_vlan_data.isid    = map_init[0].isid_vlan_data.isid;
 
     map[1].isid_vlan_data.status  = map_init[1].isid_vlan_data.status;
     map[1].isid_vlan_data.vlan    = map_init[1].isid_vlan_data.vlan;
-    map[1].isid_vlan_data.isid[0] = map_init[1].isid_vlan_data.isid[0];
-    map[1].isid_vlan_data.isid[1] = map_init[1].isid_vlan_data.isid[1];
-    map[1].isid_vlan_data.isid[2] = map_init[1].isid_vlan_data.isid[2];
+    map[1].isid_vlan_data.isid    = map_init[1].isid_vlan_data.isid;
 
     list_init(&hw->h_lport.p_isid_vlan_maps.m_entries);
     list_push_back(&hw->h_lport.p_isid_vlan_maps.m_entries,