[dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args

[dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args

[Vipin Varghese]

Allow TAP PMD to pass user desired MAC address as argument.
The argument value is processed as string, where each 2 bytes
are converted to HEX MAC address after validation.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---
 doc/guides/nics/tap.rst       |  6 +++++
 drivers/net/tap/rte_eth_tap.c | 62 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index dc6f834..6b083c8 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -69,6 +69,12 @@ for each interface string containing ``mac=fixed``. The MAC address is formatted
 as 00:'d':'t':'a':'p':[00-FF]. Convert the characters to hex and you get the
 actual MAC address: ``00:64:74:61:70:[00-FF]``.
 
+   --vdev=net_tap0,mac="00:64:74:61:70:11"
+
+The MAC address will have a user value passed as string. The MAC address is in
+format with delimeter ``:``. The string is byte converted to hex and you get
+the actual MAC address: ``00:64:74:61:70:11``.
+
 It is possible to specify a remote netdevice to capture packets from by adding
 ``remote=foo1``, for example::
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 29d6356..3489b04 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -49,7 +49,14 @@
 #define ETH_TAP_MAC_ARG         "mac"
 #define ETH_TAP_MAC_FIXED       "fixed"
 
+#define ETH_TAP_MAC_STR_FXD     1
+#define ETH_TAP_MAC_STR_USR     2
+#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
+#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
+#define ETH_TAP_MAC_ARG_FMT     "["ETH_TAP_MAC_FIXED "|" ETH_TAP_USR_MAC_FMT"]"
+
 static struct rte_vdev_driver pmd_tap_drv;
+static unsigned char user_mac[ETHER_ADDR_LEN];
 
 static const char *valid_arguments[] = {
 	ETH_TAP_IFACE_ARG,
@@ -1397,13 +1404,20 @@ enum ioctl_mode {
 		pmd->txq[i].fd = -1;
 	}
 
-	if (fixed_mac_type) {
+	if (fixed_mac_type == ETH_TAP_MAC_STR_FXD) {
 		/* fixed mac = 00:64:74:61:70:<iface_idx> */
 		static int iface_idx;
 		char mac[ETHER_ADDR_LEN] = "\0dtap";
 
 		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
 		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
+	} else if (fixed_mac_type == ETH_TAP_MAC_STR_USR) {
+		RTE_LOG(INFO, PMD,
+			"%s; user MAC (%02x:%02x:%02x:%02x:%02x:%02x) argument\n",
+			pmd->name,
+			user_mac[0], user_mac[1], user_mac[2],
+			user_mac[3], user_mac[4], user_mac[5]);
+			rte_memcpy(&pmd->eth_addr, user_mac, ETHER_ADDR_LEN);
 	} else {
 		eth_random_addr((uint8_t *)&pmd->eth_addr);
 	}
@@ -1577,10 +1591,48 @@ enum ioctl_mode {
 	     const char *value,
 	     void *extra_args)
 {
-	if (value &&
-	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
-		*(int *)extra_args = 1;
+	char mac_temp[20] = {0}, *mac_byte = NULL;
+	unsigned int index = 0;
+
+	if (!value)
+		return 0;
+
+	if (!strncasecmp(ETH_TAP_MAC_FIXED, value,
+			strlen(ETH_TAP_MAC_FIXED))) {
+		*(int *)extra_args = ETH_TAP_MAC_STR_FXD;
+		goto success;
+	}
+
+	if (strlen(value) == 17) {
+		strncpy(mac_temp, value, 18);
+		mac_temp[19] = '\0';
+		mac_byte = strtok(mac_temp, ":");
+
+		while ((mac_byte != NULL) &&
+				strspn(mac_byte, ETH_TAP_CMP_MAC_FMT) &&
+				strspn((mac_byte + 1), ETH_TAP_CMP_MAC_FMT) &&
+				strlen(mac_byte) == 2) {
+			user_mac[index] = strtoul(mac_byte, NULL, 16);
+			mac_byte = strtok(NULL, ":");
+			index += 1;
+		}
+
+		if (index != 6)
+			goto error;
+
+		*(int *)extra_args = ETH_TAP_MAC_STR_USR;
+	} else {
+		goto error;
+	}
+
+success:
+	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
+		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
+	return -1;
 }
 
 /* Open a TAP interface device.
@@ -1716,5 +1768,5 @@ enum ioctl_mode {
 RTE_PMD_REGISTER_PARAM_STRING(net_tap,
 			      ETH_TAP_IFACE_ARG "=<string> "
 			      ETH_TAP_SPEED_ARG "=<int> "
-			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
+			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT
 			      ETH_TAP_REMOTE_ARG "=<string>");
-- 
1.9.1

Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args

[Pascal Mazon]

Hi,

You didn't address my request about not using a global value. Was there
a good reason?

I paste it here again as a reminder:

  Can you also not use a global value for user_mac, but instead change the
  last argument for eth_dev_tap_create():
  Use directly a char mac[ETHER_ADDR_LEN], automatic variable from
  rte_pmd_tap_probe().
  In set_mac_type(), you can check either for "fixed" or a correct custom
  mac address.
  Then eth_dev_tap_create() can check if the provided mac is empty (!fixed
  and !custom_mac), to generate a random one.

Additional comments inline.

Best regards,
Pascal

On 31/01/2018 19:22, Vipin Varghese wrote:
> Allow TAP PMD to pass user desired MAC address as argument.
> The argument value is processed as string, where each 2 bytes
> are converted to HEX MAC address after validation.
>
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> ---
>  doc/guides/nics/tap.rst       |  6 +++++
>  drivers/net/tap/rte_eth_tap.c | 62 +++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
> index dc6f834..6b083c8 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -69,6 +69,12 @@ for each interface string containing ``mac=fixed``. The MAC address is formatted
>  as 00:'d':'t':'a':'p':[00-FF]. Convert the characters to hex and you get the
>  actual MAC address: ``00:64:74:61:70:[00-FF]``.
>  
> +   --vdev=net_tap0,mac="00:64:74:61:70:11"
> +
> +The MAC address will have a user value passed as string. The MAC address is in
> +format with delimeter ``:``. The string is byte converted to hex and you get
> +the actual MAC address: ``00:64:74:61:70:11``.
> +
>  It is possible to specify a remote netdevice to capture packets from by adding
>  ``remote=foo1``, for example::
>  
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 29d6356..3489b04 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -49,7 +49,14 @@
>  #define ETH_TAP_MAC_ARG         "mac"
>  #define ETH_TAP_MAC_FIXED       "fixed"
>  
> +#define ETH_TAP_MAC_STR_FXD     1
> +#define ETH_TAP_MAC_STR_USR     2
> +#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
> +#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
> +#define ETH_TAP_MAC_ARG_FMT     "["ETH_TAP_MAC_FIXED "|" ETH_TAP_USR_MAC_FMT"]"
> +
>  static struct rte_vdev_driver pmd_tap_drv;
> +static unsigned char user_mac[ETHER_ADDR_LEN];
>  
>  static const char *valid_arguments[] = {
>  	ETH_TAP_IFACE_ARG,
> @@ -1397,13 +1404,20 @@ enum ioctl_mode {
>  		pmd->txq[i].fd = -1;
>  	}
>  
> -	if (fixed_mac_type) {
> +	if (fixed_mac_type == ETH_TAP_MAC_STR_FXD) {
>  		/* fixed mac = 00:64:74:61:70:<iface_idx> */
>  		static int iface_idx;
>  		char mac[ETHER_ADDR_LEN] = "\0dtap";
>  
>  		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
>  		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
> +	} else if (fixed_mac_type == ETH_TAP_MAC_STR_USR) {
> +		RTE_LOG(INFO, PMD,
> +			"%s; user MAC (%02x:%02x:%02x:%02x:%02x:%02x) argument\n",
Shouldn't it be a colon there? "%s:"
> +			pmd->name,
> +			user_mac[0], user_mac[1], user_mac[2],
> +			user_mac[3], user_mac[4], user_mac[5]);
> +			rte_memcpy(&pmd->eth_addr, user_mac, ETHER_ADDR_LEN);
>  	} else {
>  		eth_random_addr((uint8_t *)&pmd->eth_addr);
>  	}
> @@ -1577,10 +1591,48 @@ enum ioctl_mode {
>  	     const char *value,
>  	     void *extra_args)
>  {
> -	if (value &&
> -	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
> -		*(int *)extra_args = 1;
> +	char mac_temp[20] = {0}, *mac_byte = NULL;
Instead of hardcoded values, I'd use
mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1]
> +	unsigned int index = 0;
> +
> +	if (!value)
> +		return 0;
> +
> +	if (!strncasecmp(ETH_TAP_MAC_FIXED, value,
> +			strlen(ETH_TAP_MAC_FIXED))) {
> +		*(int *)extra_args = ETH_TAP_MAC_STR_FXD;
> +		goto success;
> +	}
> +
> +	if (strlen(value) == 17) {
And here 17 => strlen(ETH_TAP_USR_MAC_FMT)
> +		strncpy(mac_temp, value, 18);
> +		mac_temp[19] = '\0';
Instead of those two lines, I'd rather have snprintf(mac_temp,
sizeof(mac_temp), "%s", value).
It handles the trailing \0 nicely.
> +		mac_byte = strtok(mac_temp, ":");
> +
> +		while ((mac_byte != NULL) &&
> +				strspn(mac_byte, ETH_TAP_CMP_MAC_FMT) &&
> +				strspn((mac_byte + 1), ETH_TAP_CMP_MAC_FMT) &&
> +				strlen(mac_byte) == 2) {
> +			user_mac[index] = strtoul(mac_byte, NULL, 16);
> +			mac_byte = strtok(NULL, ":");
> +			index += 1;
> +		}
> +
> +		if (index != 6)
> +			goto error;
> +
> +		*(int *)extra_args = ETH_TAP_MAC_STR_USR;
> +	} else {
> +		goto error;
> +	}
> +
> +success:
> +	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
>  	return 0;
> +
> +error:
> +	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
> +		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
> +	return -1;
>  }
>  
>  /* Open a TAP interface device.
> @@ -1716,5 +1768,5 @@ enum ioctl_mode {
>  RTE_PMD_REGISTER_PARAM_STRING(net_tap,
>  			      ETH_TAP_IFACE_ARG "=<string> "
>  			      ETH_TAP_SPEED_ARG "=<int> "
> -			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
> +			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT
>  			      ETH_TAP_REMOTE_ARG "=<string>");

Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args

[Varghese, Vipin]

Hi Pascal,

Sincere apologizes, I think I missed out since rework was asked. Please find my answers inline to the comment

> -----Original Message-----
> From: Pascal Mazon [mailto:pascal.mazon@6wind.com]
> Sent: Friday, February 2, 2018 9:16 AM
> To: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>
> Subject: Re: [PATCH] net/tap: allow user MAC to be passed as args
> 
> Hi,
> 
> You didn't address my request about not using a global value. Was there a good
> reason?
> 
> I paste it here again as a reminder:
> 
>   Can you also not use a global value for user_mac, but instead change the
>   last argument for eth_dev_tap_create():
>   Use directly a char mac[ETHER_ADDR_LEN], automatic variable from
>   rte_pmd_tap_probe().
>   In set_mac_type(), you can check either for "fixed" or a correct custom
>   mac address.
>   Then eth_dev_tap_create() can check if the provided mac is empty (!fixed
>   and !custom_mac), to generate a random one.

Last argument for eth_dev_tap_create is ' int fixed_mac_type '. Would like me to change this to 'uint64_t fixed_mac_type' to accommodate the MAC address?

Note: Should we change the API arguments?

> 
> Additional comments inline.
> 
> Best regards,
> Pascal
> 
> On 31/01/2018 19:22, Vipin Varghese wrote:


<Snipped>

> >  #define ETH_TAP_MAC_ARG         "mac"
> >  #define ETH_TAP_MAC_FIXED       "fixed"
> >
> > +#define ETH_TAP_MAC_STR_FXD     1
> > +#define ETH_TAP_MAC_STR_USR     2
> > +#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
> > +#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
> > +#define ETH_TAP_MAC_ARG_FMT     "["ETH_TAP_MAC_FIXED "|"
> ETH_TAP_USR_MAC_FMT"]"
> > +
> >  static struct rte_vdev_driver pmd_tap_drv;
> > +static unsigned char user_mac[ETHER_ADDR_LEN];
> >
> >  static const char *valid_arguments[] = {
> >  	ETH_TAP_IFACE_ARG,
> > @@ -1397,13 +1404,20 @@ enum ioctl_mode {
> >  		pmd->txq[i].fd = -1;
> >  	}
> >
> > -	if (fixed_mac_type) {
> > +	if (fixed_mac_type == ETH_TAP_MAC_STR_FXD) {
> >  		/* fixed mac = 00:64:74:61:70:<iface_idx> */
> >  		static int iface_idx;
> >  		char mac[ETHER_ADDR_LEN] = "\0dtap";
> >
> >  		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
> >  		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
> > +	} else if (fixed_mac_type == ETH_TAP_MAC_STR_USR) {
> > +		RTE_LOG(INFO, PMD,
> > +			"%s; user MAC (%02x:%02x:%02x:%02x:%02x:%02x)
> argument\n",
> Shouldn't it be a colon there? "%s:"

Ok, I can make this change.

<Snipped>

> > +	char mac_temp[20] = {0}, *mac_byte = NULL;
> Instead of hardcoded values, I'd use
> mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1]

Ok, I can make this change.

<Snipped>

> > +
> > +	if (strlen(value) == 17) {
> And here 17 => strlen(ETH_TAP_USR_MAC_FMT)

Ok

> > +		strncpy(mac_temp, value, 18);
> > +		mac_temp[19] = '\0';
> Instead of those two lines, I'd rather have snprintf(mac_temp,
> sizeof(mac_temp), "%s", value).
> It handles the trailing \0 nicely.

OK, I will check the same.

> > +		mac_byte = strtok(mac_temp, ":");


<Snipped>

Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args

[Varghese, Vipin]

Hi Pascal,

Waiting for your inputs in changing the function argument from int32_t to uint64_t.

Thanks
Vipin Varghese

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Varghese, Vipin
> Sent: Friday, February 2, 2018 9:50 AM
> To: Pascal Mazon <pascal.mazon@6wind.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args
> 
> Hi Pascal,
> 
> Sincere apologizes, I think I missed out since rework was asked. Please find my
> answers inline to the comment
> 
> > -----Original Message-----
> > From: Pascal Mazon [mailto:pascal.mazon@6wind.com]
> > Sent: Friday, February 2, 2018 9:16 AM
> > To: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Jain, Deepak K
> > <deepak.k.jain@intel.com>
> > Subject: Re: [PATCH] net/tap: allow user MAC to be passed as args
> >
> > Hi,
> >
> > You didn't address my request about not using a global value. Was
> > there a good reason?
> >
> > I paste it here again as a reminder:
> >
> >   Can you also not use a global value for user_mac, but instead change the
> >   last argument for eth_dev_tap_create():
> >   Use directly a char mac[ETHER_ADDR_LEN], automatic variable from
> >   rte_pmd_tap_probe().
> >   In set_mac_type(), you can check either for "fixed" or a correct custom
> >   mac address.
> >   Then eth_dev_tap_create() can check if the provided mac is empty (!fixed
> >   and !custom_mac), to generate a random one.
> 
> Last argument for eth_dev_tap_create is ' int fixed_mac_type '. Would like me
> to change this to 'uint64_t fixed_mac_type' to accommodate the MAC address?
> 
> Note: Should we change the API arguments?
> 
> >
> > Additional comments inline.
> >
> > Best regards,
> > Pascal
> >
> > On 31/01/2018 19:22, Vipin Varghese wrote:
> 
> 
> <Snipped>
> 
> > >  #define ETH_TAP_MAC_ARG         "mac"
> > >  #define ETH_TAP_MAC_FIXED       "fixed"
> > >
> > > +#define ETH_TAP_MAC_STR_FXD     1
> > > +#define ETH_TAP_MAC_STR_USR     2
> > > +#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
> > > +#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
> > > +#define ETH_TAP_MAC_ARG_FMT     "["ETH_TAP_MAC_FIXED "|"
> > ETH_TAP_USR_MAC_FMT"]"
> > > +
> > >  static struct rte_vdev_driver pmd_tap_drv;
> > > +static unsigned char user_mac[ETHER_ADDR_LEN];
> > >
> > >  static const char *valid_arguments[] = {
> > >  	ETH_TAP_IFACE_ARG,
> > > @@ -1397,13 +1404,20 @@ enum ioctl_mode {
> > >  		pmd->txq[i].fd = -1;
> > >  	}
> > >
> > > -	if (fixed_mac_type) {
> > > +	if (fixed_mac_type == ETH_TAP_MAC_STR_FXD) {
> > >  		/* fixed mac = 00:64:74:61:70:<iface_idx> */
> > >  		static int iface_idx;
> > >  		char mac[ETHER_ADDR_LEN] = "\0dtap";
> > >
> > >  		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
> > >  		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
> > > +	} else if (fixed_mac_type == ETH_TAP_MAC_STR_USR) {
> > > +		RTE_LOG(INFO, PMD,
> > > +			"%s; user MAC (%02x:%02x:%02x:%02x:%02x:%02x)
> > argument\n",
> > Shouldn't it be a colon there? "%s:"
> 
> Ok, I can make this change.
> 
> <Snipped>
> 
> > > +	char mac_temp[20] = {0}, *mac_byte = NULL;
> > Instead of hardcoded values, I'd use
> > mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1]
> 
> Ok, I can make this change.
> 
> <Snipped>
> 
> > > +
> > > +	if (strlen(value) == 17) {
> > And here 17 => strlen(ETH_TAP_USR_MAC_FMT)
> 
> Ok
> 
> > > +		strncpy(mac_temp, value, 18);
> > > +		mac_temp[19] = '\0';
> > Instead of those two lines, I'd rather have snprintf(mac_temp,
> > sizeof(mac_temp), "%s", value).
> > It handles the trailing \0 nicely.
> 
> OK, I will check the same.
> 
> > > +		mac_byte = strtok(mac_temp, ":");
> 
> 
> <Snipped>

[dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Pascal Mazon]

From: Vipin Varghese <vipin.varghese@intel.com>

Allow TAP PMD to pass user desired MAC address as argument.
The argument value is processed as string, where each 2 bytes
are converted to HEX MAC address after validation.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---

Hi Vipin,

I suggest the following patch for your MAC address argument, if that
suits you.
I removed ETH_TAP_MAC_STR_FXD and ETH_TAP_MAC_STR_USR that were not
really useful. I find it nicer to deal with MAC mostly inside
set_mac_type().
What do you think? Can you test that it fits your needs?

Best regards,
Pascal

 doc/guides/nics/tap.rst       |  6 ++++
 drivers/net/tap/rte_eth_tap.c | 80 +++++++++++++++++++++++++++++--------------
 2 files changed, 60 insertions(+), 26 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index dc6f834ca0a1..6b083c846b21 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -69,6 +69,12 @@ for each interface string containing ``mac=fixed``. The MAC address is formatted
 as 00:'d':'t':'a':'p':[00-FF]. Convert the characters to hex and you get the
 actual MAC address: ``00:64:74:61:70:[00-FF]``.
 
+   --vdev=net_tap0,mac="00:64:74:61:70:11"
+
+The MAC address will have a user value passed as string. The MAC address is in
+format with delimeter ``:``. The string is byte converted to hex and you get
+the actual MAC address: ``00:64:74:61:70:11``.
+
 It is possible to specify a remote netdevice to capture packets from by adding
 ``remote=foo1``, for example::
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 29d635613c0b..d9e5ec92b8f0 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -49,6 +49,10 @@
 #define ETH_TAP_MAC_ARG         "mac"
 #define ETH_TAP_MAC_FIXED       "fixed"
 
+#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
+#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
+#define ETH_TAP_MAC_ARG_FMT     "["ETH_TAP_MAC_FIXED "|" ETH_TAP_USR_MAC_FMT"]"
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -278,15 +282,8 @@ tap_rx_offload_get_queue_capa(void)
 static bool
 tap_rxq_are_offloads_valid(struct rte_eth_dev *dev, uint64_t offloads)
 {
-	uint64_t port_offloads = dev->data->dev_conf.rxmode.offloads;
-	uint64_t queue_supp_offloads = tap_rx_offload_get_queue_capa();
-	uint64_t port_supp_offloads = tap_rx_offload_get_port_capa();
-
-	if ((offloads & (queue_supp_offloads | port_supp_offloads)) !=
-	    offloads)
-		return false;
-	if ((port_offloads ^ offloads) & port_supp_offloads)
-		return false;
+	(void)dev;
+	(void)offloads;
 	return true;
 }
 
@@ -1335,7 +1332,7 @@ static const struct eth_dev_ops ops = {
 
 static int
 eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
-		   char *remote_iface, int fixed_mac_type)
+		   char *remote_iface, struct ether_addr *mac_addr)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev;
@@ -1397,16 +1394,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
 		pmd->txq[i].fd = -1;
 	}
 
-	if (fixed_mac_type) {
-		/* fixed mac = 00:64:74:61:70:<iface_idx> */
-		static int iface_idx;
-		char mac[ETHER_ADDR_LEN] = "\0dtap";
-
-		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
-		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
-	} else {
+	if (is_zero_ether_addr(mac_addr))
 		eth_random_addr((uint8_t *)&pmd->eth_addr);
-	}
+	else
+		rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(mac_addr));
 
 	/* Immediately create the netdevice (this will create the 1st queue). */
 	/* rx queue */
@@ -1577,10 +1568,47 @@ set_mac_type(const char *key __rte_unused,
 	     const char *value,
 	     void *extra_args)
 {
-	if (value &&
-	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
-		*(int *)extra_args = 1;
+	char mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1], *mac_byte = NULL;
+	struct ether_addr *user_mac = extra_args;
+	unsigned int index = 0;
+
+	if (!value)
+		return 0;
+
+	if (!strncasecmp(ETH_TAP_MAC_FIXED, value,
+			strlen(ETH_TAP_MAC_FIXED))) {
+		static int iface_idx;
+
+		/* fixed mac = 00:64:74:61:70:<iface_idx> */
+		memcpy((char *)user_mac->addr_bytes, "\0dtap", ETHER_ADDR_LEN);
+		user_mac->addr_bytes[ETHER_ADDR_LEN - 1] = iface_idx++ + '0';
+		goto success;
+	}
+
+	if (strlen(value) != strlen(ETH_TAP_USR_MAC_FMT))
+		goto error;
+
+	snprintf(mac_temp, sizeof(mac_temp), "%s", value);
+	mac_byte = strtok(mac_temp, ":");
+
+	while ((mac_byte != NULL) &&
+			strspn(mac_byte, ETH_TAP_CMP_MAC_FMT) &&
+			strlen(mac_byte) == 2) {
+		user_mac->addr_bytes[index++] = (char) strtoul(mac_byte, NULL, 16);
+		mac_byte = strtok(NULL, ":");
+	}
+
+	if (index != 6)
+		goto error;
+
+success:
+	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
+		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
+	return -1;
 }
 
 /* Open a TAP interface device.
@@ -1594,7 +1622,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 	int speed;
 	char tap_name[RTE_ETH_NAME_MAX_LEN];
 	char remote_iface[RTE_ETH_NAME_MAX_LEN];
-	int fixed_mac_type = 0;
+	struct ether_addr user_mac = {0};
 
 	name = rte_vdev_device_name(dev);
 	params = rte_vdev_device_args(dev);
@@ -1640,7 +1668,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 				ret = rte_kvargs_process(kvlist,
 							 ETH_TAP_MAC_ARG,
 							 &set_mac_type,
-							 &fixed_mac_type);
+							 &user_mac);
 				if (ret == -1)
 					goto leave;
 			}
@@ -1651,7 +1679,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
 		name, tap_name);
 
-	ret = eth_dev_tap_create(dev, tap_name, remote_iface, fixed_mac_type);
+	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
 
 leave:
 	if (ret == -1) {
@@ -1716,5 +1744,5 @@ RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
 RTE_PMD_REGISTER_PARAM_STRING(net_tap,
 			      ETH_TAP_IFACE_ARG "=<string> "
 			      ETH_TAP_SPEED_ARG "=<int> "
-			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
+			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT
 			      ETH_TAP_REMOTE_ARG "=<string>");
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Varghese, Vipin]

Hi Pascal,

Thanks for the update, I see your point instead of mixing up with int32_t and uint64_t; converting to ' struct ether_addr' makes sense.

I will check internally and thanks once again for the suggestion.

Thanks
Vipin Varghese

> -----Original Message-----
> From: Pascal Mazon [mailto:pascal.mazon@6wind.com]
> Sent: Monday, February 5, 2018 3:55 PM
> To: dev@dpdk.org; Varghese, Vipin <vipin.varghese@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>; Pascal Mazon <pascal.mazon@6wind.com>
> Subject: [PATCH v2] net/tap: allow user MAC to be passed as args
> 
> From: Vipin Varghese <vipin.varghese@intel.com>
> 
> Allow TAP PMD to pass user desired MAC address as argument.
> The argument value is processed as string, where each 2 bytes are converted to
> HEX MAC address after validation.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
> 
> Hi Vipin,
> 
> I suggest the following patch for your MAC address argument, if that suits you.
> I removed ETH_TAP_MAC_STR_FXD and ETH_TAP_MAC_STR_USR that were not
> really useful. I find it nicer to deal with MAC mostly inside set_mac_type().
> What do you think? Can you test that it fits your needs?
> 
> Best regards,
> Pascal
> 
>  doc/guides/nics/tap.rst       |  6 ++++
>  drivers/net/tap/rte_eth_tap.c | 80 +++++++++++++++++++++++++++++-----------
> ---
>  2 files changed, 60 insertions(+), 26 deletions(-)
> 
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst index
> dc6f834ca0a1..6b083c846b21 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -69,6 +69,12 @@ for each interface string containing ``mac=fixed``. The
> MAC address is formatted  as 00:'d':'t':'a':'p':[00-FF]. Convert the characters to
> hex and you get the  actual MAC address: ``00:64:74:61:70:[00-FF]``.
> 
> +   --vdev=net_tap0,mac="00:64:74:61:70:11"
> +
> +The MAC address will have a user value passed as string. The MAC
> +address is in format with delimeter ``:``. The string is byte converted
> +to hex and you get the actual MAC address: ``00:64:74:61:70:11``.
> +
>  It is possible to specify a remote netdevice to capture packets from by adding
> ``remote=foo1``, for example::
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index
> 29d635613c0b..d9e5ec92b8f0 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -49,6 +49,10 @@
>  #define ETH_TAP_MAC_ARG         "mac"
>  #define ETH_TAP_MAC_FIXED       "fixed"
> 
> +#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
> +#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
> +#define ETH_TAP_MAC_ARG_FMT     "["ETH_TAP_MAC_FIXED "|"
> ETH_TAP_USR_MAC_FMT"]"
> +
>  static struct rte_vdev_driver pmd_tap_drv;
> 
>  static const char *valid_arguments[] = { @@ -278,15 +282,8 @@
> tap_rx_offload_get_queue_capa(void)
>  static bool
>  tap_rxq_are_offloads_valid(struct rte_eth_dev *dev, uint64_t offloads)  {
> -	uint64_t port_offloads = dev->data->dev_conf.rxmode.offloads;
> -	uint64_t queue_supp_offloads = tap_rx_offload_get_queue_capa();
> -	uint64_t port_supp_offloads = tap_rx_offload_get_port_capa();
> -
> -	if ((offloads & (queue_supp_offloads | port_supp_offloads)) !=
> -	    offloads)
> -		return false;
> -	if ((port_offloads ^ offloads) & port_supp_offloads)
> -		return false;
> +	(void)dev;
> +	(void)offloads;
>  	return true;
>  }
> 
> @@ -1335,7 +1332,7 @@ static const struct eth_dev_ops ops = {
> 
>  static int
>  eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> -		   char *remote_iface, int fixed_mac_type)
> +		   char *remote_iface, struct ether_addr *mac_addr)
>  {
>  	int numa_node = rte_socket_id();
>  	struct rte_eth_dev *dev;
> @@ -1397,16 +1394,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev,
> char *tap_name,
>  		pmd->txq[i].fd = -1;
>  	}
> 
> -	if (fixed_mac_type) {
> -		/* fixed mac = 00:64:74:61:70:<iface_idx> */
> -		static int iface_idx;
> -		char mac[ETHER_ADDR_LEN] = "\0dtap";
> -
> -		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
> -		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
> -	} else {
> +	if (is_zero_ether_addr(mac_addr))
>  		eth_random_addr((uint8_t *)&pmd->eth_addr);
> -	}
> +	else
> +		rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(mac_addr));
> 
>  	/* Immediately create the netdevice (this will create the 1st queue). */
>  	/* rx queue */
> @@ -1577,10 +1568,47 @@ set_mac_type(const char *key __rte_unused,
>  	     const char *value,
>  	     void *extra_args)
>  {
> -	if (value &&
> -	    !strncasecmp(ETH_TAP_MAC_FIXED, value,
> strlen(ETH_TAP_MAC_FIXED)))
> -		*(int *)extra_args = 1;
> +	char mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1], *mac_byte =
> NULL;
> +	struct ether_addr *user_mac = extra_args;
> +	unsigned int index = 0;
> +
> +	if (!value)
> +		return 0;
> +
> +	if (!strncasecmp(ETH_TAP_MAC_FIXED, value,
> +			strlen(ETH_TAP_MAC_FIXED))) {
> +		static int iface_idx;
> +
> +		/* fixed mac = 00:64:74:61:70:<iface_idx> */
> +		memcpy((char *)user_mac->addr_bytes, "\0dtap",
> ETHER_ADDR_LEN);
> +		user_mac->addr_bytes[ETHER_ADDR_LEN - 1] = iface_idx++ +
> '0';
> +		goto success;
> +	}
> +
> +	if (strlen(value) != strlen(ETH_TAP_USR_MAC_FMT))
> +		goto error;
> +
> +	snprintf(mac_temp, sizeof(mac_temp), "%s", value);
> +	mac_byte = strtok(mac_temp, ":");
> +
> +	while ((mac_byte != NULL) &&
> +			strspn(mac_byte, ETH_TAP_CMP_MAC_FMT) &&
> +			strlen(mac_byte) == 2) {
> +		user_mac->addr_bytes[index++] = (char) strtoul(mac_byte,
> NULL, 16);
> +		mac_byte = strtok(NULL, ":");
> +	}
> +
> +	if (index != 6)
> +		goto error;
> +
> +success:
> +	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
>  	return 0;
> +
> +error:
> +	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
> +		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
> +	return -1;
>  }
> 
>  /* Open a TAP interface device.
> @@ -1594,7 +1622,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>  	int speed;
>  	char tap_name[RTE_ETH_NAME_MAX_LEN];
>  	char remote_iface[RTE_ETH_NAME_MAX_LEN];
> -	int fixed_mac_type = 0;
> +	struct ether_addr user_mac = {0};
> 
>  	name = rte_vdev_device_name(dev);
>  	params = rte_vdev_device_args(dev);
> @@ -1640,7 +1668,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>  				ret = rte_kvargs_process(kvlist,
>  							 ETH_TAP_MAC_ARG,
>  							 &set_mac_type,
> -							 &fixed_mac_type);
> +							 &user_mac);
>  				if (ret == -1)
>  					goto leave;
>  			}
> @@ -1651,7 +1679,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>  	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
>  		name, tap_name);
> 
> -	ret = eth_dev_tap_create(dev, tap_name, remote_iface,
> fixed_mac_type);
> +	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
> 
>  leave:
>  	if (ret == -1) {
> @@ -1716,5 +1744,5 @@ RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
> RTE_PMD_REGISTER_PARAM_STRING(net_tap,
>  			      ETH_TAP_IFACE_ARG "=<string> "
>  			      ETH_TAP_SPEED_ARG "=<int> "
> -			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
> +			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT
>  			      ETH_TAP_REMOTE_ARG "=<string>");
> --
> 2.11.0

Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Varghese, Vipin]

Hi Pascal,

Please find my observation inline

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Varghese, Vipin
> Sent: Monday, February 5, 2018 3:59 PM
> To: Pascal Mazon <pascal.mazon@6wind.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
> 
> Hi Pascal,
> 
> Thanks for the update, I see your point instead of mixing up with int32_t and
> uint64_t; converting to ' struct ether_addr' makes sense.
> 
> I will check internally and thanks once again for the suggestion.
> 
> Thanks
> Vipin Varghese
> 
> > -----Original Message-----
> > From: Pascal Mazon [mailto:pascal.mazon@6wind.com]
> > Sent: Monday, February 5, 2018 3:55 PM
> > To: dev@dpdk.org; Varghese, Vipin <vipin.varghese@intel.com>
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Jain, Deepak K
> > <deepak.k.jain@intel.com>; Pascal Mazon <pascal.mazon@6wind.com>
> > Subject: [PATCH v2] net/tap: allow user MAC to be passed as args
> >
> > From: Vipin Varghese <vipin.varghese@intel.com>
> >
> > Allow TAP PMD to pass user desired MAC address as argument.
> > The argument value is processed as string, where each 2 bytes are
> > converted to HEX MAC address after validation.
> >
> > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> >
> > Hi Vipin,
> >
> > I suggest the following patch for your MAC address argument, if that suits you.
> > I removed ETH_TAP_MAC_STR_FXD and ETH_TAP_MAC_STR_USR that were
> not
> > really useful. I find it nicer to deal with MAC mostly inside set_mac_type().
> > What do you think? Can you test that it fits your needs?
> >
> > Best regards,
> > Pascal
> >
> >  doc/guides/nics/tap.rst       |  6 ++++
> >  drivers/net/tap/rte_eth_tap.c | 80
> > +++++++++++++++++++++++++++++-----------
> > ---
> >  2 files changed, 60 insertions(+), 26 deletions(-)
> >
> > diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst index
> > dc6f834ca0a1..6b083c846b21 100644
> > --- a/doc/guides/nics/tap.rst
> > +++ b/doc/guides/nics/tap.rst

<Snipped>

> > +
> > +	while ((mac_byte != NULL) &&
> > +			strspn(mac_byte, ETH_TAP_CMP_MAC_FMT) &&

Added ' strspn(mac_byte + 1, ETH_TAP_CMP_MAC_FMT) &&'; because 2 character validation was not taking place

<Snipped>

> >
> >  /* Open a TAP interface device.
> > @@ -1594,7 +1622,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
> >  	int speed;
> >  	char tap_name[RTE_ETH_NAME_MAX_LEN];
> >  	char remote_iface[RTE_ETH_NAME_MAX_LEN];
> > -	int fixed_mac_type = 0;
> > +	struct ether_addr user_mac = {0};

Changed to ' struct ether_addr user_mac = { .addr_bytes={0}};'; to prevent compilation failure.

> >
> >  	name = rte_vdev_device_name(dev);
> >  	params = rte_vdev_device_args(dev);
> > @@ -1640,7 +1668,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
> >  				ret = rte_kvargs_process(kvlist,
> >  							 ETH_TAP_MAC_ARG,
> >  							 &set_mac_type,
> > -							 &fixed_mac_type);
> > +							 &user_mac);
> >  				if (ret == -1)
> >  					goto leave;
> >  			}
> > @@ -1651,7 +1679,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
> >  	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
> >  		name, tap_name);
> >
> > -	ret = eth_dev_tap_create(dev, tap_name, remote_iface,
> > fixed_mac_type);
> > +	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
> >
> >  leave:
> >  	if (ret == -1) {
> > @@ -1716,5 +1744,5 @@ RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
> > RTE_PMD_REGISTER_PARAM_STRING(net_tap,
> >  			      ETH_TAP_IFACE_ARG "=<string> "
> >  			      ETH_TAP_SPEED_ARG "=<int> "
> > -			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
> > +			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT
> >  			      ETH_TAP_REMOTE_ARG "=<string>");
> > --
> > 2.11.0

With these changes it is reviewed from my end.

Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Ferruh Yigit]

On 2/5/2018 3:54 PM, Pascal Mazon wrote:
> From: Vipin Varghese <vipin.varghese@intel.com>
> 
> Allow TAP PMD to pass user desired MAC address as argument.
> The argument value is processed as string, where each 2 bytes
> are converted to HEX MAC address after validation.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>

<...>

 @@ -278,15 +282,8 @@ tap_rx_offload_get_queue_capa(void)
>  static bool
>  tap_rxq_are_offloads_valid(struct rte_eth_dev *dev, uint64_t offloads)
>  {
> -	uint64_t port_offloads = dev->data->dev_conf.rxmode.offloads;
> -	uint64_t queue_supp_offloads = tap_rx_offload_get_queue_capa();
> -	uint64_t port_supp_offloads = tap_rx_offload_get_port_capa();
> -
> -	if ((offloads & (queue_supp_offloads | port_supp_offloads)) !=
> -	    offloads)
> -		return false;
> -	if ((port_offloads ^ offloads) & port_supp_offloads)
> -		return false;
> +	(void)dev;
> +	(void)offloads;

This part seems unrelated with the patch?

[dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args

[Vipin Varghese]

Allow TAP PMD to pass user desired MAC address as argument.
The argument value is processed as string delimited by  ':',
is parsed and converted to HEX MAC address after validation.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---

Changes:
 - seggrated the function for hex sting validation - Ferruh
 - Fixed the logic lookup for MAC address
---
 drivers/net/tap/rte_eth_tap.c | 75 +++++++++++++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 17 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 9d39384..5f67d51 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -48,6 +48,10 @@
 #define ETH_TAP_MAC_ARG         "mac"
 #define ETH_TAP_MAC_FIXED       "fixed"
 
+#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
+#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
+#define ETH_TAP_MAC_ARG_FMT     ETH_TAP_MAC_FIXED "|" ETH_TAP_USR_MAC_FMT
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -1335,7 +1339,7 @@ enum ioctl_mode {
 
 static int
 eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
-		   char *remote_iface, int fixed_mac_type)
+		   char *remote_iface, struct ether_addr *mac_addr)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev;
@@ -1397,16 +1401,10 @@ enum ioctl_mode {
 		pmd->txq[i].fd = -1;
 	}
 
-	if (fixed_mac_type) {
-		/* fixed mac = 00:64:74:61:70:<iface_idx> */
-		static int iface_idx;
-		char mac[ETHER_ADDR_LEN] = "\0dtap";
-
-		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
-		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
-	} else {
+	if (is_zero_ether_addr(mac_addr))
 		eth_random_addr((uint8_t *)&pmd->eth_addr);
-	}
+	else
+		rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(mac_addr));
 
 	/* Immediately create the netdevice (this will create the 1st queue). */
 	/* rx queue */
@@ -1567,15 +1565,58 @@ enum ioctl_mode {
 	return 0;
 }
 
+static int parse_user_mac(struct ether_addr *user_mac,
+		const char *value)
+{
+	unsigned int index = 0;
+	char mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1], *mac_byte = NULL;
+
+	if (user_mac == NULL || value == NULL)
+		return 0;
+
+	snprintf(mac_temp, sizeof(mac_temp), "%s", value);
+	mac_byte = strtok(mac_temp, ":");
+
+	while ((mac_byte != NULL) &&
+			(strlen(mac_byte) <= 2) &&
+			(strlen(mac_byte) == strspn(mac_byte,
+					ETH_TAP_CMP_MAC_FMT))) {
+		user_mac->addr_bytes[index++] = strtoul(mac_byte, NULL, 16);
+		mac_byte = strtok(NULL, ":");
+	}
+
+	return index;
+}
+
 static int
 set_mac_type(const char *key __rte_unused,
 	     const char *value,
 	     void *extra_args)
 {
-	if (value &&
-	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
-		*(int *)extra_args = 1;
+	struct ether_addr *user_mac = extra_args;
+
+	if (!value)
+		return 0;
+
+	if (!strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED))) {
+		static int iface_idx;
+
+		/* fixed mac = 00:64:74:61:70:<iface_idx> */
+		memcpy((char *)user_mac->addr_bytes, "\0dtap", ETHER_ADDR_LEN);
+		user_mac->addr_bytes[ETHER_ADDR_LEN - 1] = iface_idx++ + '0';
+		goto success;
+	}
+
+	if (parse_user_mac(user_mac, value) != 6)
+		goto error;
+success:
+	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
+		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
+	return -1;
 }
 
 /* Open a TAP interface device.
@@ -1589,7 +1630,7 @@ enum ioctl_mode {
 	int speed;
 	char tap_name[RTE_ETH_NAME_MAX_LEN];
 	char remote_iface[RTE_ETH_NAME_MAX_LEN];
-	int fixed_mac_type = 0;
+	struct ether_addr user_mac;
 
 	name = rte_vdev_device_name(dev);
 	params = rte_vdev_device_args(dev);
@@ -1626,7 +1667,7 @@ enum ioctl_mode {
 				ret = rte_kvargs_process(kvlist,
 							 ETH_TAP_MAC_ARG,
 							 &set_mac_type,
-							 &fixed_mac_type);
+							 &user_mac);
 				if (ret == -1)
 					goto leave;
 			}
@@ -1637,7 +1678,7 @@ enum ioctl_mode {
 	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
 		name, tap_name);
 
-	ret = eth_dev_tap_create(dev, tap_name, remote_iface, fixed_mac_type);
+	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
 
 leave:
 	if (ret == -1) {
@@ -1701,5 +1742,5 @@ enum ioctl_mode {
 RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
 RTE_PMD_REGISTER_PARAM_STRING(net_tap,
 			      ETH_TAP_IFACE_ARG "=<string> "
-			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
+			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT " "
 			      ETH_TAP_REMOTE_ARG "=<string>");
-- 
1.9.1

Re: [dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args

[Pascal Mazon]

Acked-by: Pascal Mazon <pascal.mazon@6wind.com>


On 12/02/2018 15:44, Vipin Varghese wrote:
> Allow TAP PMD to pass user desired MAC address as argument.
> The argument value is processed as string delimited by  ':',
> is parsed and converted to HEX MAC address after validation.
>
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
>
> Changes:
>  - seggrated the function for hex sting validation - Ferruh
>  - Fixed the logic lookup for MAC address
> ---
>  drivers/net/tap/rte_eth_tap.c | 75 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 58 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 9d39384..5f67d51 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -48,6 +48,10 @@
>  #define ETH_TAP_MAC_ARG         "mac"
>  #define ETH_TAP_MAC_FIXED       "fixed"
>  
> +#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
> +#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
> +#define ETH_TAP_MAC_ARG_FMT     ETH_TAP_MAC_FIXED "|" ETH_TAP_USR_MAC_FMT
> +
>  static struct rte_vdev_driver pmd_tap_drv;
>  
>  static const char *valid_arguments[] = {
> @@ -1335,7 +1339,7 @@ enum ioctl_mode {
>  
>  static int
>  eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> -		   char *remote_iface, int fixed_mac_type)
> +		   char *remote_iface, struct ether_addr *mac_addr)
>  {
>  	int numa_node = rte_socket_id();
>  	struct rte_eth_dev *dev;
> @@ -1397,16 +1401,10 @@ enum ioctl_mode {
>  		pmd->txq[i].fd = -1;
>  	}
>  
> -	if (fixed_mac_type) {
> -		/* fixed mac = 00:64:74:61:70:<iface_idx> */
> -		static int iface_idx;
> -		char mac[ETHER_ADDR_LEN] = "\0dtap";
> -
> -		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
> -		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
> -	} else {
> +	if (is_zero_ether_addr(mac_addr))
>  		eth_random_addr((uint8_t *)&pmd->eth_addr);
> -	}
> +	else
> +		rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(mac_addr));
>  
>  	/* Immediately create the netdevice (this will create the 1st queue). */
>  	/* rx queue */
> @@ -1567,15 +1565,58 @@ enum ioctl_mode {
>  	return 0;
>  }
>  
> +static int parse_user_mac(struct ether_addr *user_mac,
> +		const char *value)
> +{
> +	unsigned int index = 0;
> +	char mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1], *mac_byte = NULL;
> +
> +	if (user_mac == NULL || value == NULL)
> +		return 0;
> +
> +	snprintf(mac_temp, sizeof(mac_temp), "%s", value);
> +	mac_byte = strtok(mac_temp, ":");
> +
> +	while ((mac_byte != NULL) &&
> +			(strlen(mac_byte) <= 2) &&
> +			(strlen(mac_byte) == strspn(mac_byte,
> +					ETH_TAP_CMP_MAC_FMT))) {
> +		user_mac->addr_bytes[index++] = strtoul(mac_byte, NULL, 16);
> +		mac_byte = strtok(NULL, ":");
> +	}
> +
> +	return index;
> +}
> +
>  static int
>  set_mac_type(const char *key __rte_unused,
>  	     const char *value,
>  	     void *extra_args)
>  {
> -	if (value &&
> -	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
> -		*(int *)extra_args = 1;
> +	struct ether_addr *user_mac = extra_args;
> +
> +	if (!value)
> +		return 0;
> +
> +	if (!strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED))) {
> +		static int iface_idx;
> +
> +		/* fixed mac = 00:64:74:61:70:<iface_idx> */
> +		memcpy((char *)user_mac->addr_bytes, "\0dtap", ETHER_ADDR_LEN);
> +		user_mac->addr_bytes[ETHER_ADDR_LEN - 1] = iface_idx++ + '0';
> +		goto success;
> +	}
> +
> +	if (parse_user_mac(user_mac, value) != 6)
> +		goto error;
> +success:
> +	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
>  	return 0;
> +
> +error:
> +	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
> +		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
> +	return -1;
>  }
>  
>  /* Open a TAP interface device.
> @@ -1589,7 +1630,7 @@ enum ioctl_mode {
>  	int speed;
>  	char tap_name[RTE_ETH_NAME_MAX_LEN];
>  	char remote_iface[RTE_ETH_NAME_MAX_LEN];
> -	int fixed_mac_type = 0;
> +	struct ether_addr user_mac;
>  
>  	name = rte_vdev_device_name(dev);
>  	params = rte_vdev_device_args(dev);
> @@ -1626,7 +1667,7 @@ enum ioctl_mode {
>  				ret = rte_kvargs_process(kvlist,
>  							 ETH_TAP_MAC_ARG,
>  							 &set_mac_type,
> -							 &fixed_mac_type);
> +							 &user_mac);
>  				if (ret == -1)
>  					goto leave;
>  			}
> @@ -1637,7 +1678,7 @@ enum ioctl_mode {
>  	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
>  		name, tap_name);
>  
> -	ret = eth_dev_tap_create(dev, tap_name, remote_iface, fixed_mac_type);
> +	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
>  
>  leave:
>  	if (ret == -1) {
> @@ -1701,5 +1742,5 @@ enum ioctl_mode {
>  RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
>  RTE_PMD_REGISTER_PARAM_STRING(net_tap,
>  			      ETH_TAP_IFACE_ARG "=<string> "
> -			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
> +			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT " "
>  			      ETH_TAP_REMOTE_ARG "=<string>");

Re: [dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args

[Ferruh Yigit]

On 2/12/2018 2:44 PM, Vipin Varghese wrote:
> Allow TAP PMD to pass user desired MAC address as argument.
> The argument value is processed as string delimited by  ':',
> is parsed and converted to HEX MAC address after validation.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>

<...>

> @@ -1589,7 +1630,7 @@ enum ioctl_mode {
>  	int speed;
>  	char tap_name[RTE_ETH_NAME_MAX_LEN];
>  	char remote_iface[RTE_ETH_NAME_MAX_LEN];
> -	int fixed_mac_type = 0;
> +	struct ether_addr user_mac;
>  
>  	name = rte_vdev_device_name(dev);
>  	params = rte_vdev_device_args(dev);
> @@ -1626,7 +1667,7 @@ enum ioctl_mode {
>  				ret = rte_kvargs_process(kvlist,
>  							 ETH_TAP_MAC_ARG,
>  							 &set_mac_type,
> -							 &fixed_mac_type);
> +							 &user_mac);
>  				if (ret == -1)
>  					goto leave;
>  			}
> @@ -1637,7 +1678,7 @@ enum ioctl_mode {
>  	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
>  		name, tap_name);
>  
> -	ret = eth_dev_tap_create(dev, tap_name, remote_iface, fixed_mac_type);
> +	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);

"user_mac" without initial value is leading error when no "mac" argument is
provided. It should be zeroed out.

Re: [dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args

[Varghese, Vipin]

Hi Ferruh,

You are correct about this, I will add initialization send a next version patch.

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, March 6, 2018 4:42 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org;
> pascal.mazon@6wind.com
> Cc: Jain, Deepak K <deepak.k.jain@intel.com>
> Subject: Re: [PATCH v1] net/tap: allow user MAC to be passed as args
> 
> On 2/12/2018 2:44 PM, Vipin Varghese wrote:
> > Allow TAP PMD to pass user desired MAC address as argument.
> > The argument value is processed as string delimited by  ':', is parsed
> > and converted to HEX MAC address after validation.
> >
> > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> 
> <...>
> 
> > @@ -1589,7 +1630,7 @@ enum ioctl_mode {
> >  	int speed;
> >  	char tap_name[RTE_ETH_NAME_MAX_LEN];
> >  	char remote_iface[RTE_ETH_NAME_MAX_LEN];
> > -	int fixed_mac_type = 0;
> > +	struct ether_addr user_mac;
> >
> >  	name = rte_vdev_device_name(dev);
> >  	params = rte_vdev_device_args(dev);
> > @@ -1626,7 +1667,7 @@ enum ioctl_mode {
> >  				ret = rte_kvargs_process(kvlist,
> >  							 ETH_TAP_MAC_ARG,
> >  							 &set_mac_type,
> > -							 &fixed_mac_type);
> > +							 &user_mac);
> >  				if (ret == -1)
> >  					goto leave;
> >  			}
> > @@ -1637,7 +1678,7 @@ enum ioctl_mode {
> >  	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
> >  		name, tap_name);
> >
> > -	ret = eth_dev_tap_create(dev, tap_name, remote_iface,
> fixed_mac_type);
> > +	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
> 
> "user_mac" without initial value is leading error when no "mac" argument is
> provided. It should be zeroed out.

[dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Vipin Varghese]

Allow TAP PMD to pass user desired MAC address as argument.
The argument value is processed as string delimited by  ':',
is parsed and converted to HEX MAC address after validation.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Acked-by: Pascal Mazon <pascal.mazon@6wind.com>
---

Changes in V2:
 - updated the auto variable for user_mac - Ferruh
 - added documentation update - Vipin
---
 doc/guides/nics/tap.rst       |  6 ++++
 drivers/net/tap/rte_eth_tap.c | 75 +++++++++++++++++++++++++++++++++----------
 2 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index ea61be3..76eb0bd 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -37,6 +37,12 @@ for each interface string containing ``mac=fixed``. The MAC address is formatted
 as 00:'d':'t':'a':'p':[00-FF]. Convert the characters to hex and you get the
 actual MAC address: ``00:64:74:61:70:[00-FF]``.
 
+   --vdev=net_tap0,mac="00:64:74:61:70:11"
+
+The MAC address will have a user value passed as string. The MAC address is in
+format with delimeter ``:``. The string is byte converted to hex and you get
+the actual MAC address: ``00:64:74:61:70:11``.
+
 It is possible to specify a remote netdevice to capture packets from by adding
 ``remote=foo1``, for example::
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index f09db0e..6a20802 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -48,6 +48,10 @@
 #define ETH_TAP_MAC_ARG         "mac"
 #define ETH_TAP_MAC_FIXED       "fixed"
 
+#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
+#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
+#define ETH_TAP_MAC_ARG_FMT     ETH_TAP_MAC_FIXED "|" ETH_TAP_USR_MAC_FMT
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -1337,7 +1341,7 @@ enum ioctl_mode {
 
 static int
 eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
-		   char *remote_iface, int fixed_mac_type)
+		   char *remote_iface, struct ether_addr *mac_addr)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev;
@@ -1399,16 +1403,10 @@ enum ioctl_mode {
 		pmd->txq[i].fd = -1;
 	}
 
-	if (fixed_mac_type) {
-		/* fixed mac = 00:64:74:61:70:<iface_idx> */
-		static int iface_idx;
-		char mac[ETHER_ADDR_LEN] = "\0dtap";
-
-		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
-		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
-	} else {
+	if (is_zero_ether_addr(mac_addr))
 		eth_random_addr((uint8_t *)&pmd->eth_addr);
-	}
+	else
+		rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(mac_addr));
 
 	/* Immediately create the netdevice (this will create the 1st queue). */
 	/* rx queue */
@@ -1569,15 +1567,58 @@ enum ioctl_mode {
 	return 0;
 }
 
+static int parse_user_mac(struct ether_addr *user_mac,
+		const char *value)
+{
+	unsigned int index = 0;
+	char mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1], *mac_byte = NULL;
+
+	if (user_mac == NULL || value == NULL)
+		return 0;
+
+	snprintf(mac_temp, sizeof(mac_temp), "%s", value);
+	mac_byte = strtok(mac_temp, ":");
+
+	while ((mac_byte != NULL) &&
+			(strlen(mac_byte) <= 2) &&
+			(strlen(mac_byte) == strspn(mac_byte,
+					ETH_TAP_CMP_MAC_FMT))) {
+		user_mac->addr_bytes[index++] = strtoul(mac_byte, NULL, 16);
+		mac_byte = strtok(NULL, ":");
+	}
+
+	return index;
+}
+
 static int
 set_mac_type(const char *key __rte_unused,
 	     const char *value,
 	     void *extra_args)
 {
-	if (value &&
-	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
-		*(int *)extra_args = 1;
+	struct ether_addr *user_mac = extra_args;
+
+	if (!value)
+		return 0;
+
+	if (!strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED))) {
+		static int iface_idx;
+
+		/* fixed mac = 00:64:74:61:70:<iface_idx> */
+		memcpy((char *)user_mac->addr_bytes, "\0dtap", ETHER_ADDR_LEN);
+		user_mac->addr_bytes[ETHER_ADDR_LEN - 1] = iface_idx++ + '0';
+		goto success;
+	}
+
+	if (parse_user_mac(user_mac, value) != 6)
+		goto error;
+success:
+	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
+		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
+	return -1;
 }
 
 /* Open a TAP interface device.
@@ -1591,7 +1632,7 @@ enum ioctl_mode {
 	int speed;
 	char tap_name[RTE_ETH_NAME_MAX_LEN];
 	char remote_iface[RTE_ETH_NAME_MAX_LEN];
-	int fixed_mac_type = 0;
+	struct ether_addr user_mac = { .addr_bytes = {0, 0, 0, 0, 0, 0} };
 
 	name = rte_vdev_device_name(dev);
 	params = rte_vdev_device_args(dev);
@@ -1628,7 +1669,7 @@ enum ioctl_mode {
 				ret = rte_kvargs_process(kvlist,
 							 ETH_TAP_MAC_ARG,
 							 &set_mac_type,
-							 &fixed_mac_type);
+							 &user_mac);
 				if (ret == -1)
 					goto leave;
 			}
@@ -1639,7 +1680,7 @@ enum ioctl_mode {
 	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
 		name, tap_name);
 
-	ret = eth_dev_tap_create(dev, tap_name, remote_iface, fixed_mac_type);
+	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
 
 leave:
 	if (ret == -1) {
@@ -1703,5 +1744,5 @@ enum ioctl_mode {
 RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
 RTE_PMD_REGISTER_PARAM_STRING(net_tap,
 			      ETH_TAP_IFACE_ARG "=<string> "
-			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
+			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT " "
 			      ETH_TAP_REMOTE_ARG "=<string>");
-- 
1.9.1

Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Ferruh Yigit]

On 3/12/2018 8:21 PM, Vipin Varghese wrote:
> @@ -1591,7 +1632,7 @@ enum ioctl_mode {
>  	int speed;
>  	char tap_name[RTE_ETH_NAME_MAX_LEN];
>  	char remote_iface[RTE_ETH_NAME_MAX_LEN];
> -	int fixed_mac_type = 0;
> +	struct ether_addr user_mac = { .addr_bytes = {0, 0, 0, 0, 0, 0} };

A single '0' will do the job J
struct ether_addr user_mac = { .addr_bytes = {0} };

Would you mind sending a new version?

Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Varghese, Vipin]

Sure Ferruh, I will spin a new one.

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 12, 2018 8:39 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org;
> pascal.mazon@6wind.com
> Subject: Re: [PATCH v2] net/tap: allow user MAC to be passed as args
> 
> On 3/12/2018 8:21 PM, Vipin Varghese wrote:
> > @@ -1591,7 +1632,7 @@ enum ioctl_mode {
> >  	int speed;
> >  	char tap_name[RTE_ETH_NAME_MAX_LEN];
> >  	char remote_iface[RTE_ETH_NAME_MAX_LEN];
> > -	int fixed_mac_type = 0;
> > +	struct ether_addr user_mac = { .addr_bytes = {0, 0, 0, 0, 0, 0} };
> 
> A single '0' will do the job J
> struct ether_addr user_mac = { .addr_bytes = {0} };
> 
> Would you mind sending a new version?

Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args

[Stephen Hemminger]

On Tue, 13 Mar 2018 01:51:34 +0530
Vipin Varghese <vipin.varghese@intel.com> wrote:

> +static int parse_user_mac(struct ether_addr *user_mac,
> +		const char *value)
> +{
> +	unsigned int index = 0;
> +	char mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1], *mac_byte = NULL;
> +
> +	if (user_mac == NULL || value == NULL)
> +		return 0;
> +
> +	snprintf(mac_temp, sizeof(mac_temp), "%s", value);
> +	mac_byte = strtok(mac_temp, ":");
> +
> +	while ((mac_byte != NULL) &&
> +			(strlen(mac_byte) <= 2) &&
> +			(strlen(mac_byte) == strspn(mac_byte,
> +					ETH_TAP_CMP_MAC_FMT))) {
> +		user_mac->addr_bytes[index++] = strtoul(mac_byte, NULL, 16);
> +		mac_byte = strtok(NULL, ":");
> +	}
> +
> +	return index;
> +}

We need rte_ether_aton or better yet replace the DPDK custom definition of 
"struct ether_addr" with the with the one from Linux/BSD <netinet/ether.h>
There is no value in having a private version of all these routines...

[dpdk-dev] [PATCH v3] net/tap: allow user MAC to be passed as args

[Vipin Varghese]

Allow TAP PMD to pass user desired MAC address as argument.
The argument value is processed as string delimited by  ':',
is parsed and converted to HEX MAC address after validation.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Acked-by: Pascal Mazon <pascal.mazon@6wind.com>
---

Changed in V3:
 - change the default init for user_mac - Ferruh

Changes in V2:
 - updated the auto variable for user_mac - Ferruh
 - added documentation update - Vipin
---
 doc/guides/nics/tap.rst       |  6 ++++
 drivers/net/tap/rte_eth_tap.c | 75 +++++++++++++++++++++++++++++++++----------
 2 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index ea61be3..76eb0bd 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -37,6 +37,12 @@ for each interface string containing ``mac=fixed``. The MAC address is formatted
 as 00:'d':'t':'a':'p':[00-FF]. Convert the characters to hex and you get the
 actual MAC address: ``00:64:74:61:70:[00-FF]``.
 
+   --vdev=net_tap0,mac="00:64:74:61:70:11"
+
+The MAC address will have a user value passed as string. The MAC address is in
+format with delimeter ``:``. The string is byte converted to hex and you get
+the actual MAC address: ``00:64:74:61:70:11``.
+
 It is possible to specify a remote netdevice to capture packets from by adding
 ``remote=foo1``, for example::
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index f09db0e..3e4f7a8 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -48,6 +48,10 @@
 #define ETH_TAP_MAC_ARG         "mac"
 #define ETH_TAP_MAC_FIXED       "fixed"
 
+#define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
+#define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
+#define ETH_TAP_MAC_ARG_FMT     ETH_TAP_MAC_FIXED "|" ETH_TAP_USR_MAC_FMT
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -1337,7 +1341,7 @@ enum ioctl_mode {
 
 static int
 eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
-		   char *remote_iface, int fixed_mac_type)
+		   char *remote_iface, struct ether_addr *mac_addr)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev;
@@ -1399,16 +1403,10 @@ enum ioctl_mode {
 		pmd->txq[i].fd = -1;
 	}
 
-	if (fixed_mac_type) {
-		/* fixed mac = 00:64:74:61:70:<iface_idx> */
-		static int iface_idx;
-		char mac[ETHER_ADDR_LEN] = "\0dtap";
-
-		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
-		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
-	} else {
+	if (is_zero_ether_addr(mac_addr))
 		eth_random_addr((uint8_t *)&pmd->eth_addr);
-	}
+	else
+		rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(mac_addr));
 
 	/* Immediately create the netdevice (this will create the 1st queue). */
 	/* rx queue */
@@ -1569,15 +1567,58 @@ enum ioctl_mode {
 	return 0;
 }
 
+static int parse_user_mac(struct ether_addr *user_mac,
+		const char *value)
+{
+	unsigned int index = 0;
+	char mac_temp[strlen(ETH_TAP_USR_MAC_FMT) + 1], *mac_byte = NULL;
+
+	if (user_mac == NULL || value == NULL)
+		return 0;
+
+	snprintf(mac_temp, sizeof(mac_temp), "%s", value);
+	mac_byte = strtok(mac_temp, ":");
+
+	while ((mac_byte != NULL) &&
+			(strlen(mac_byte) <= 2) &&
+			(strlen(mac_byte) == strspn(mac_byte,
+					ETH_TAP_CMP_MAC_FMT))) {
+		user_mac->addr_bytes[index++] = strtoul(mac_byte, NULL, 16);
+		mac_byte = strtok(NULL, ":");
+	}
+
+	return index;
+}
+
 static int
 set_mac_type(const char *key __rte_unused,
 	     const char *value,
 	     void *extra_args)
 {
-	if (value &&
-	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
-		*(int *)extra_args = 1;
+	struct ether_addr *user_mac = extra_args;
+
+	if (!value)
+		return 0;
+
+	if (!strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED))) {
+		static int iface_idx;
+
+		/* fixed mac = 00:64:74:61:70:<iface_idx> */
+		memcpy((char *)user_mac->addr_bytes, "\0dtap", ETHER_ADDR_LEN);
+		user_mac->addr_bytes[ETHER_ADDR_LEN - 1] = iface_idx++ + '0';
+		goto success;
+	}
+
+	if (parse_user_mac(user_mac, value) != 6)
+		goto error;
+success:
+	RTE_LOG(DEBUG, PMD, "TAP user MAC param (%s)\n", value);
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s|%s)\n",
+		value, ETH_TAP_MAC_FIXED, ETH_TAP_USR_MAC_FMT);
+	return -1;
 }
 
 /* Open a TAP interface device.
@@ -1591,7 +1632,7 @@ enum ioctl_mode {
 	int speed;
 	char tap_name[RTE_ETH_NAME_MAX_LEN];
 	char remote_iface[RTE_ETH_NAME_MAX_LEN];
-	int fixed_mac_type = 0;
+	struct ether_addr user_mac = { .addr_bytes = {0} };
 
 	name = rte_vdev_device_name(dev);
 	params = rte_vdev_device_args(dev);
@@ -1628,7 +1669,7 @@ enum ioctl_mode {
 				ret = rte_kvargs_process(kvlist,
 							 ETH_TAP_MAC_ARG,
 							 &set_mac_type,
-							 &fixed_mac_type);
+							 &user_mac);
 				if (ret == -1)
 					goto leave;
 			}
@@ -1639,7 +1680,7 @@ enum ioctl_mode {
 	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
 		name, tap_name);
 
-	ret = eth_dev_tap_create(dev, tap_name, remote_iface, fixed_mac_type);
+	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac);
 
 leave:
 	if (ret == -1) {
@@ -1703,5 +1744,5 @@ enum ioctl_mode {
 RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
 RTE_PMD_REGISTER_PARAM_STRING(net_tap,
 			      ETH_TAP_IFACE_ARG "=<string> "
-			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
+			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT " "
 			      ETH_TAP_REMOTE_ARG "=<string>");
-- 
1.9.1

Re: [dpdk-dev] [PATCH v3] net/tap: allow user MAC to be passed as args

[Ferruh Yigit]

On 3/12/2018 9:53 PM, Vipin Varghese wrote:
> Allow TAP PMD to pass user desired MAC address as argument.
> The argument value is processed as string delimited by  ':',
> is parsed and converted to HEX MAC address after validation.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Acked-by: Pascal Mazon <pascal.mazon@6wind.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v3] net/tap: allow user MAC to be passed as args

[Ferruh Yigit]

On 3/12/2018 5:45 PM, Ferruh Yigit wrote:
> On 3/12/2018 9:53 PM, Vipin Varghese wrote:
>> Allow TAP PMD to pass user desired MAC address as argument.
>> The argument value is processed as string delimited by  ':',
>> is parsed and converted to HEX MAC address after validation.
>>
>> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
>> Acked-by: Pascal Mazon <pascal.mazon@6wind.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH v1] net/tap: allow user mac to be passed as args

[Vipin Varghese]

One of the uses cases from customer site is use TAP PMD to intake
user specific MAC address during probe. This allows applications
make use of interfaces with desired MAC.

Extending MAC argumentinfrastructure for tap PMD; we pass custom
MAC address in string format (sample - 11:22:33:44:55:66).

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

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 6b27679..0c53458 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -81,6 +81,8 @@
 #define FLOWER_KERNEL_VERSION KERNEL_VERSION(4, 2, 0)
 #define FLOWER_VLAN_KERNEL_VERSION KERNEL_VERSION(4, 9, 0)
 
+static unsigned char user_mac[ETHER_ADDR_LEN];
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -1291,13 +1293,20 @@ enum ioctl_mode {
 		pmd->txq[i].fd = -1;
 	}
 
-	if (fixed_mac_type) {
+	if (fixed_mac_type == 1) {
 		/* fixed mac = 00:64:74:61:70:<iface_idx> */
 		static int iface_idx;
 		char mac[ETHER_ADDR_LEN] = "\0dtap";
 
 		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
 		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
+	} else if (fixed_mac_type == 2) {
+		/* user mac is recieved */
+		RTE_LOG(INFO, PMD,
+			"Using user MAC (%02x:%02x:%02x:%02x:%02x:%02x)\n",
+			user_mac[0], user_mac[1], user_mac[2],
+			user_mac[3], user_mac[4], user_mac[5]);
+		rte_memcpy(&pmd->eth_addr, user_mac, ETHER_ADDR_LEN);
 	} else {
 		eth_random_addr((uint8_t *)&pmd->eth_addr);
 	}
@@ -1471,9 +1480,48 @@ enum ioctl_mode {
 	     const char *value,
 	     void *extra_args)
 {
-	if (value &&
-	    !strncasecmp(ETH_TAP_MAC_FIXED, value, strlen(ETH_TAP_MAC_FIXED)))
-		*(int *)extra_args = 1;
+	char macTemp[20], *macByte = NULL;
+	unsigned int index = 0;
+
+	if (value) {
+		if (!strncasecmp(ETH_TAP_MAC_FIXED, value,
+			strlen(ETH_TAP_MAC_FIXED))) {
+			*(int *)extra_args = 1;
+		} else {
+			RTE_LOG(INFO, PMD, "User shared MAC param (%s)\n",
+				value);
+
+			/* desired format aa:bb:cc:dd:ee:ff:11 */
+			if (strlen(value) == 17) {
+				strncpy(macTemp, value, 18);
+
+				macByte = strtok(macTemp, ":");
+				while ((macByte != NULL) &&
+					(strspn(macByte, "0123456789ABCDEFabcdef")) &&
+					(strspn((macByte + 1), "0123456789ABCDEFabcdef")) &&
+					(strlen(macByte) == 2)) {
+					user_mac[index] = strtoul(macByte, NULL, 16);
+					macByte = strtok(NULL, ":");
+					index += 1;
+				}
+
+				if (index != 6) {
+					RTE_LOG(ERR, PMD, " failure in MAC value (%s) at %u\n",
+						macByte, index + 1);
+					return -1;
+				}
+
+				RTE_LOG(DEBUG, PMD, " User MAC (%s) considered\n",
+					value);
+				*(int *)extra_args = 2;
+			} else {
+				RTE_LOG(ERR, PMD, " Mismatch on format for (%s)\n",
+					value);
+				return -1;
+			}
+		}
+	}
+
 	return 0;
 }
 
-- 
1.9.1

[dpdk-dev] [PATCH v2] net/tap: allow user mac to be passed as args

[Vipin Varghese]

Added fixes for user TAP user MAC
1) user format to RTE_PMD_REGISTER_PARAM_STRING
2) TAP to the RTE_LOG in absence of dynamic RTE_LOG
3) Boundary case for MAC string added

---------------------------------------------------------------

Other Details:
1) not to extract "string to mac" conversion to its own function
2) set_mac_type does not take any pmd or device name
3) Segault for boundary cases 'mac="01:01:01:01:01:01', not found
4) Added user MAC format string

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

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 0c53458..85c12af 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -81,6 +81,11 @@
 #define FLOWER_KERNEL_VERSION KERNEL_VERSION(4, 2, 0)
 #define FLOWER_VLAN_KERNEL_VERSION KERNEL_VERSION(4, 9, 0)
 
+#define MAC_STRING_NULL (0)
+#define MAC_STRING_FIXED (1)
+#define MAC_STRING_USER (2)
+#define ETH_TAP_USER_MAC_FMT ("xx:xx:xx:xx:xx:xx")
+
 static unsigned char user_mac[ETHER_ADDR_LEN];
 
 static struct rte_vdev_driver pmd_tap_drv;
@@ -1293,17 +1298,18 @@ enum ioctl_mode {
 		pmd->txq[i].fd = -1;
 	}
 
-	if (fixed_mac_type == 1) {
+	if (fixed_mac_type == MAC_STRING_FIXED) {
 		/* fixed mac = 00:64:74:61:70:<iface_idx> */
 		static int iface_idx;
 		char mac[ETHER_ADDR_LEN] = "\0dtap";
 
 		mac[ETHER_ADDR_LEN - 1] = iface_idx++;
 		rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
-	} else if (fixed_mac_type == 2) {
-		/* user mac is recieved */
+	} else if (fixed_mac_type == MAC_STRING_USER) {
+		/* user mac is received */
 		RTE_LOG(INFO, PMD,
-			"Using user MAC (%02x:%02x:%02x:%02x:%02x:%02x)\n",
+			"%s; Using user MAC (%02x:%02x:%02x:%02x:%02x:%02x) argument\n",
+			pmd->name,
 			user_mac[0], user_mac[1], user_mac[2],
 			user_mac[3], user_mac[4], user_mac[5]);
 		rte_memcpy(&pmd->eth_addr, user_mac, ETHER_ADDR_LEN);
@@ -1476,53 +1482,49 @@ enum ioctl_mode {
 }
 
 static int
-set_mac_type(const char *key __rte_unused,
-	     const char *value,
-	     void *extra_args)
+set_mac_type(const char *key __rte_unused, const char *value, void *extra_args)
 {
-	char macTemp[20], *macByte = NULL;
+	char mac_temp[20] = {0}, *mac_byte = NULL;
 	unsigned int index = 0;
 
 	if (value) {
+		RTE_LOG(DEBUG, PMD, "TAP user MAC (%s) to set.\n", value);
+
 		if (!strncasecmp(ETH_TAP_MAC_FIXED, value,
 			strlen(ETH_TAP_MAC_FIXED))) {
-			*(int *)extra_args = 1;
+			*(int *)extra_args = MAC_STRING_FIXED;
 		} else {
-			RTE_LOG(INFO, PMD, "User shared MAC param (%s)\n",
-				value);
-
 			/* desired format aa:bb:cc:dd:ee:ff:11 */
 			if (strlen(value) == 17) {
-				strncpy(macTemp, value, 18);
-
-				macByte = strtok(macTemp, ":");
-				while ((macByte != NULL) &&
-					(strspn(macByte, "0123456789ABCDEFabcdef")) &&
-					(strspn((macByte + 1), "0123456789ABCDEFabcdef")) &&
-					(strlen(macByte) == 2)) {
-					user_mac[index] = strtoul(macByte, NULL, 16);
-					macByte = strtok(NULL, ":");
+				strncpy(mac_temp, value, 18);
+				mac_temp[19] = "\0";
+				mac_byte = strtok(mac_temp, ":");
+
+				while ((mac_byte != NULL) &&
+					(strspn(mac_byte, "0123456789ABCDEFabcdef")) &&
+					(strspn((mac_byte + 1), "0123456789ABCDEFabcdef")) &&
+					(strlen(mac_byte) == 2)) {
+					user_mac[index] = strtoul(mac_byte, NULL, 16);
+					mac_byte = strtok(NULL, ":");
 					index += 1;
 				}
 
-				if (index != 6) {
-					RTE_LOG(ERR, PMD, " failure in MAC value (%s) at %u\n",
-						macByte, index + 1);
-					return -1;
-				}
+				if (index != 6)
+					goto error;
 
-				RTE_LOG(DEBUG, PMD, " User MAC (%s) considered\n",
-					value);
-				*(int *)extra_args = 2;
-			} else {
-				RTE_LOG(ERR, PMD, " Mismatch on format for (%s)\n",
-					value);
-				return -1;
-			}
+				*(int *)extra_args = MAC_STRING_USER;
+			} else
+				goto error;
 		}
+		RTE_LOG(DEBUG, PMD, "TAP user MAC (%s) considered\n", value);
 	}
 
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s)\n",
+		value, ETH_TAP_USER_MAC_FMT);
+	return -1;
 }
 
 /* Open a TAP interface device.
@@ -1536,7 +1538,7 @@ enum ioctl_mode {
 	int speed;
 	char tap_name[RTE_ETH_NAME_MAX_LEN];
 	char remote_iface[RTE_ETH_NAME_MAX_LEN];
-	int fixed_mac_type = 0;
+	int fixed_mac_type = MAC_STRING_NULL;
 
 	name = rte_vdev_device_name(dev);
 	params = rte_vdev_device_args(dev);
@@ -1656,7 +1658,7 @@ enum ioctl_mode {
 RTE_PMD_REGISTER_VDEV(net_tap, pmd_tap_drv);
 RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
 RTE_PMD_REGISTER_PARAM_STRING(net_tap,
-			      ETH_TAP_IFACE_ARG "=<string> "
-			      ETH_TAP_SPEED_ARG "=<int> "
-			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
-			      ETH_TAP_REMOTE_ARG "=<string>");
+	ETH_TAP_IFACE_ARG "=<string> "
+	ETH_TAP_SPEED_ARG "=<int> "
+	ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
+	ETH_TAP_REMOTE_ARG "=<string>");
-- 
1.9.1

Re: [dpdk-dev] [PATCH v2] net/tap: allow user mac to be passed as args

[Ferruh Yigit]

On 12/21/2017 4:01 PM, Vipin Varghese wrote:
> Added fixes for user TAP user MAC
> 1) user format to RTE_PMD_REGISTER_PARAM_STRING
> 2) TAP to the RTE_LOG in absence of dynamic RTE_LOG
> 3) Boundary case for MAC string added
> > ---------------------------------------------------------------
> 
> Other Details:
> 1) not to extract "string to mac" conversion to its own function
> 2) set_mac_type does not take any pmd or device name
> 3) Segault for boundary cases 'mac="01:01:01:01:01:01', not found
> 4) Added user MAC format string

Hi Vipin,

Patch version changes shouldn't be part of git commit. But it is good to put
them after "---" in commit log. Can you please update the commit log?

Also please add maintainer to the cc/to .

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

<...>

> -				strncpy(macTemp, value, 18);
> -
> -				macByte = strtok(macTemp, ":");
> -				while ((macByte != NULL) &&
> -					(strspn(macByte, "0123456789ABCDEFabcdef")) &&
> -					(strspn((macByte + 1), "0123456789ABCDEFabcdef")) &&
> -					(strlen(macByte) == 2)) {
> -					user_mac[index] = strtoul(macByte, NULL, 16);
> -					macByte = strtok(NULL, ":");
> +				strncpy(mac_temp, value, 18);
> +				mac_temp[19] = "\0";

This cause a build error [1], should be used '\0':

[1]
...drivers/net/tap/rte_eth_tap.c: In function ‘set_mac_type’:
...drivers/net/tap/rte_eth_tap.c:1467:18: error: assignment makes integer from
pointer without a cast [-Werror=int-conversion]
     mac_temp[19] = "\0";
                  ^
<....>

> -			      ETH_TAP_IFACE_ARG "=<string> "
> -			      ETH_TAP_SPEED_ARG "=<int> "
> -			      ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
> -			      ETH_TAP_REMOTE_ARG "=<string>");
> +	ETH_TAP_IFACE_ARG "=<string> "
> +	ETH_TAP_SPEED_ARG "=<int> "
> +	ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
> +	ETH_TAP_REMOTE_ARG "=<string>");

This also build a build error [2] because of how ETH_TAP_USER_MAC_FMT defined [3].

[2]
.../drivers/net/tap/rte_eth_tap.c: At top level:
.../drivers/net/tap/rte_eth_tap.c:45:33: error: called object is not a function
or function pointer
 #define ETH_TAP_IFACE_ARG       "iface"
                                 ^
.../build/include/rte_dev.h:265:25: note: in definition of macro
‘RTE_PMD_REGISTER_PARAM_STRING’
 __attribute__((used)) = str
                         ^~~
.../drivers/net/tap/rte_eth_tap.c:1628:2: note: in expansion of macro
‘ETH_TAP_IFACE_ARG’
  ETH_TAP_IFACE_ARG "=<string> "
  ^~~~~~~~~~~~~~~~~
.../drivers/net/tap/rte_eth_tap.c:1630:65: error: expected ‘,’ or ‘;’ before
string constant
  ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
                                                                 ^
.../build/include/rte_dev.h:265:25: note: in definition of macro
‘RTE_PMD_REGISTER_PARAM_STRING’
 __attribute__((used)) = str
                         ^~~

[3]
#define ETH_TAP_USER_MAC_FMT ("xx:xx:xx:xx:xx:xx")
parenthesis ...