[ovs-dev] [PATCH v3 08/41] ofproto: Use list when handling monitor requests

Ben Pfaff blp at nicira.com
Tue Jun 24 14:50:24 UTC 2014


On Tue, Jun 24, 2014 at 05:22:08PM +0900, Simon Horman wrote:
> On Mon, Jun 23, 2014 at 05:00:53PM -0700, Ben Pfaff wrote:
> > On Mon, Jun 16, 2014 at 11:29:28AM +0900, Simon Horman wrote:
> > > Use a list rather than an array to track monitor requests
> > > in handle_flow_monitor_request().
> > > 
> > > This is in preparation for supporting OpenFlow1.4 flow monitor requests
> > > with delete and modify commands.
> > > 
> > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > 
> > I took a really cursory look at later patches and didn't immediately
> > spot how the change from an array to a list helped.  Can you explain?
> 
> It is used by "ofproto: Handle monitor and delete commands in flow monitor
> requests" which unlinks requests if it is added or modified and then
> subsequently deleted in the same request message.
> 
> The unlink occurs through the call to flow_monitor_delete()
> which in turn calls ofmonitor_destroy().

Thanks for explaining.

I think I made a number of suggestions on the first several patches.  I
guess that you will consider them and repost?

> > Instead of list_insert(), I would use list_push_back().  It has the
> > same effect but the meaning is more obvious:
> > > +        list_insert(&monitor_list, &m->list_node);
> 
> Thanks. I don't think the order isn't important, but I'm happy to
> use list_push_back all the same.

Thanks.



More information about the dev mailing list