On 14/02/2017 01:54, Chandran, Sugesh wrote:


Regards
_Sugesh


-----Original Message-----
From: Roi Dayan [mailto:[email protected]]
Sent: Wednesday, February 8, 2017 3:29 PM
To: [email protected]
Cc: Paul Blakey <[email protected]>; Or Gerlitz
<[email protected]>; Hadar Hen Zion <[email protected]>; Shahar
Klein <[email protected]>; Mark Bloch <[email protected]>; Rony
Efraim <[email protected]>; Fastabend, John R
<[email protected]>; Joe Stringer <[email protected]>; Andy
Gospodarek <[email protected]>; Lance Richardson
<[email protected]>; Marcelo Ricardo Leitner <[email protected]>;
Simon Horman <[email protected]>; Jiri Pirko
<[email protected]>; Chandran, Sugesh <[email protected]>
Subject: [PATCH ovs V3 10/25] netdev-tc-offloads: Add ufid to tc/netdev
map

.....
+
[Sugesh] Good to have comment on each functions explaining about the return 
values and parameters?

ok

+static bool
+del_ufid_tc_mapping(const ovs_u128 *ufid) {
+    size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
+    struct ufid_to_tc_data *data;
+
+    ovs_mutex_lock(&ufid_lock);
+    HMAP_FOR_EACH_WITH_HASH(data, node, hash, &ufid_to_tc) {
+        if (ovs_u128_equals(*ufid, data->ufid)) {
+            break;
+        }
+    }
+    if (data) {
+        hmap_remove(&ufid_to_tc, &data->node);
+        ovs_mutex_unlock(&ufid_lock);
+        netdev_close(data->netdev);
+        free(data);
+        return true;
+    }
+    ovs_mutex_unlock(&ufid_lock);
+    return false;
+}
+
+static ovs_u128 *
+find_ufid(int prio, int handle, struct netdev *netdev) {
+    int ifindex = netdev_get_ifindex(netdev);
+    struct ufid_to_tc_data *data;
+
+    ovs_mutex_lock(&ufid_lock);
+    HMAP_FOR_EACH(data, node, &ufid_to_tc) {
+        if (data->prio == prio && data->handle == handle
+            && netdev_get_ifindex(data->netdev) == ifindex) {
+            break;
+        }
+    }
+    ovs_mutex_unlock(&ufid_lock);
+    if (data) {
+        return &data->ufid;
+    }
+    return NULL;
+}
+
+static int
+get_ufid_tc_mapping(const ovs_u128 *ufid, int *prio, struct netdev
+**netdev) {
+    size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
+    struct ufid_to_tc_data *data;
+
+    ovs_mutex_lock(&ufid_lock);
+    HMAP_FOR_EACH_WITH_HASH(data, node, hash, &ufid_to_tc) {
+        if (ovs_u128_equals(*ufid, data->ufid)) {
+            break;
+        }
+    }
+    ovs_mutex_unlock(&ufid_lock);
+    if (data) {
+        if (prio) {
+            *prio = data->prio;
+        }
+        if (netdev) {
+            *netdev = netdev_ref(data->netdev);
+        }
+        return data->handle;
+    }
+    return 0;
+}
+
[Sugesh] I assume its add +replace than just add? May be its good to add 
comment or changing the function name accordingly.

ok
I'll also reorder a little here. add/del/get/find/

+static bool
+add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
+                    struct netdev *netdev) {
+    size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
+    bool replace = del_ufid_tc_mapping(ufid);
+    struct ufid_to_tc_data *new_data = xzalloc(sizeof *new_data);
+
+    new_data->ufid = *ufid;
+    new_data->prio = prio;
+    new_data->handle = handle;
+    new_data->netdev = netdev_ref(netdev);
+
+    ovs_mutex_lock(&ufid_lock);
+    hmap_insert(&ufid_to_tc, &new_data->node, hash);
+    ovs_mutex_unlock(&ufid_lock);
+
+    return replace;
+}
+
 int
 netdev_tc_flow_flush(struct netdev *netdev)  {
--
2.7.4

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to