On 7/12/2024 4:47 PM, Serhii Iliushyk wrote:
> Adds support for eth_dev configure, start, stop, close, and infos_get.
> The internal structs of ntnic is also added and initialized.
>
> Signed-off-by: Serhii Iliushyk <sil-plv@napatech.com>
> ---
> v6
> * Replace if_index with n_intf_no
> * Unnecessry resources free was fixed
> * Fix typo
> * Useless vars were removed
<...>
> +
> +static int
> +eth_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *dev_info)
> +{
> + struct pmd_internals *internals = (struct pmd_internals *)eth_dev->data->dev_private;
> +
> + dev_info->if_index = internals->n_intf_no;
>
I commented on this before, but 'if_index' is not a valid field for
physical devices, so it is wrong to set it.
What is the intention to set this value?
<...>
> +static int
> +eth_dev_stop(struct rte_eth_dev *eth_dev)
> +{
> + struct pmd_internals *internals = (struct pmd_internals *)eth_dev->data->dev_private;
> +
> + NT_LOG_DBGX(DEBUG, NTNIC, "Port %u, %u\n",
> + internals->n_intf_no, internals->n_intf_no);
>
Why same value, 'n_intf_no', logged twice?
Btw, log says "Port", and "struct pmd_internals" has 'port_id' field but
it prints 'n_intf_no', is this intentionally?
"struct pmd_internals" has "int n_intf_no", "uint32_t port", "uint32_t
port_id", are these redundant fields?
<...>
> +
> +static struct eth_dev_ops nthw_eth_dev_ops = {
> + .dev_configure = eth_dev_configure,
> + .dev_start = eth_dev_start,
> + .dev_stop = eth_dev_stop,
> + .dev_close = eth_dev_close,
> + .dev_infos_get = eth_dev_infos_get,
> +};
>
This struct can be 'const'.
<...>
> @@ -58,6 +252,31 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
>
> snprintf(name, sizeof(name), "ntnic%d", n_intf_no);
>
> + internals = rte_zmalloc_socket(name, sizeof(struct pmd_internals),
> + RTE_CACHE_LINE_SIZE, pci_dev->device.numa_node);
> +
> + if (!internals) {
> + NT_LOG_DBGX(ERR, NTNIC, "%s: %s: error=%d\n",
> + (pci_dev->name[0] ? pci_dev->name : "NA"), name, -1);
> + return -1;
> + }
> +
> + internals->pci_dev = pci_dev;
> + internals->n_intf_no = n_intf_no;
> +
> + /* Setup queue_ids */
> + if (nb_rx_queues > 1) {
> + NT_LOG(DBG, NTNIC,
> + "(%i) NTNIC configured with Rx multi queues. %i queues\n",
> + 0 /*port*/, nb_rx_queues);
>
What is hardcoded '0' for "(%i) NTNIC ..."
And normally number of Rx/Tx queues set by user via
'rte_eth_dev_configure()' API, this initialization has queue numbers
hardcoded as '1'. I assume this is for this initial version wher
configure support is missing, but just reminding here in any case.
> + }
> +
> + if (nb_tx_queues > 1) {
> + NT_LOG(DBG, NTNIC,
> + "(%i) NTNIC configured with Tx multi queues. %i queues\n",
> + 0 /*port*/, nb_tx_queues);
> + }
> +
> eth_dev = rte_eth_dev_allocate(name);
>
> if (!eth_dev) {
> @@ -66,9 +285,14 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
> return -1;
> }
>
> - NT_LOG_DBGX(DEBUG, NTNIC, "eth_dev %p, port_id %u, if_index %u\n",
> + NT_LOG_DBGX(DEBUG, NTNIC, "eth_dev %p, port_id %u, n_intf_no %u\n",
>
Is above change intentional? Why not add the log correct at first place
instead of updating it here?
new file mode 100644
@@ -0,0 +1,17 @@
+/*
+ * SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Napatech A/S
+ */
+
+#ifndef __NTDRV_4GA_H__
+#define __NTDRV_4GA_H__
+
+
+typedef struct ntdrv_4ga_s {
+ uint32_t pciident;
+ char *p_drv_name;
+
+ volatile bool b_shutdown;
+} ntdrv_4ga_t;
+
+#endif /* __NTDRV_4GA_H__ */
new file mode 100644
@@ -0,0 +1,32 @@
+/*
+ * SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Napatech A/S
+ */
+
+#ifndef __NTOS_DRV_H__
+#define __NTOS_DRV_H__
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <inttypes.h>
+
+#include <rte_ether.h>
+
+#define NUM_MAC_ADDRS_PER_PORT (16U)
+#define NUM_MULTICAST_ADDRS_PER_PORT (16U)
+
+#define NUM_ADAPTER_MAX (8)
+#define NUM_ADAPTER_PORTS_MAX (128)
+
+struct pmd_internals {
+ const struct rte_pci_device *pci_dev;
+ char name[20];
+ int n_intf_no;
+ uint32_t port;
+ uint32_t port_id;
+ struct drv_s *p_drv;
+ struct pmd_internals *next;
+};
+
+#endif /* __NTOS_DRV_H__ */
new file mode 100644
@@ -0,0 +1,19 @@
+/*
+ * SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Napatech A/S
+ */
+
+#ifndef __NTOS_SYSTEM_H__
+#define __NTOS_SYSTEM_H__
+
+#include "ntdrv_4ga.h"
+
+struct drv_s {
+ int adapter_no;
+ struct rte_pci_device *p_dev;
+ struct ntdrv_4ga_s ntdrv;
+
+ int n_eth_dev_init_count;
+};
+
+#endif /* __NTOS_SYSTEM_H__ */
@@ -10,6 +10,7 @@ endif
# includes
includes = [
include_directories('.'),
+ include_directories('include'),
include_directories('ntlog'),
include_directories('ntutil'),
]
@@ -15,6 +15,9 @@
#include "ntlog.h"
+#include "ntdrv_4ga.h"
+#include "ntos_drv.h"
+#include "ntos_system.h"
#include "ntnic_vfio.h"
#include "nt_util.h"
@@ -26,30 +29,221 @@ static const struct rte_pci_id nthw_pci_id_map[] = {
}, /* sentinel */
};
+static rte_spinlock_t hwlock = RTE_SPINLOCK_INITIALIZER;
+
+/*
+ * Store and get adapter info
+ */
+
+static struct drv_s *_g_p_drv[NUM_ADAPTER_MAX] = { NULL };
+
+static void
+store_pdrv(struct drv_s *p_drv)
+{
+ if (p_drv->adapter_no > NUM_ADAPTER_MAX) {
+ NT_LOG(ERR, NTNIC,
+ "Internal error adapter number %u out of range. Max number of adapters: %u\n",
+ p_drv->adapter_no, NUM_ADAPTER_MAX);
+ return;
+ }
+
+ if (_g_p_drv[p_drv->adapter_no] != 0) {
+ NT_LOG(WRN, NTNIC,
+ "Overwriting adapter structure for PCI " PCIIDENT_PRINT_STR
+ " with adapter structure for PCI " PCIIDENT_PRINT_STR "\n",
+ PCIIDENT_TO_DOMAIN(_g_p_drv[p_drv->adapter_no]->ntdrv.pciident),
+ PCIIDENT_TO_BUSNR(_g_p_drv[p_drv->adapter_no]->ntdrv.pciident),
+ PCIIDENT_TO_DEVNR(_g_p_drv[p_drv->adapter_no]->ntdrv.pciident),
+ PCIIDENT_TO_FUNCNR(_g_p_drv[p_drv->adapter_no]->ntdrv.pciident),
+ PCIIDENT_TO_DOMAIN(p_drv->ntdrv.pciident),
+ PCIIDENT_TO_BUSNR(p_drv->ntdrv.pciident),
+ PCIIDENT_TO_DEVNR(p_drv->ntdrv.pciident),
+ PCIIDENT_TO_FUNCNR(p_drv->ntdrv.pciident));
+ }
+
+ rte_spinlock_lock(&hwlock);
+ _g_p_drv[p_drv->adapter_no] = p_drv;
+ rte_spinlock_unlock(&hwlock);
+}
+
+static void
+clear_pdrv(struct drv_s *p_drv)
+{
+ if (p_drv->adapter_no > NUM_ADAPTER_MAX)
+ return;
+
+ rte_spinlock_lock(&hwlock);
+ _g_p_drv[p_drv->adapter_no] = NULL;
+ rte_spinlock_unlock(&hwlock);
+}
+
+static struct drv_s *
+get_pdrv_from_pci(struct rte_pci_addr addr)
+{
+ int i;
+ struct drv_s *p_drv = NULL;
+ rte_spinlock_lock(&hwlock);
+
+ for (i = 0; i < NUM_ADAPTER_MAX; i++) {
+ if (_g_p_drv[i]) {
+ if (PCIIDENT_TO_DOMAIN(_g_p_drv[i]->ntdrv.pciident) == addr.domain &&
+ PCIIDENT_TO_BUSNR(_g_p_drv[i]->ntdrv.pciident) == addr.bus) {
+ p_drv = _g_p_drv[i];
+ break;
+ }
+ }
+ }
+
+ rte_spinlock_unlock(&hwlock);
+ return p_drv;
+}
+
+static int
+eth_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *dev_info)
+{
+ struct pmd_internals *internals = (struct pmd_internals *)eth_dev->data->dev_private;
+
+ dev_info->if_index = internals->n_intf_no;
+ dev_info->driver_name = internals->name;
+
+ return 0;
+}
+
+static int
+eth_dev_configure(struct rte_eth_dev *eth_dev)
+{
+ NT_LOG_DBGX(DEBUG, NTNIC, "Called for eth_dev %p\n", eth_dev);
+
+ /* The device is ALWAYS running promiscuous mode. */
+ eth_dev->data->promiscuous ^= ~eth_dev->data->promiscuous;
+ return 0;
+}
+
+static int
+eth_dev_start(struct rte_eth_dev *eth_dev)
+{
+ struct pmd_internals *internals = (struct pmd_internals *)eth_dev->data->dev_private;
+
+ NT_LOG_DBGX(DEBUG, NTNIC, "Port %u\n", internals->n_intf_no);
+
+ return 0;
+}
+
+static int
+eth_dev_stop(struct rte_eth_dev *eth_dev)
+{
+ struct pmd_internals *internals = (struct pmd_internals *)eth_dev->data->dev_private;
+
+ NT_LOG_DBGX(DEBUG, NTNIC, "Port %u, %u\n",
+ internals->n_intf_no, internals->n_intf_no);
+
+ eth_dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN;
+ return 0;
+}
+
+static void
+drv_deinit(struct drv_s *p_drv)
+{
+ if (p_drv == NULL)
+ return;
+ /*
+ * Mark the global pdrv for cleared. Used by some threads to terminate.
+ * 1 second to give the threads a chance to see the termination.
+ */
+ clear_pdrv(p_drv);
+ nt_os_wait_usec(1000000);
+
+ /* clean memory */
+ rte_free(p_drv);
+ p_drv = NULL;
+}
+
+static int
+eth_dev_close(struct rte_eth_dev *eth_dev)
+{
+ struct pmd_internals *internals = (struct pmd_internals *)eth_dev->data->dev_private;
+ struct drv_s *p_drv = internals->p_drv;
+
+ internals->p_drv = NULL;
+
+ rte_eth_dev_release_port(eth_dev);
+
+ /* decrease initialized ethernet devices */
+ p_drv->n_eth_dev_init_count--;
+
+ /*
+ * rte_pci_dev has no private member for p_drv
+ * wait until all rte_eth_dev's are closed - then close adapters via p_drv
+ */
+ if (!p_drv->n_eth_dev_init_count && p_drv)
+ drv_deinit(p_drv);
+
+ return 0;
+}
+
+static struct eth_dev_ops nthw_eth_dev_ops = {
+ .dev_configure = eth_dev_configure,
+ .dev_start = eth_dev_start,
+ .dev_stop = eth_dev_stop,
+ .dev_close = eth_dev_close,
+ .dev_infos_get = eth_dev_infos_get,
+};
+
static int
nthw_pci_dev_init(struct rte_pci_device *pci_dev)
{
nt_vfio_init();
+ struct drv_s *p_drv;
+ ntdrv_4ga_t *p_nt_drv;
uint32_t n_port_mask = -1; /* All ports enabled by default */
+ uint32_t nb_rx_queues = 1;
+ uint32_t nb_tx_queues = 1;
int n_phy_ports;
NT_LOG_DBGX(DEBUG, NTNIC, "Dev %s PF #%i Init : %02x:%02x:%i\n", pci_dev->name,
pci_dev->addr.function, pci_dev->addr.bus, pci_dev->addr.devid,
pci_dev->addr.function);
+ /* alloc */
+ p_drv = rte_zmalloc_socket(pci_dev->name, sizeof(struct drv_s), RTE_CACHE_LINE_SIZE,
+ pci_dev->device.numa_node);
+
+ if (!p_drv) {
+ NT_LOG_DBGX(ERR, NTNIC, "%s: error %d\n",
+ (pci_dev->name[0] ? pci_dev->name : "NA"), -1);
+ return -1;
+ }
+
/* Setup VFIO context */
int vfio = nt_vfio_setup(pci_dev);
if (vfio < 0) {
NT_LOG_DBGX(ERR, TNIC, "%s: vfio_setup error %d\n",
(pci_dev->name[0] ? pci_dev->name : "NA"), -1);
+ rte_free(p_drv);
return -1;
}
+ /* context */
+ p_nt_drv = &p_drv->ntdrv;
+
+ p_drv->p_dev = pci_dev;
+
+ /* Set context for NtDrv */
+ p_nt_drv->pciident = BDF_TO_PCIIDENT(pci_dev->addr.domain, pci_dev->addr.bus,
+ pci_dev->addr.devid, pci_dev->addr.function);
+
+
+ p_nt_drv->b_shutdown = false;
+
+ /* store context */
+ store_pdrv(p_drv);
+
n_phy_ports = 0;
for (int n_intf_no = 0; n_intf_no < n_phy_ports; n_intf_no++) {
+ struct pmd_internals *internals = NULL;
struct rte_eth_dev *eth_dev = NULL;
char name[32];
@@ -58,6 +252,31 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
snprintf(name, sizeof(name), "ntnic%d", n_intf_no);
+ internals = rte_zmalloc_socket(name, sizeof(struct pmd_internals),
+ RTE_CACHE_LINE_SIZE, pci_dev->device.numa_node);
+
+ if (!internals) {
+ NT_LOG_DBGX(ERR, NTNIC, "%s: %s: error=%d\n",
+ (pci_dev->name[0] ? pci_dev->name : "NA"), name, -1);
+ return -1;
+ }
+
+ internals->pci_dev = pci_dev;
+ internals->n_intf_no = n_intf_no;
+
+ /* Setup queue_ids */
+ if (nb_rx_queues > 1) {
+ NT_LOG(DBG, NTNIC,
+ "(%i) NTNIC configured with Rx multi queues. %i queues\n",
+ 0 /*port*/, nb_rx_queues);
+ }
+
+ if (nb_tx_queues > 1) {
+ NT_LOG(DBG, NTNIC,
+ "(%i) NTNIC configured with Tx multi queues. %i queues\n",
+ 0 /*port*/, nb_tx_queues);
+ }
+
eth_dev = rte_eth_dev_allocate(name);
if (!eth_dev) {
@@ -66,9 +285,14 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
return -1;
}
- NT_LOG_DBGX(DEBUG, NTNIC, "eth_dev %p, port_id %u, if_index %u\n",
+ NT_LOG_DBGX(DEBUG, NTNIC, "eth_dev %p, port_id %u, n_intf_no %u\n",
eth_dev, eth_dev->data->port_id, n_intf_no);
+ /* connect structs */
+ internals->p_drv = p_drv;
+ eth_dev->data->dev_private = internals;
+
+ internals->port_id = eth_dev->data->port_id;
struct rte_eth_link pmd_link;
pmd_link.link_speed = RTE_ETH_SPEED_NUM_NONE;
@@ -78,7 +302,8 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
eth_dev->device = &pci_dev->device;
eth_dev->data->dev_link = pmd_link;
- eth_dev->dev_ops = NULL;
+ eth_dev->dev_ops = &nthw_eth_dev_ops;
+
eth_dev_pci_specific_init(eth_dev, pci_dev);
rte_eth_dev_probing_finish(eth_dev);
@@ -162,6 +387,9 @@ nthw_pci_remove(struct rte_pci_device *pci_dev)
{
NT_LOG_DBGX(DEBUG, NTNIC);
+ struct drv_s *p_drv = get_pdrv_from_pci(pci_dev->addr);
+ drv_deinit(p_drv);
+
return rte_eth_dev_pci_generic_remove(pci_dev, nthw_pci_dev_deinit);
}