On Tue, Mar 25, 2014 at 8:35 AM, Ben Pfaff <b...@nicira.com> wrote:
> On Mon, Mar 24, 2014 at 06:58:42PM -0700, Andy Zhou wrote:
>> Recirculation ID needs to be unique per datapath. Its usage will be
>> tracked by the backer that corresponds to the datapath.
>>
>> In theory, Recirculation ID can be any uint32_t value, except 0. This
>> implementation limits to a smaller range just for ease of debugging.
>> Make the range size 0 effectively disables recirculation.
>>
>> Signed-off-by: Andy Zhou <az...@nicira.com>
>
> There's some trailing whitespace:
> /home/blp/nicira/ovs/.git/rebase-apply/patch:79: trailing whitespace.
> struct ovs_mutex lock;
> /home/blp/nicira/ovs/.git/rebase-apply/patch:261: trailing whitespace.
> * ID pool keeps track recirculation ids.
> /home/blp/nicira/ovs/.git/rebase-apply/patch:273: trailing whitespace.
> *
> warning: 3 lines add whitespace errors.
>
> Missing {} here in rid_pool_alloc_id():
> + if (rids->n_ids == 0)
> + return 0;
> also here:
> + if ((rid_pool_find(rids, id)))
> + goto found_free_id;
> Extra () here in rid_pool_alloc_id():
> + if (!(rid_pool_find(rids, rids->next_free_id))) {
> and here:
> + if ((rid_pool_find(rids, id)))
>
> It looks like open_dpif_backer() creates the recirculation pool after it
> starts udpif threads. It might be a good idea to use the other order (I
> guess receiving packets could eventually trigger recirc id allocation).
>
> Acked-by: Ben Pfaff <b...@nicira.com>
>
Thanks. I will apply with following fixes:
diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index e464e4c..9e7e9a3 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -38,7 +38,7 @@ struct rid_pool {
};
struct recirc_id_pool {
- struct ovs_mutex lock;
+ struct ovs_mutex lock;
struct rid_pool rids;
};
@@ -146,8 +146,9 @@ rid_pool_alloc_id(struct rid_pool *rids)
{
uint32_t id;
- if (rids->n_ids == 0)
+ if (rids->n_ids == 0) {
return 0;
+ }
if (!(rid_pool_find(rids, rids->next_free_id))) {
id = rids->next_free_id;
@@ -155,8 +156,9 @@ rid_pool_alloc_id(struct rid_pool *rids)
}
for(id = rids->base; id < rids->base + rids->n_ids; id++) {
- if ((rid_pool_find(rids, id)))
+ if (rid_pool_find(rids, id)) {
goto found_free_id;
+ }
}
/* Not available. */
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 4020205..3344e2a 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -27,7 +27,7 @@ struct recirc_id_pool;
* ======================
*
* Recirculation ID needs to be unique for each datapath. Recirculation
- * ID pool keeps track recirculation ids.
+ * ID pool keeps track recirculation ids.
*
* Typically, there is one recirculation ID pool for each backer.
*
@@ -39,7 +39,7 @@ struct recirc_id_pool;
* =============
*
* All APIs are thread safe.
- *
+ *
*/
struct recirc_id_pool *recirc_id_pool_create(void);
void recirc_id_pool_destroy(struct recirc_id_pool *pool);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 56e0ed7..87a61f7 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -898,13 +898,12 @@ open_dpif_backer(const char *type, struct
dpif_backer **backerp)
}
backer->variable_length_userdata = check_variable_length_userdata(backer);
backer->max_mpls_depth = check_max_mpls_depth(backer);
+ backer->rid_pool = recirc_id_pool_create();
if (backer->recv_set_enable) {
udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
}
- backer->rid_pool = recirc_id_pool_create();
-
return error;
}
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
