Re: [ovs-dev] [fd leak v2 2/3] dpif-linux: Generalize dpif_linux_recv_set() to synchronize channels.

[Ben Pfaff]

On Mon, Nov 25, 2013 at 11:31:14AM -0800, Alex Wang wrote:
> LGTM, one comment below,
> 
> 
> 
> > -    if ((dpif->epoll_fd >= 0) == enable) {
> > -        return 0;
> > -    }
> > -
> > -    if (!enable) {
> > -        destroy_channels(dpif);
> > -    } else {
> > -        struct dpif_port_dump port_dump;
> > -        struct dpif_port port;
> > -
> > +    /* To start with, we need an epoll fd. */
> > +    if (dpif->epoll_fd < 0) {
> > +        dpif->epoll_fd = epoll_create(10);
> >          if (dpif->epoll_fd < 0) {
> >              dpif->epoll_fd = epoll_create(10);
> >              if (dpif->epoll_fd < 0) {
> >                  return errno;
> >              }
> >          }
> > +    }
> >
> 
> 
> Do we need two "if (dpif->epoll_fd < 0)" ?

We need two checks, but we don't need two calls to epoll_create()!

I changed this to:
    /* To start with, we need an epoll fd. */
    if (dpif->epoll_fd < 0) {
        dpif->epoll_fd = epoll_create(10);
        if (dpif->epoll_fd < 0) {
            return errno;
        }
    }

Thanks,

Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev