From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) by dpdk.org (Postfix) with ESMTP id D5EB15F0D for ; Thu, 20 Sep 2018 09:57:59 +0200 (CEST) Received: by mail-lj1-f196.google.com with SMTP id j19-v6so7483610ljc.7 for ; Thu, 20 Sep 2018 00:57:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=rnYt0apV017IGFU4sMaz4SaYb+ZYZTBf0gxeOQhfl5s=; b=dHOiGCvqndIWbtkDCWIB+p/QB5ngjoMhBEuH9qUDgbHjy3Jnm4+LHTg9r66MILP67g FUeUw+s12oCNC4EzliqunjbMLW522yflKZReHM4G7c3FhDiFVycXdb4GFWevz0hmJbwM RTXvS0/hOH62GDQX7rs/XlIM/lk59BmBq3j5OVyPRs2zgOVG8KyK5jCwGCYNMIBTtejY /8+ovjtaoLb+y8IzLESwXQMuHXE67Tl+bHh/JPLj1BLHfrEOUmXQnjrFq7Ymw4Ni+3kG W820Eo9s0dBWxo8+NK8B+hF4P13vCFDkXNHrdJlpp2TMZzfSlYbriJ33KWgL6sk96q1J pOuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=rnYt0apV017IGFU4sMaz4SaYb+ZYZTBf0gxeOQhfl5s=; b=Ms13C9isXeJ2Q9KB63nV7PfBs1V7IvRbbmbuciPyNQeqmcREkicJj1XtDEWcbEn/UM SkRfLsfmcrmsR7vyE/bXCSSr9vDt61xRsx5+oatRHkGbwk0oYge/+1FwgDjE0NfroNum CapyTyfTIogJRboNEa3e5pXJhtzt7e7ewOqtsjdSdEoPk+FZc1Tuhq8SlrgPdMeb/dnR ZuRa7BJB0ufq/lm26Ywqr9trvQOTmEsBNKXVl2gpkNF+q+d3TYrYPTDN5DAwkM23nhNO tG1kvGzKQ7foPoPyq8uJT3EvH+rG0aeY19kAn6vcU8VQXVRTQppKXlWcylJNHi0EPY6A m9kA== X-Gm-Message-State: APzg51DGSUr2caHAxq65ZG6HUOmjzjjliSdVHc+Dqn0U36iR7nO1uzF3 FXlbkO4wtrRk2oQOOKCMVzP0rg== X-Google-Smtp-Source: ANB0VdYOmzFrlQdS4Mb+UaU9fwK6z0m0ca78ivQNnkeDqkCXUVhtPzr8dMfXfISjNG6BwmvBdzr0iA== X-Received: by 2002:a2e:2e02:: with SMTP id u2-v6mr1808839lju.77.1537430279329; Thu, 20 Sep 2018 00:57:59 -0700 (PDT) Received: from [10.0.0.72] (31-172-191-173.noc.fibertech.net.pl. [31.172.191.173]) by smtp.gmail.com with ESMTPSA id f70-v6sm1543160lfb.6.2018.09.20.00.57.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Sep 2018 00:57:58 -0700 (PDT) To: Stephen Hemminger Cc: dev@dpdk.org, mw@semihalf.com, zr@semihalf.com, tdu@semihalf.com, nadavh@marvell.com References: <1535720386-18775-1-git-send-email-amo@semihalf.com> <1537369294-17099-1-git-send-email-amo@semihalf.com> <1537369294-17099-2-git-send-email-amo@semihalf.com> <20180919092809.7ccde128@xeon-e3> From: Andrzej Ostruszka Message-ID: Date: Thu, 20 Sep 2018 09:57:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180919092809.7ccde128@xeon-e3> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4 1/8] net/mvneta: add neta PMD skeleton X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Sep 2018 07:58:00 -0000 On 19.09.2018 18:28, Stephen Hemminger wrote: > On Wed, 19 Sep 2018 17:01:27 +0200 > Andrzej Ostruszka wrote: > >> +/** >> + * Create private device structure. >> + * >> + * @param dev_name >> + * Pointer to the port name passed in the initialization parameters. >> + * >> + * @return >> + * Pointer to the newly allocated private device structure. >> + */ >> +static struct mvneta_priv * >> +mvneta_priv_create(const char *dev_name) >> +{ >> + struct mvneta_priv *priv; >> + >> + priv = rte_zmalloc_socket(dev_name, sizeof(*priv), 0, rte_socket_id()); >> + if (!priv) >> + return NULL; >> + >> + return priv; >> +} > > Why make this a function, it really doesn't add anything over just doing it inline. True. Removed it. >> +static int >> +mvneta_eth_dev_create(struct rte_vdev_device *vdev, const char *name) >> +{ >> + int ret, fd = socket(AF_INET, SOCK_DGRAM, 0); >> + struct rte_eth_dev *eth_dev; >> + struct mvneta_priv *priv; >> + struct ifreq req; >> + >> + eth_dev = rte_eth_dev_allocate(name); >> + if (!eth_dev) >> + return -ENOMEM; >> + >> + priv = mvneta_priv_create(name); >> + >> + if (!priv) { > > nit: no blank line needed. > >> + ret = -ENOMEM; >> + goto out_free_dev; > > You have error goto's backwards. > >> + } >> + >> + eth_dev->data->mac_addrs = >> + rte_zmalloc("mac_addrs", >> + ETHER_ADDR_LEN * MVNETA_MAC_ADDRS_MAX, 0); >> + if (!eth_dev->data->mac_addrs) { >> + MVNETA_LOG(ERR, "Failed to allocate space for eth addrs"); >> + ret = -ENOMEM; >> + goto out_free_priv; >> + } >> + >> + memset(&req, 0, sizeof(req)); >> + strcpy(req.ifr_name, name); > > > >> +out_free_mac: >> + rte_free(eth_dev->data->mac_addrs); >> +out_free_dev: >> + rte_eth_dev_release_port(eth_dev); >> +out_free_priv: >> + rte_free(priv); > > These are backwards: out_free_priv is called if ioctl fails and will > leak eth_dev port. Good catch - 'out_free_priv' should go before 'out_free_dev'! Thank you. The problem is not for the case of ioctl failure but I guess that is just a wording lapsus. Best regards Andrzej