From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH 32/56] net/sfc: implement driver operation to
 init device on attach
Date: Wed, 23 Nov 2016 15:26:47 +0000
Message-ID: <76a60072-2c7b-b674-ca7f-1dc9ca66815a@intel.com>
References: <1479740470-6723-1-git-send-email-arybchenko@solarflare.com>
 <1479740470-6723-33-git-send-email-arybchenko@solarflare.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
To: Andrew Rybchenko <arybchenko@solarflare.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id 977B65686
 for <dev@dpdk.org>; Wed, 23 Nov 2016 16:26:49 +0100 (CET)
In-Reply-To: <1479740470-6723-33-git-send-email-arybchenko@solarflare.com>
List-Id: patches and discussions about DPDK <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 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> The setup and configuration of the PMD is not performance sensitive,
> but is not thread safe either. It is possible that the multiple
> read/writes during PMD setup and configuration could be corrupted
> in a multi-thread environment.  

Right, this is not thread-safe, but this is valid for all PMDs, it is
not expected to have initialization in multi-threaded environment, that
said so, synchronization also won't hurt, as you said this is not fast
path, just may not be necessary.

> Since this is not performance
> sensitive, the developer can choose to add their own layer to provide
> thread-safe setup and configuration. It is expected that, in most
> applications, the initial configuration of the network ports would be
> done by a single thread at startup.
> 
> Reviewed-by: Andy Moreton <amoreton@solarflare.com>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>

> diff --git a/drivers/net/sfc/efx/sfc_ethdev.c b/drivers/net/sfc/efx/sfc_ethdev.c
> index 0deff07..e5b609c 100644
> --- a/drivers/net/sfc/efx/sfc_ethdev.c
> +++ b/drivers/net/sfc/efx/sfc_ethdev.c
> @@ -31,6 +31,8 @@
>  #include <rte_ethdev.h>
>  #include <rte_pci.h>
>  
> +#include "efx.h"
> +
>  #include "sfc.h"
>  #include "sfc_debug.h"
>  #include "sfc_log.h"
> @@ -55,6 +57,8 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
>  	struct sfc_adapter *sa = dev->data->dev_private;
>  	struct rte_pci_device *pci_dev = dev->pci_dev;
>  	int rc;
> +	const efx_nic_cfg_t *encp;
> +	const struct ether_addr *from;
>  
>  	/* Required for logging */
>  	sa->eth_dev = dev;
> @@ -73,11 +77,43 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
>  
>  	sfc_log_init(sa, "entry");
>  
> +	dev->data->mac_addrs = rte_zmalloc("sfc", ETHER_ADDR_LEN, 0);
> +	if (dev->data->mac_addrs == NULL) {
> +		rc = ENOMEM;
> +		goto fail_mac_addrs;
> +	}
> +
> +	sfc_adapter_lock_init(sa);
> +	sfc_adapter_lock(sa);
> +
> +	sfc_log_init(sa, "attaching");
> +	rc = sfc_attach(sa);
> +	if (rc != 0)
> +		goto fail_attach;
> +
> +	encp = efx_nic_cfg_get(sa->nic);
> +
> +	/*
> +	 * The arguments are really reverse order in comparison to
> +	 * Linux kernel. Copy from NIC config to Ethernet device data.
> +	 */

Yes it is J, and agreed this is confusing.

> +	from = (const struct ether_addr *)(encp->enc_mac_addr);
> +	ether_addr_copy(from, &dev->data->mac_addrs[0]);
> +

<...>