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 at solarflare.com>
> Signed-off-by: Andrew Rybchenko <arybchenko at 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]);
> +
<...>
