From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shreyansh Jain <shreyansh.jain@nxp.com>
Subject: Re: [PATCH v4 25/41] net/dpaa: add support for Tx and Rx
	queue setup
Date: Thu, 28 Sep 2017 17:21:23 +0530
Message-ID: <a8db4f01-08ed-a23c-ea59-250516b51fc6@nxp.com>
References: <20170823141213.25476-1-shreyansh.jain@nxp.com>
 <20170909112132.13936-1-shreyansh.jain@nxp.com>
 <20170909112132.13936-26-shreyansh.jain@nxp.com>
 <5b28593b-5d6c-f77a-80a2-9e8332820b8e@intel.com>
 <0c4268d8-04e0-4e68-d7e9-8b639e3154d2@nxp.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 8bit
Cc: <dev@dpdk.org>, <hemant.agrawal@nxp.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from NAM01-SN1-obe.outbound.protection.outlook.com
 (mail-sn1nam01on0081.outbound.protection.outlook.com [104.47.32.81])
 by dpdk.org (Postfix) with ESMTP id 1447D3195
 for <dev@dpdk.org>; Thu, 28 Sep 2017 13:40:33 +0200 (CEST)
In-Reply-To: <0c4268d8-04e0-4e68-d7e9-8b639e3154d2@nxp.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thursday 21 September 2017 06:29 PM, Shreyansh Jain wrote:
> Hello Ferruh,
> 
> Apologies for delay in response for these, I am already working to get 
> the next version based on your comments. Meanwhile, some comments inline...
> 
> On Monday 18 September 2017 08:25 PM, Ferruh Yigit wrote:
>> On 9/9/2017 12:21 PM, Shreyansh Jain wrote:
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>
>> <...>

[...]

>>
>>> +    }
>>> +
>>> +    /* Populate ethdev structure */
>>>       eth_dev->dev_ops = &dpaa_devops;
>>> +    eth_dev->rx_pkt_burst = dpaa_eth_queue_rx;
>>> +    eth_dev->tx_pkt_burst = dpaa_eth_tx_drop_all;
>>> +
>>> +    /* Allocate memory for storing MAC addresses */
>>> +    eth_dev->data->mac_addrs = rte_zmalloc("mac_addr",
>>> +        ETHER_ADDR_LEN * DPAA_MAX_MAC_FILTER, 0);
>>> +    if (eth_dev->data->mac_addrs == NULL) {
>>> +        DPAA_PMD_ERR("Failed to allocate %d bytes needed to "
>>> +                        "store MAC addresses",
>>> +                ETHER_ADDR_LEN * DPAA_MAX_MAC_FILTER);
>>
>> free dpaa_intf->rx_queues, tx_queues ?
> 
> yes, certainly an issue. I will fix it.

I have fixed this in v5.

> 
>>
>>> +        return -ENOMEM;
>>> +    }
>>> +
>>> +    /* copy the primary mac address */
>>> +    memcpy(eth_dev->data->mac_addrs[0].addr_bytes,
>>> +        fman_intf->mac_addr.addr_bytes,
>>> +        ETHER_ADDR_LEN);
>>
>> Instead can use ether_addr_copy() instead.
> 
> :) Yes, I can.

Unfortunately, I forgot to fix this in v5.
If you want, I can send a small patch against this. Sending a v6 because 
of this would be overkill. But, this is definitely a valid comment.
Sorry.

> 
>>
>> <...>
>>
> 
>