> Thanks for the report and the fix.
>
> I think that we can avoid doing the allocations entirely for this case.
> What do you think of the following?  I guess that it moves allocations
> into the lock, but I don't know of a performance bottleneck on this
> lock.
>
> --8<--------------------------cut here-------------------------->8--
>
> From: Ben Pfaff <b...@ovn.org>
> Date: Thu, 12 Oct 2017 10:33:06 -0700
> Subject: [PATCH] netdev: Fix memory leak on error path in
>  netdev_ports_insert().
>
> Reported-by: William Tu <u9012...@gmail.com>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
>  lib/netdev.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index b4e570bafd08..8d611bc826c2 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -2206,27 +2206,24 @@ netdev_ports_insert(struct netdev *netdev, const 
> struct dpif_class *dpif_class,
>                      struct dpif_port *dpif_port)
>  {
>      size_t hash = NETDEV_PORTS_HASH_INT(dpif_port->port_no, dpif_class);
> -    struct port_to_netdev_data *data;
> -    struct ifindex_to_port_data *ifidx;
>      int ifindex = netdev_get_ifindex(netdev);
>
>      if (ifindex < 0) {
>          return ENODEV;
>      }
>
> -    data = xzalloc(sizeof *data);
> -    ifidx = xzalloc(sizeof *ifidx);
> -
>      ovs_mutex_lock(&netdev_hmap_mutex);
>      if (netdev_ports_lookup(dpif_port->port_no, dpif_class)) {
>          ovs_mutex_unlock(&netdev_hmap_mutex);
>          return EEXIST;
>      }
>
> +    struct port_to_netdev_data *data = xzalloc(sizeof *data);
>      data->netdev = netdev_ref(netdev);
>      data->dpif_class = dpif_class;
>      dpif_port_clone(&data->dpif_port, dpif_port);
>
> +    struct ifindex_to_port_data *ifidx = xzalloc(sizeof *ifidx);
>      ifidx->ifindex = ifindex;
>      ifidx->port = dpif_port->port_no;
>
> --
> 2.10.2
>
Thanks for the feedback.
Yes, I think it looks better, and since port insert is not in
performance critical path, allocation inside the mutex should not have
much impact.

Let's me test it and resubmit v2.
William
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to