Re: [ovs-dev] [bond megaflow v2 1/5] ofproto-dpif: Added Per backer recirculation ID management

2014-03-19 Thread Andy Zhou
On Mon, Mar 17, 2014 at 3:50 PM, Ben Pfaff  wrote:
> On Tue, Mar 11, 2014 at 04:56:17PM -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 
>
> It looks like this code isn't very tightly tied to ofproto-dpif.c.
> That file is already too big, so it might be a good idea to put it in
> a file of its own.  (Did I suggest that before?  It sounds familiar.)
>
> Do we expect to add more data to rid_node sometime later?  Otherwise
> this representation may be more expensive than a bitmap of free rids.

Nothing planed, but I thought it will make things easier down the
road. For example,
I'd like to store information how the recirc-id is being used, so that
we could display actions as 'action: bond0',
instead of 'action: recirc(300)',  whenever make sense.
>
> There is an extra set of parentheses here in recirc_id_alloc():
> +if (!(ridset_find(set, set->next_free_id))) {
> and here:
> +if ((ridset_find(set, id)))
>
> Also in recirc_id_alloc(), missing {}:
> +if (set->n_ids == 0)
> +return 0;
> and here:
> +if ((ridset_find(set, id)))
> +goto found_free_id;

I will fix those
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [bond megaflow v2 1/5] ofproto-dpif: Added Per backer recirculation ID management

2014-03-17 Thread Ben Pfaff
On Tue, Mar 11, 2014 at 04:56:17PM -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 

It looks like this code isn't very tightly tied to ofproto-dpif.c.
That file is already too big, so it might be a good idea to put it in
a file of its own.  (Did I suggest that before?  It sounds familiar.)

Do we expect to add more data to rid_node sometime later?  Otherwise
this representation may be more expensive than a bitmap of free rids.

There is an extra set of parentheses here in recirc_id_alloc():
+if (!(ridset_find(set, set->next_free_id))) {
and here:
+if ((ridset_find(set, id)))

Also in recirc_id_alloc(), missing {}:
+if (set->n_ids == 0)
+return 0;
and here:
+if ((ridset_find(set, id)))
+goto found_free_id;
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev