[ovs-dev] [cmap 2/2] dpif-netdev: Use cmap for ports.

Ben Pfaff blp at nicira.com
Thu May 8 23:41:11 UTC 2014


On Fri, May 02, 2014 at 11:42:46AM -0700, Jarno Rajahalme wrote:
> 
> On May 1, 2014, at 5:14 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > Signed-off-by: Ben Pfaff <blp at nicira.com>

Thanks.

I fixed the OVS_REQUIRES annotations that you pointed out.

> > static int
> > get_port_by_name(struct dp_netdev *dp,
> >                  const char *devname, struct dp_netdev_port **portp)
> > -    OVS_REQ_RDLOCK(dp->port_rwlock)
> > {
> >     struct dp_netdev_port *port;
> > +    struct cmap_cursor cursor;
> > 
> > -    HMAP_FOR_EACH (port, node, &dp->ports) {
> > +    CMAP_FOR_EACH (port, node, &cursor, &dp->ports) {
> 
> Would it be possible for the iteration to miss a port if the camp is
> being modified at the same time with iteration? E.g., a node is
> moved from a ?later? bucket to an ?earlier? one, so that the node is
> not seen by the iteration at all?

Yes, this is possible.

This isn't a fast path, so I changed the code back to take
dp->port_mutex.

> (It is also possible that all modifications to ports map come from
> the same (main) thread)

I don't want to rely on that.

> > @@ -1541,10 +1536,8 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute)
> >     miniflow_initialize(&key.flow, key.buf);
> >     miniflow_extract(execute->packet, md, &key.flow);
> > 
> > -    ovs_rwlock_rdlock(&dp->port_rwlock);
> 
> What was the lock protecting in this case?

OVS_ACTION_ATTR_OUTPUT and OVS_ACTION_ATTR_RECIRC look up ports.



More information about the dev mailing list