[v3,2/2] net/pcap: enable data path for secondary

Message ID 20181114195647.196648-3-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series fix pcap handlers for secondary |

Checks

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

Commit Message

Qi Zhang Nov. 14, 2018, 7:56 p.m. UTC
  Private vdev was the way previously, when pdump developed, now with shared
device mode on virtual devices, pcap data path in secondary is not working.

When secondary adds a virtual device, related data transferred to primary
and primary creates the device and shares device back with secondary.
When pcap device created in primary, pcap handlers (pointers) are process
local and they are not valid for secondary process. This breaks secondary.

So we can't directly share the pcap handlers, but need to create a new set
of handlers for secondary, that's what we done in this patch.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 61 +++++++++++++++++++++++++++++++++++------
 1 file changed, 52 insertions(+), 9 deletions(-)
  

Comments

Ferruh Yigit Nov. 14, 2018, 11:08 p.m. UTC | #1
On 11/14/2018 7:56 PM, Qi Zhang wrote:
> Private vdev was the way previously, when pdump developed, now with shared
> device mode on virtual devices, pcap data path in secondary is not working.
> 
> When secondary adds a virtual device, related data transferred to primary
> and primary creates the device and shares device back with secondary.
> When pcap device created in primary, pcap handlers (pointers) are process
> local and they are not valid for secondary process. This breaks secondary.
> 
> So we can't directly share the pcap handlers, but need to create a new set
> of handlers for secondary, that's what we done in this patch.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

<...>

> @@ -1155,16 +1157,18 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  			PMD_LOG(ERR, "Failed to probe %s", name);
>  			return -1;
>  		}
> -		/* 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;
> -	}
>  
> -	kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_arguments);
> -	if (kvlist == NULL)
> -		return -1;
> +		internal = eth_dev->data->dev_private;
> +
> +		kvlist = rte_kvargs_parse(internal->devargs, valid_arguments);
> +		if (kvlist == NULL)
> +			return -1;

Copying devargs to internal->devargs seems missing, it is still needed right?
  
Qi Zhang Nov. 15, 2018, 12:06 a.m. UTC | #2
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, November 14, 2018 3:08 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 v3 2/2] net/pcap: enable data path for secondary
> 
> On 11/14/2018 7:56 PM, Qi Zhang wrote:
> > Private vdev was the way previously, when pdump developed, now with
> > shared device mode on virtual devices, pcap data path in secondary is not
> working.
> >
> > When secondary adds a virtual device, related data transferred to
> > primary and primary creates the device and shares device back with
> secondary.
> > When pcap device created in primary, pcap handlers (pointers) are
> > process local and they are not valid for secondary process. This breaks
> secondary.
> >
> > So we can't directly share the pcap handlers, but need to create a new
> > set of handlers for secondary, that's what we done in this patch.
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> <...>
> 
> > @@ -1155,16 +1157,18 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
> >  			PMD_LOG(ERR, "Failed to probe %s", name);
> >  			return -1;
> >  		}
> > -		/* 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;
> > -	}
> >
> > -	kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_arguments);
> > -	if (kvlist == NULL)
> > -		return -1;
> > +		internal = eth_dev->data->dev_private;
> > +
> > +		kvlist = rte_kvargs_parse(internal->devargs, valid_arguments);
> > +		if (kvlist == NULL)
> > +			return -1;
> 
> Copying devargs to internal->devargs seems missing, it is still needed right?

Yes it is missed, I just notice I forgot to git format-patch again after I add this...
  

Patch

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 88cb404f1..245288428 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -77,6 +77,7 @@  struct pcap_tx_queue {
 struct pmd_internals {
 	struct pcap_rx_queue rx_queue[RTE_PMD_PCAP_MAX_QUEUES];
 	struct pcap_tx_queue tx_queue[RTE_PMD_PCAP_MAX_QUEUES];
+	char devargs[ETH_PCAP_ARG_MAXLEN];
 	struct ether_addr eth_addr;
 	int if_index;
 	int single_iface;
@@ -1139,6 +1140,7 @@  pmd_pcap_probe(struct rte_vdev_device *dev)
 	struct pmd_devargs pcaps = {0};
 	struct pmd_devargs dumpers = {0};
 	struct rte_eth_dev *eth_dev;
+	struct pmd_internals *internal;
 	int single_iface = 0;
 	int ret;
 
@@ -1155,16 +1157,18 @@  pmd_pcap_probe(struct rte_vdev_device *dev)
 			PMD_LOG(ERR, "Failed to probe %s", name);
 			return -1;
 		}
-		/* 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;
-	}
 
-	kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_arguments);
-	if (kvlist == NULL)
-		return -1;
+		internal = eth_dev->data->dev_private;
+
+		kvlist = rte_kvargs_parse(internal->devargs, valid_arguments);
+		if (kvlist == NULL)
+			return -1;
+	} else {
+		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
@@ -1229,6 +1233,45 @@  pmd_pcap_probe(struct rte_vdev_device *dev)
 		goto free_kvlist;
 
 create_eth:
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		struct pmd_process_private *pp;
+		unsigned int i;
+
+		internal = eth_dev->data->dev_private;
+			pp = (struct pmd_process_private *)
+				rte_zmalloc(NULL,
+					sizeof(struct pmd_process_private),
+					RTE_CACHE_LINE_SIZE);
+
+		if (pp == NULL) {
+			PMD_LOG(ERR,
+				"Failed to allocate memory for process private");
+			return -1;
+		}
+
+		eth_dev->dev_ops = &ops;
+		eth_dev->device = &dev->device;
+
+		/* setup process private */
+		for (i = 0; i < pcaps.num_of_queue; i++)
+			pp->rx_pcap[i] = pcaps.queue[i].pcap;
+
+		for (i = 0; i < dumpers.num_of_queue; i++) {
+			pp->tx_dumper[i] = dumpers.queue[i].dumper;
+			pp->tx_pcap[i] = dumpers.queue[i].pcap;
+		}
+
+		eth_dev->process_private = pp;
+		eth_dev->rx_pkt_burst = eth_pcap_rx;
+		if (is_tx_pcap)
+			eth_dev->tx_pkt_burst = eth_pcap_tx_dumper;
+		else
+			eth_dev->tx_pkt_burst = eth_pcap_tx;
+
+		rte_eth_dev_probing_finish(eth_dev);
+		return 0;
+	}
+
 	ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
 		dumpers.num_of_queue, single_iface, is_tx_pcap);