net/pcap: enable data path on secondary

Message ID 20181105210823.38757-1-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/pcap: enable data path on secondary |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Qi Zhang Nov. 5, 2018, 9:08 p.m. UTC
  Private vdev on secondary is never supported by the new shared
device mode but pdump still relies on a private pcap PMD on secondary.
The patch enables pcap PMD's data path on secondary so that pdump can
work as usual.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 98 ++++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 36 deletions(-)
  

Comments

Ferruh Yigit Nov. 9, 2018, 9:13 p.m. UTC | #1
On 11/5/2018 9:08 PM, Qi Zhang wrote:
> Private vdev on secondary is never supported by the new shared
> device mode but pdump still relies on a private pcap PMD on secondary.

After your updates on hotplug multi process, isn't any virtual PMD added into
secondary will be added into primary too?
Is current pdump logic still valid after latest update?

> The patch enables pcap PMD's data path on secondary so that pdump can
> work as usual.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
  
Qi Zhang Nov. 9, 2018, 9:24 p.m. UTC | #2
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, November 9, 2018 2:14 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: thomas@monjalon.net; dev@dpdk.org; Lin, Xueqin
> <xueqin.lin@intel.com>
> Subject: Re: [PATCH] net/pcap: enable data path on secondary
> 
> On 11/5/2018 9:08 PM, Qi Zhang wrote:
> > Private vdev on secondary is never supported by the new shared device
> > mode but pdump still relies on a private pcap PMD on secondary.
> 
> After your updates on hotplug multi process, isn't any virtual PMD added into
> secondary will be added into primary too?

this becomes mandatory now, PMD have no choice. There is no "rte_dev_probe_local".

> Is current pdump logic still valid after latest update?

the requirement for PCAP PMD from pdump is, it need to support data path on secondary process, so the patch enable it.
Btw, I just got validation result, seems the patch only works on PCAP file dump, it still have issue to dump packet to a netdev interface.
So I want to hold this patch, I will submit a v2 to fix that issue.



> 
> > The patch enables pcap PMD's data path on secondary so that pdump can
> > work as usual.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>
  

Patch

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 7bbe72e25..cbb1fc88e 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -81,6 +81,7 @@  struct pmd_internals {
 	int if_index;
 	int single_iface;
 	int phy_mac;
+	char devargs[ETH_PCAP_ARG_MAXLEN];
 };
 
 struct pmd_devargs {
@@ -892,19 +893,19 @@  static int
 pmd_init_internals(struct rte_vdev_device *vdev,
 		const unsigned int nb_rx_queues,
 		const unsigned int nb_tx_queues,
-		struct pmd_internals **internals,
 		struct rte_eth_dev **eth_dev)
 {
 	struct rte_eth_dev_data *data;
 	unsigned int numa_node = vdev->device.numa_node;
+	struct pmd_internals *internals;
 
 	PMD_LOG(INFO, "Creating pcap-backed ethdev on numa socket %d",
 		numa_node);
 
 	/* reserve an ethdev entry */
-	*eth_dev = rte_eth_vdev_allocate(vdev, sizeof(**internals));
+	*eth_dev = rte_eth_vdev_allocate(vdev, sizeof(*internals));
 	if (!(*eth_dev))
-		return -1;
+		return -ENODEV;
 
 	/* now put it all together
 	 * - store queue data in internals,
@@ -912,21 +913,21 @@  pmd_init_internals(struct rte_vdev_device *vdev,
 	 * - point eth_dev_data to internals
 	 * - and point eth_dev structure to new eth_dev_data structure
 	 */
-	*internals = (*eth_dev)->data->dev_private;
+	internals = (*eth_dev)->data->dev_private;
 	/*
 	 * Interface MAC = 02:70:63:61:70:<iface_idx>
 	 * derived from: 'locally administered':'p':'c':'a':'p':'iface_idx'
 	 * where the middle 4 characters are converted to hex.
 	 */
-	(*internals)->eth_addr = (struct ether_addr) {
+	internals->eth_addr = (struct ether_addr) {
 		.addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
 	};
-	(*internals)->phy_mac = 0;
+	internals->phy_mac = 0;
 	data = (*eth_dev)->data;
 	data->nb_rx_queues = (uint16_t)nb_rx_queues;
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
 	data->dev_link = pmd_link;
-	data->mac_addrs = &(*internals)->eth_addr;
+	data->mac_addrs = &internals->eth_addr;
 
 	/*
 	 * NOTE: we'll replace the data element, of originally allocated
@@ -934,6 +935,10 @@  pmd_init_internals(struct rte_vdev_device *vdev,
 	 */
 	(*eth_dev)->dev_ops = &ops;
 
+	/* store a copy of devargs for secondary process */
+	strlcpy(internals->devargs, rte_vdev_device_args(vdev),
+			ETH_PCAP_ARG_MAXLEN);
+
 	return 0;
 }
 
@@ -1022,10 +1027,11 @@  eth_pcap_update_mac(const char *if_name, struct rte_eth_dev *eth_dev,
 }
 
 static int
-eth_from_pcaps_common(struct rte_vdev_device *vdev,
-		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
-		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
-		struct pmd_internals **internals, struct rte_eth_dev **eth_dev)
+eth_from_pcaps_common(struct pmd_devargs *rx_queues,
+			const unsigned int nb_rx_queues,
+			struct pmd_devargs *tx_queues,
+			const unsigned int nb_tx_queues,
+			struct pmd_internals *internals)
 {
 	unsigned int i;
 
@@ -1035,12 +1041,8 @@  eth_from_pcaps_common(struct rte_vdev_device *vdev,
 	if (tx_queues == NULL && nb_tx_queues > 0)
 		return -1;
 
-	if (pmd_init_internals(vdev, nb_rx_queues, nb_tx_queues, internals,
-			eth_dev) < 0)
-		return -1;
-
 	for (i = 0; i < nb_rx_queues; i++) {
-		struct pcap_rx_queue *rx = &(*internals)->rx_queue[i];
+		struct pcap_rx_queue *rx = &internals->rx_queue[i];
 		struct devargs_queue *queue = &rx_queues->queue[i];
 
 		rx->pcap = queue->pcap;
@@ -1049,7 +1051,7 @@  eth_from_pcaps_common(struct rte_vdev_device *vdev,
 	}
 
 	for (i = 0; i < nb_tx_queues; i++) {
-		struct pcap_tx_queue *tx = &(*internals)->tx_queue[i];
+		struct pcap_tx_queue *tx = &internals->tx_queue[i];
 		struct devargs_queue *queue = &tx_queues->queue[i];
 
 		tx->dumper = queue->dumper;
@@ -1062,17 +1064,17 @@  eth_from_pcaps_common(struct rte_vdev_device *vdev,
 }
 
 static int
-eth_from_pcaps(struct rte_vdev_device *vdev,
+eth_from_pcaps(struct rte_vdev_device *vdev, struct rte_eth_dev *eth_dev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
 		int single_iface, unsigned int using_dumpers)
 {
 	struct pmd_internals *internals = NULL;
-	struct rte_eth_dev *eth_dev = NULL;
 	int ret;
 
-	ret = eth_from_pcaps_common(vdev, rx_queues, nb_rx_queues,
-		tx_queues, nb_tx_queues, &internals, &eth_dev);
+	internals = eth_dev->data->dev_private;
+	ret = eth_from_pcaps_common(rx_queues, nb_rx_queues,
+		tx_queues, nb_tx_queues, internals);
 
 	if (ret < 0)
 		return ret;
@@ -1099,7 +1101,6 @@  eth_from_pcaps(struct rte_vdev_device *vdev,
 	else
 		eth_dev->tx_pkt_burst = eth_pcap_tx;
 
-	rte_eth_dev_probing_finish(eth_dev);
 	return 0;
 }
 
@@ -1108,12 +1109,15 @@  pmd_pcap_probe(struct rte_vdev_device *dev)
 {
 	const char *name;
 	unsigned int is_rx_pcap = 0, is_tx_pcap = 0;
-	struct rte_kvargs *kvlist;
+	struct rte_kvargs *kvlist = NULL;
 	struct pmd_devargs pcaps = {0};
 	struct pmd_devargs dumpers = {0};
-	struct rte_eth_dev *eth_dev;
+	struct rte_eth_dev *eth_dev = NULL;
+	struct pmd_internals *internals = NULL;
+	unsigned int nb_rx_queue;
+	unsigned int nb_tx_queue;
 	int single_iface = 0;
-	int ret;
+	int ret = 0;
 
 	name = rte_vdev_device_name(dev);
 	PMD_LOG(INFO, "Initializing pmd_pcap for %s", name);
@@ -1122,23 +1126,41 @@  pmd_pcap_probe(struct rte_vdev_device *dev)
 	start_cycles = rte_get_timer_cycles();
 	hz = rte_get_timer_hz();
 
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		kvlist = rte_kvargs_parse(rte_vdev_device_args(dev),
+				valid_arguments);
+		if (kvlist == NULL)
+			return -EINVAL;
+		if (rte_kvargs_count(kvlist, ETH_PCAP_IFACE_ARG) == 1) {
+			nb_rx_queue = 1;
+			nb_tx_queue = 1;
+		} else {
+			nb_rx_queue =
+				rte_kvargs_count(kvlist,
+					ETH_PCAP_RX_PCAP_ARG) ? 1 : 0;
+			nb_tx_queue =
+				rte_kvargs_count(kvlist,
+					ETH_PCAP_TX_PCAP_ARG) ? 1 : 0;
+		}
+		ret = pmd_init_internals(dev, nb_rx_queue,
+				nb_tx_queue, &eth_dev);
+		if (ret != 0)
+			goto free_kvlist;
+	} else {
 		eth_dev = rte_eth_dev_attach_secondary(name);
 		if (!eth_dev) {
 			PMD_LOG(ERR, "Failed to probe %s", name);
-			return -1;
+			ret = -ENODEV;
+			goto free_kvlist;
 		}
-		/* TODO: request info from primary to set up Rx and Tx */
 		eth_dev->dev_ops = &ops;
 		eth_dev->device = &dev->device;
-		rte_eth_dev_probing_finish(eth_dev);
-		return 0;
+		internals = eth_dev->data->dev_private;
+		kvlist = rte_kvargs_parse(internals->devargs, valid_arguments);
+		if (kvlist == NULL)
+			return -EINVAL;
 	}
 
-	kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_arguments);
-	if (kvlist == NULL)
-		return -1;
-
 	/*
 	 * If iface argument is passed we open the NICs and use them for
 	 * reading / writing
@@ -1202,8 +1224,12 @@  pmd_pcap_probe(struct rte_vdev_device *dev)
 		goto free_kvlist;
 
 create_eth:
-	ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
-		dumpers.num_of_queue, single_iface, is_tx_pcap);
+	ret = eth_from_pcaps(dev, eth_dev, &pcaps, pcaps.num_of_queue,
+			&dumpers, dumpers.num_of_queue,
+			single_iface, is_tx_pcap);
+	if (ret != 0)
+		goto free_kvlist;
+	rte_eth_dev_probing_finish(eth_dev);
 
 free_kvlist:
 	rte_kvargs_free(kvlist);