[dpdk-dev] net/tap: fix the vdev data sharing for tun

Message ID 1526107055-26637-1-git-send-email-vipin.varghese@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Varghese, Vipin May 12, 2018, 6:37 a.m. UTC
  Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")

Enables TUN PMD sharing by attaching the port from the shared data.

Cc: zhihong.wang@intel.com
Cc: ferruh.yigit@intel.com

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---
---
 drivers/net/tap/rte_eth_tap.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
  

Comments

Wiles, Keith May 15, 2018, 12:35 p.m. UTC | #1
> On May 12, 2018, at 1:37 AM, Vipin Varghese <vipin.varghese@intel.com> wrote:

> 

> Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")

> 

> Enables TUN PMD sharing by attaching the port from the shared data.

> 

> Cc: zhihong.wang@intel.com

> Cc: ferruh.yigit@intel.com

> 

> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>

> ---

> ---

> drivers/net/tap/rte_eth_tap.c | 13 +++++++++++++

> 1 file changed, 13 insertions(+)

> 

> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c

> index db397d6..ea6d899 100644

> --- a/drivers/net/tap/rte_eth_tap.c

> +++ b/drivers/net/tap/rte_eth_tap.c

> @@ -1657,6 +1657,7 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)

> 	struct rte_kvargs *kvlist = NULL;

> 	char tun_name[RTE_ETH_NAME_MAX_LEN];

> 	char remote_iface[RTE_ETH_NAME_MAX_LEN];

> +	struct rte_eth_dev *eth_dev;

> 

> 	tap_type = 0;

> 	strcpy(tuntap_name, "TUN");

> @@ -1665,6 +1666,18 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)

> 	params = rte_vdev_device_args(dev);

> 	memset(remote_iface, 0, RTE_ETH_NAME_MAX_LEN);

> 

> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&

> +	    strlen(params) == 0) {

> +		eth_dev = rte_eth_dev_attach_secondary(name);

> +		if (!eth_dev) {

> +			TAP_LOG(ERR, "Failed to probe %s", name);

> +			return -1;

> +		}

> +		/* TODO: request info from primary to set up Rx and Tx */


What is the rule (if we have one) about having TODO, FIXME, … like comments in the code? Maybe remove the todo and create a enhancement request for later.

> +		eth_dev->dev_ops = &ops;

> +		return 0;

> +	}

> +

> 	snprintf(tun_name, sizeof(tun_name), "%s%u",

> 		 DEFAULT_TUN_NAME, tun_unit++);

> 

> -- 

> 2.7.4

> 


Regards,
Keith
  
Varghese, Vipin May 15, 2018, 12:56 p.m. UTC | #2
HI Keith,

Thanks for the input, please find my answer inline to email therad

<Snipped>

> > +		/* TODO: request info from primary to set up Rx and Tx */

> 

> What is the rule (if we have one) about having TODO, FIXME, … like comments in

> the code? Maybe remove the todo and create a enhancement request for later.

> 


I am not aware of the 'To Do' planned. This came in as part of logic block from previous commit for all PMD to support Secondary

<Snipped> 

> 

> Regards,

> Keith
  
Ferruh Yigit May 22, 2018, 9:50 a.m. UTC | #3
On 5/15/2018 1:56 PM, Varghese, Vipin wrote:
> HI Keith,
> 
> Thanks for the input, please find my answer inline to email therad
> 
> <Snipped>
> 
>>> +		/* TODO: request info from primary to set up Rx and Tx */
>>
>> What is the rule (if we have one) about having TODO, FIXME, … like comments in
>> the code? Maybe remove the todo and create a enhancement request for later.
>>
> 
> I am not aware of the 'To Do' planned. This came in as part of logic block from previous commit for all PMD to support Secondary

Specially when there is a work not planned for this, agreed to drop todo, can
you please send a new version without that note please?

> 
> <Snipped> 
> 
>>
>> Regards,
>> Keith
>
  

Patch

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index db397d6..ea6d899 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1657,6 +1657,7 @@  rte_pmd_tun_probe(struct rte_vdev_device *dev)
 	struct rte_kvargs *kvlist = NULL;
 	char tun_name[RTE_ETH_NAME_MAX_LEN];
 	char remote_iface[RTE_ETH_NAME_MAX_LEN];
+	struct rte_eth_dev *eth_dev;
 
 	tap_type = 0;
 	strcpy(tuntap_name, "TUN");
@@ -1665,6 +1666,18 @@  rte_pmd_tun_probe(struct rte_vdev_device *dev)
 	params = rte_vdev_device_args(dev);
 	memset(remote_iface, 0, RTE_ETH_NAME_MAX_LEN);
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+	    strlen(params) == 0) {
+		eth_dev = rte_eth_dev_attach_secondary(name);
+		if (!eth_dev) {
+			TAP_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;
+		return 0;
+	}
+
 	snprintf(tun_name, sizeof(tun_name), "%s%u",
 		 DEFAULT_TUN_NAME, tun_unit++);