[dpdk-dev] [PATCH 0/4] support secondary process for failsafe

[dpdk-dev] [PATCH 0/4] support secondary process for failsafe

[Raslan Darawsheh]

this set of patches are intended to support secondary process for failsafe.

Raslan Darawsheh (4):
  net/failsafe: replace local device with shared data
  net/failsafe: change back-reference from sub-device
  net/failsafe: replace local sub-device with shared data
  net/failsafe: support secondary process

 drivers/net/failsafe/failsafe.c         | 45 +++++++++++++++++++++++++++++++--
 drivers/net/failsafe/failsafe_eal.c     |  4 +--
 drivers/net/failsafe/failsafe_ether.c   | 22 ++++++++--------
 drivers/net/failsafe/failsafe_intr.c    | 20 +++++++--------
 drivers/net/failsafe/failsafe_ops.c     |  4 +--
 drivers/net/failsafe/failsafe_private.h | 30 +++++++++++++++-------
 drivers/net/failsafe/failsafe_rxtx.c    |  4 +--
 7 files changed, 92 insertions(+), 37 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/4] net/failsafe: replace local device with shared data

[Raslan Darawsheh]

In multiprocess context, the private structure is shared between
processes. The back reference from private to generic data was using
a pointer to a per process eth_dev. It's now changed to a reference of
the shared data.

Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/failsafe/failsafe.c         |  2 +-
 drivers/net/failsafe/failsafe_intr.c    | 10 +++++-----
 drivers/net/failsafe/failsafe_ops.c     |  4 ++--
 drivers/net/failsafe/failsafe_private.h |  8 ++++++--
 drivers/net/failsafe/failsafe_rxtx.c    |  4 ++--
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 06e859e..68926ca 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -181,7 +181,7 @@ fs_eth_dev_create(struct rte_vdev_device *vdev)
 		return -1;
 	}
 	priv = PRIV(dev);
-	priv->dev = dev;
+	priv->data = dev->data;
 	dev->dev_ops = &failsafe_ops;
 	dev->data->mac_addrs = &PRIV(dev)->mac_addrs[0];
 	dev->data->dev_link = eth_link;
diff --git a/drivers/net/failsafe/failsafe_intr.c b/drivers/net/failsafe/failsafe_intr.c
index 1c2cb71..09aa3c6 100644
--- a/drivers/net/failsafe/failsafe_intr.c
+++ b/drivers/net/failsafe/failsafe_intr.c
@@ -133,8 +133,8 @@ fs_rx_event_proxy_service_install(struct fs_priv *priv)
 	/* prepare service info */
 	memset(&service, 0, sizeof(struct rte_service_spec));
 	snprintf(service.name, sizeof(service.name), "%s_Rx_service",
-		 priv->dev->data->name);
-	service.socket_id = priv->dev->data->numa_node;
+		 priv->data->name);
+	service.socket_id = priv->data->numa_node;
 	service.callback = fs_rx_event_proxy_routine;
 	service.callback_userdata = priv;
 
@@ -437,7 +437,7 @@ fs_rx_intr_vec_install(struct fs_priv *priv)
 	unsigned int count;
 	struct rte_intr_handle *intr_handle;
 
-	rxqs_n = priv->dev->data->nb_rx_queues;
+	rxqs_n = priv->data->nb_rx_queues;
 	n = RTE_MIN(rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
 	count = 0;
 	intr_handle = &priv->intr_handle;
@@ -452,7 +452,7 @@ fs_rx_intr_vec_install(struct fs_priv *priv)
 		return -rte_errno;
 	}
 	for (i = 0; i < n; i++) {
-		struct rxq *rxq = priv->dev->data->rx_queues[i];
+		struct rxq *rxq = priv->data->rx_queues[i];
 
 		/* Skip queues that cannot request interrupts. */
 		if (rxq == NULL || rxq->event_fd < 0) {
@@ -521,7 +521,7 @@ failsafe_rx_intr_install(struct rte_eth_dev *dev)
 {
 	struct fs_priv *priv = PRIV(dev);
 	const struct rte_intr_conf *const intr_conf =
-			&priv->dev->data->dev_conf.intr_conf;
+			&priv->data->dev_conf.intr_conf;
 
 	if (intr_conf->rxq == 0 || dev->intr_handle != NULL)
 		return 0;
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index e3add40..65957a2 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -452,7 +452,7 @@ fs_rx_queue_release(void *queue)
 	if (queue == NULL)
 		return;
 	rxq = queue;
-	dev = rxq->priv->dev;
+	dev = &rte_eth_devices[rxq->priv->data->port_id];
 	fs_lock(dev, 0);
 	if (rxq->event_fd > 0)
 		close(rxq->event_fd);
@@ -636,7 +636,7 @@ fs_tx_queue_release(void *queue)
 	if (queue == NULL)
 		return;
 	txq = queue;
-	dev = txq->priv->dev;
+	dev = &rte_eth_devices[txq->priv->data->port_id];
 	fs_lock(dev, 0);
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
 		if (ETH(sdev)->data->tx_queues != NULL &&
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index 0dfea65..91b7167 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -128,14 +128,18 @@ struct sub_device {
 	unsigned int lsc_callback:1;
 };
 
+/*
+ * This is referenced by eth_dev->data->dev_private
+ * This is shared between processes.
+ */
 struct fs_priv {
-	struct rte_eth_dev *dev;
+	struct rte_eth_dev_data *data; /* backreference to shared data. */
 	/*
 	 * Set of sub_devices.
 	 * subs[0] is the preferred device
 	 * any other is just another slave
 	 */
-	struct sub_device *subs;
+	struct sub_device *subs; /* shared between processes */
 	uint8_t subs_head; /* if head == tail, no subs */
 	uint8_t subs_tail; /* first invalid */
 	uint8_t subs_tx; /* current emitting device */
diff --git a/drivers/net/failsafe/failsafe_rxtx.c b/drivers/net/failsafe/failsafe_rxtx.c
index 034f47b..231c832 100644
--- a/drivers/net/failsafe/failsafe_rxtx.c
+++ b/drivers/net/failsafe/failsafe_rxtx.c
@@ -126,7 +126,7 @@ failsafe_tx_burst(void *queue,
 	uint16_t nb_tx;
 
 	txq = queue;
-	sdev = TX_SUBDEV(txq->priv->dev);
+	sdev = TX_SUBDEV(&rte_eth_devices[txq->priv->data->port_id]);
 	if (unlikely(fs_tx_unsafe(sdev)))
 		return 0;
 	sub_txq = ETH(sdev)->data->tx_queues[txq->qid];
@@ -147,7 +147,7 @@ failsafe_tx_burst_fast(void *queue,
 	uint16_t nb_tx;
 
 	txq = queue;
-	sdev = TX_SUBDEV(txq->priv->dev);
+	sdev = TX_SUBDEV(&rte_eth_devices[txq->priv->data->port_id]);
 	RTE_ASSERT(!fs_tx_unsafe(sdev));
 	sub_txq = ETH(sdev)->data->tx_queues[txq->qid];
 	FS_ATOMIC_P(txq->refcnt[sdev->sid]);
-- 
2.7.4

[dpdk-dev] [PATCH 2/4] net/failsafe: change back-reference from sub-device

[Raslan Darawsheh]

In multiprocess context, the sub-device structure is shared
between processes. The reference to the failsafe device was
a per process pointer. It's changed to port id which is the
same for all processes.

Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/failsafe/failsafe_eal.c     |  2 +-
 drivers/net/failsafe/failsafe_ether.c   | 15 ++++++++-------
 drivers/net/failsafe/failsafe_intr.c    | 10 +++++-----
 drivers/net/failsafe/failsafe_private.h | 11 ++++++++---
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index 8a888b1..56d1669 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -114,7 +114,7 @@ fs_bus_init(struct rte_eth_dev *dev)
 		}
 		ETH(sdev) = &rte_eth_devices[pid];
 		SUB_ID(sdev) = i;
-		sdev->fs_dev = dev;
+		sdev->fs_port_id = dev->data->port_id;
 		sdev->dev = ETH(sdev)->device;
 		sdev->state = DEV_PROBED;
 	}
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 1783165..8d958e0 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -298,7 +298,7 @@ fs_dev_remove(struct sub_device *sdev)
 		break;
 	}
 	sdev->remove = 0;
-	failsafe_hotplug_alarm_install(sdev->fs_dev);
+	failsafe_hotplug_alarm_install(FSDEV_FROM_SUBDEV(sdev));
 }
 
 static void
@@ -318,8 +318,9 @@ fs_dev_stats_save(struct sub_device *sdev)
 			WARN("Using latest snapshot taken before %"PRIu64" seconds.\n",
 				 (rte_rdtsc() - timestamp) / rte_get_tsc_hz());
 	}
-	failsafe_stats_increment(&PRIV(sdev->fs_dev)->stats_accumulator,
-			err ? &sdev->stats_snapshot.stats : &stats);
+	failsafe_stats_increment
+		(&PRIV(FSDEV_FROM_SUBDEV(sdev))->stats_accumulator,
+		err ? &sdev->stats_snapshot.stats : &stats);
 	memset(&sdev->stats_snapshot, 0, sizeof(sdev->stats_snapshot));
 }
 
@@ -566,17 +567,17 @@ failsafe_eth_rmv_event_callback(uint16_t port_id __rte_unused,
 {
 	struct sub_device *sdev = cb_arg;
 
-	fs_lock(sdev->fs_dev, 0);
+	fs_lock(FSDEV_FROM_SUBDEV(sdev), 0);
 	/* Switch as soon as possible tx_dev. */
-	fs_switch_dev(sdev->fs_dev, sdev);
+	fs_switch_dev(FSDEV_FROM_SUBDEV(sdev), sdev);
 	/* Use safe bursts in any case. */
-	failsafe_set_burst_fn(sdev->fs_dev, 1);
+	failsafe_set_burst_fn(FSDEV_FROM_SUBDEV(sdev), 1);
 	/*
 	 * Async removal, the sub-PMD will try to unregister
 	 * the callback at the source of the current thread context.
 	 */
 	sdev->remove = 1;
-	fs_unlock(sdev->fs_dev, 0);
+	fs_unlock(FSDEV_FROM_SUBDEV(sdev), 0);
 	return 0;
 }
 
diff --git a/drivers/net/failsafe/failsafe_intr.c b/drivers/net/failsafe/failsafe_intr.c
index 09aa3c6..6fe5816 100644
--- a/drivers/net/failsafe/failsafe_intr.c
+++ b/drivers/net/failsafe/failsafe_intr.c
@@ -274,14 +274,14 @@ failsafe_eth_rx_intr_ctl_subdevice(struct sub_device *sdev, int op)
 	int rc;
 	int ret = 0;
 
+	fsdev = FSDEV_FROM_SUBDEV(sdev);
 	if (sdev == NULL || (ETH(sdev) == NULL) ||
-	    sdev->fs_dev == NULL || (PRIV(sdev->fs_dev) == NULL)) {
+		fsdev == NULL || (PRIV(fsdev) == NULL)) {
 		ERROR("Called with invalid arguments");
 		return -EINVAL;
 	}
 	dev = ETH(sdev);
-	fsdev = sdev->fs_dev;
-	epfd = PRIV(sdev->fs_dev)->rxp.efd;
+	epfd = PRIV(fsdev)->rxp.efd;
 	pid = PORT_ID(sdev);
 
 	if (epfd <= 0) {
@@ -330,7 +330,7 @@ int failsafe_rx_intr_install_subdevice(struct sub_device *sdev)
 	const struct rte_intr_conf *const intr_conf =
 				&ETH(sdev)->data->dev_conf.intr_conf;
 
-	fsdev = sdev->fs_dev;
+	fsdev = FSDEV_FROM_SUBDEV(sdev);
 	rxq = (struct rxq **)fsdev->data->rx_queues;
 	if (intr_conf->rxq == 0)
 		return 0;
@@ -368,7 +368,7 @@ void failsafe_rx_intr_uninstall_subdevice(struct sub_device *sdev)
 	struct rte_eth_dev *fsdev;
 	struct rxq *fsrxq;
 
-	fsdev = sdev->fs_dev;
+	fsdev = FSDEV_FROM_SUBDEV(sdev);
 	for (qid = 0; qid < ETH(sdev)->data->nb_rx_queues; qid++) {
 		if (qid < fsdev->data->nb_rx_queues) {
 			fsrxq = fsdev->data->rx_queues[qid];
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index 91b7167..9b53a19 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -117,7 +117,7 @@ struct sub_device {
 	/* Others are retrieved through a file descriptor */
 	char *fd_str;
 	/* fail-safe device backreference */
-	struct rte_eth_dev *fs_dev;
+	uint16_t fs_port_id; /* shared between processes*/
 	/* flag calling for recollection */
 	volatile unsigned int remove:1;
 	/* flow isolation state */
@@ -251,6 +251,9 @@ extern int failsafe_mac_from_arg;
 /* dev: (struct rte_eth_dev *) fail-safe device */
 #define PRIV(dev) \
 	((struct fs_priv *)(dev)->data->dev_private)
+/* sdev: (struct sub_device *) */
+#define FSDEV_FROM_SUBDEV(sdev) \
+	(&rte_eth_devices[sdev->fs_port_id])
 
 /* sdev: (struct sub_device *) */
 #define ETH(sdev) \
@@ -324,7 +327,8 @@ extern int failsafe_mac_from_arg;
  */
 #define FS_ATOMIC_RX(s, i) \
 	rte_atomic64_read( \
-	 &((struct rxq *)((s)->fs_dev->data->rx_queues[i]))->refcnt[(s)->sid] \
+	 &((struct rxq *) \
+	 (FSDEV_FROM_SUBDEV(s)->data->rx_queues[i]))->refcnt[(s)->sid] \
 	)
 /**
  * s: (struct sub_device *)
@@ -332,7 +336,8 @@ extern int failsafe_mac_from_arg;
  */
 #define FS_ATOMIC_TX(s, i) \
 	rte_atomic64_read( \
-	 &((struct txq *)((s)->fs_dev->data->tx_queues[i]))->refcnt[(s)->sid] \
+	 &((struct txq *) \
+	 (FSDEV_FROM_SUBDEV(s)->data->tx_queues[i]))->refcnt[(s)->sid] \
 	)
 
 #ifdef RTE_EXEC_ENV_BSDAPP
-- 
2.7.4

[dpdk-dev] [PATCH 3/4] net/failsafe: replace local sub-device with shared data

[Raslan Darawsheh]

In multiprocess context, the pointer to sub-device is shared between
processes. Previously, it was a pointer to per process eth_dev so
it's needed to replace this dependency.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
 drivers/net/failsafe/failsafe_eal.c     |  2 +-
 drivers/net/failsafe/failsafe_ether.c   |  7 ++++---
 drivers/net/failsafe/failsafe_private.h | 11 +++++++----
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index 56d1669..6fac4b6 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -112,7 +112,7 @@ fs_bus_init(struct rte_eth_dev *dev)
 				continue;
 			}
 		}
-		ETH(sdev) = &rte_eth_devices[pid];
+		sdev->data = rte_eth_devices[pid].data;
 		SUB_ID(sdev) = i;
 		sdev->fs_port_id = dev->data->port_id;
 		sdev->dev = ETH(sdev)->device;
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 8d958e0..9b9753f 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -267,18 +267,19 @@ static void
 fs_dev_remove(struct sub_device *sdev)
 {
 	int ret;
+	struct rte_eth_dev *edev = ETH(sdev);
 
 	if (sdev == NULL)
 		return;
 	switch (sdev->state) {
 	case DEV_STARTED:
 		failsafe_rx_intr_uninstall_subdevice(sdev);
-		rte_eth_dev_stop(PORT_ID(sdev));
+		rte_eth_dev_stop(edev->data->port_id);
 		sdev->state = DEV_ACTIVE;
 		/* fallthrough */
 	case DEV_ACTIVE:
 		failsafe_eth_dev_unregister_callbacks(sdev);
-		rte_eth_dev_close(PORT_ID(sdev));
+		rte_eth_dev_close(edev->data->port_id);
 		sdev->state = DEV_PROBED;
 		/* fallthrough */
 	case DEV_PROBED:
@@ -287,7 +288,7 @@ fs_dev_remove(struct sub_device *sdev)
 			ERROR("Bus detach failed for sub_device %u",
 			      SUB_ID(sdev));
 		} else {
-			rte_eth_dev_release_port(ETH(sdev));
+			rte_eth_dev_release_port(edev);
 		}
 		sdev->state = DEV_PARSED;
 		/* fallthrough */
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index 9b53a19..60ee0a7 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -100,13 +100,16 @@ struct fs_stats {
 	uint64_t timestamp;
 };
 
+/*
+ * Allocated in shared memory.
+ */
 struct sub_device {
 	/* Exhaustive DPDK device description */
 	struct sub_device *next;
 	struct rte_devargs devargs;
-	struct rte_bus *bus;
-	struct rte_device *dev;
-	struct rte_eth_dev *edev;
+	struct rte_bus *bus; /* per process. */
+	struct rte_device *dev; /* per process. */
+	struct rte_eth_dev_data *data; /* shared between processes */
 	uint8_t sid;
 	/* Device state machine */
 	enum dev_state state;
@@ -257,7 +260,7 @@ extern int failsafe_mac_from_arg;
 
 /* sdev: (struct sub_device *) */
 #define ETH(sdev) \
-	((sdev)->edev)
+	(sdev->data == NULL ? NULL : &rte_eth_devices[sdev->data->port_id])
 
 /* sdev: (struct sub_device *) */
 #define PORT_ID(sdev) \
-- 
2.7.4

[dpdk-dev] [PATCH 4/4] net/failsafe: support secondary process

[Raslan Darawsheh]

Add implementation for probe in secondary.

Failsafe will attempt to attach all the sub-devices in
secondary process.

Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/failsafe/failsafe.c | 43 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 68926ca..fa083fa 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -313,10 +313,27 @@ fs_rte_eth_free(const char *name)
 }
 
 static int
+devargs_already_listed(struct rte_devargs *devargs)
+{
+	struct rte_devargs *list_da;
+
+	RTE_EAL_DEVARGS_FOREACH(devargs->bus->name, list_da) {
+		if (strcmp(list_da->name, devargs->name) == 0)
+			/* devargs already in the list */
+			return 1;
+	}
+	return 0;
+}
+
+static int
 rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
 {
 	const char *name;
 	struct rte_eth_dev *eth_dev;
+	struct sub_device  *sdev;
+	struct rte_devargs devargs;
+	uint8_t i;
+	int ret;
 
 	name = rte_vdev_device_name(vdev);
 	INFO("Initializing " FAILSAFE_DRIVER_NAME " for %s",
@@ -329,9 +346,33 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
 			ERROR("Failed to probe %s", name);
 			return -1;
 		}
-		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &failsafe_ops;
 		eth_dev->device = &vdev->device;
+		eth_dev->rx_pkt_burst = (eth_rx_burst_t)&failsafe_rx_burst;
+		eth_dev->tx_pkt_burst = (eth_tx_burst_t)&failsafe_tx_burst;
+		/*
+		 * Failsafe will attempt to probe all of its sub-devices.
+		 * Any failure in sub-devices is not a fatal error.
+		 * A sub-device can be plugged later.
+		 */
+		FOREACH_SUBDEV(sdev, i, eth_dev) {
+			/* rebuild devargs to be able to get the bus name. */
+			ret = rte_devargs_parse(&devargs,
+						sdev->devargs.name);
+			if (ret != 0) {
+				ERROR("Failed to parse devargs %s",
+					devargs.name);
+				continue;
+			}
+			if (!devargs_already_listed(&devargs)) {
+				ret = rte_dev_probe(devargs.name);
+				if (ret != 0) {
+					ERROR("Failed to probe devargs %s",
+					      devargs.name);
+					continue;
+				}
+			}
+		}
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
-- 
2.7.4

Re: [dpdk-dev] [PATCH 1/4] net/failsafe: replace local device with shared data

[Thomas Monjalon]

28/02/2019 16:49, Raslan Darawsheh:
> In multiprocess context, the private structure is shared between
> processes. The back reference from private to generic data was using
> a pointer to a per process eth_dev. It's now changed to a reference of
> the shared data.
> 
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> -	struct sub_device *subs;
> +	struct sub_device *subs; /* shared between processes */

I think this comment would have more sense if added in patch 3
which is about subdev.
Sorry for nitpicking ;)

Re: [dpdk-dev] [PATCH 2/4] net/failsafe: change back-reference from sub-device

[Stephen Hemminger]

On Thu, 28 Feb 2019 15:49:26 +0000
Raslan Darawsheh <rasland@mellanox.com> wrote:

> +/* sdev: (struct sub_device *) */
> +#define FSDEV_FROM_SUBDEV(sdev) \
> +	(&rte_eth_devices[sdev->fs_port_id])

If at all possible, inline functions are preferable to macros because
inline functions don't allow side effects and keep the type checking.

This could just be an inline function without any impact to code
generation.

Re: [dpdk-dev] [PATCH 3/4] net/failsafe: replace local sub-device with shared data

[Stephen Hemminger]

On Thu, 28 Feb 2019 15:49:27 +0000
Raslan Darawsheh <rasland@mellanox.com> wrote:

>  
>  /* sdev: (struct sub_device *) */
>  #define ETH(sdev) \
> -	((sdev)->edev)
> +	(sdev->data == NULL ? NULL : &rte_eth_devices[sdev->data->port_id])

Macro arguments should always be parenthesised and you can shorten
by using abbreviated trigraph.

#define ETH(sdev) \
	((sdev)->data ? &rte_eth_devices[(sdev)->data->port_id] : )

Re: [dpdk-dev] [PATCH 4/4] net/failsafe: support secondary process

[Stephen Hemminger]

On Thu, 28 Feb 2019 15:49:28 +0000
Raslan Darawsheh <rasland@mellanox.com> wrote:

>  
>  static int
> +devargs_already_listed(struct rte_devargs *devargs)

Why not make this function bool?

static bool
devargs_already_listed(...)

Re: [dpdk-dev] [PATCH 4/4] net/failsafe: support secondary process

[Stephen Hemminger]

On Thu, 28 Feb 2019 15:49:28 +0000
Raslan Darawsheh <rasland@mellanox.com> wrote:

> +		eth_dev->rx_pkt_burst = (eth_rx_burst_t)&failsafe_rx_burst;
> +		eth_dev->tx_pkt_burst = (eth_tx_burst_t)&failsafe_tx_burst;

Why is cast necessary here. The function signature should match.

Re: [dpdk-dev] [PATCH 4/4] net/failsafe: support secondary process

[Raslan Darawsheh]

Hi Stephen, 

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, February 28, 2019 7:25 PM
> To: Raslan Darawsheh <rasland@mellanox.com>
> Cc: gaetan.rivet@6wind.com; dev@dpdk.org; Thomas Monjalon
> <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH 4/4] net/failsafe: support secondary process
> 
> On Thu, 28 Feb 2019 15:49:28 +0000
> Raslan Darawsheh <rasland@mellanox.com> wrote:
> 
> > +		eth_dev->rx_pkt_burst =
> (eth_rx_burst_t)&failsafe_rx_burst;
> > +		eth_dev->tx_pkt_burst =
> (eth_tx_burst_t)&failsafe_tx_burst;
> 
> Why is cast necessary here. The function signature should match.

I don't think it's necessary, but this is the same as the primary process implementation

Kindest regards
Raslan Darawsheh

Re: [dpdk-dev] [PATCH 4/4] net/failsafe: support secondary process

[Thomas Monjalon]

05/03/2019 10:33, Raslan Darawsheh:
> Hi Stephen, 
> 
> From: Stephen Hemminger <stephen@networkplumber.org>
> > On Thu, 28 Feb 2019 15:49:28 +0000
> > Raslan Darawsheh <rasland@mellanox.com> wrote:
> > 
> > > +		eth_dev->rx_pkt_burst =
> > (eth_rx_burst_t)&failsafe_rx_burst;
> > > +		eth_dev->tx_pkt_burst =
> > (eth_tx_burst_t)&failsafe_tx_burst;
> > 
> > Why is cast necessary here. The function signature should match.
> 
> I don't think it's necessary, but this is the same as the primary process implementation

If it's not necessary, better to not add them.
Don't worry about consistency for pointer casting :)