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

[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

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

[Ferruh Yigit]

On 11/30/2017 11:49 AM, Vipin Varghese wrote:
> 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).

Overall lgtm, please check a few comments below.

> 
> 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) {

Instead of hardcoded type values 1 & 2, can you please use macros?

>  		/* 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 */

s/recieved/received

> +		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);

Is this log needs to be in INFO level, since there is already and info level log
when MAC set, what about making this debug?

> +
> +			/* desired format aa:bb:cc:dd:ee:ff:11 */

Can you please update RTE_PMD_REGISTER_PARAM_STRING with new param, put expected
format info there as well?

> +			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;
> +				}

I would extract the string to mac logic into its own function, but up to you.

> +
> +				if (index != 6) {
> +					RTE_LOG(ERR, PMD, " failure in MAC value (%s) at %u\n",
> +						macByte, index + 1);
> +					return -1;

And this is not related to this patch, but just as reminder, when a virtual
driver probe fails vdev bus stops probing with an error, so all remaining
virtual devices are not probed, in case one might want to fix ;)

> +				}
> +
> +				RTE_LOG(DEBUG, PMD, " User MAC (%s) considered\n",

"User defined MAC" ? And can you please add an port identifier, port_id or
device name or interface name, for the case there are multiple tap device to
identify which one get user defined MAC.

> +					value);
> +				*(int *)extra_args = 2;
> +			} else {
> +				RTE_LOG(ERR, PMD, " Mismatch on format for (%s)\n",
> +					value);
> +				return -1;
> +			}
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> 

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

[Pascal Mazon]

Hi,

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.

/* desired format aa:bb:cc:dd:ee:ff:11 */      The :11 goes beyond
standard MAC addresses ;-)

The commit log has few mistakes:
- missing "to" after "applications"
- "argumentinfrastructure"

I otherwise concur with Ferruh's remarks.

Best regards,
Pascal

On 07/12/2017 21:11, Ferruh Yigit wrote:
> On 11/30/2017 11:49 AM, Vipin Varghese wrote:
>> 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).
> Overall lgtm, please check a few comments below.
>
>> 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) {
> Instead of hardcoded type values 1 & 2, can you please use macros?
>
>>  		/* 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 */
> s/recieved/received
>
>> +		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);
> Is this log needs to be in INFO level, since there is already and info level log
> when MAC set, what about making this debug?
>
>> +
>> +			/* desired format aa:bb:cc:dd:ee:ff:11 */
> Can you please update RTE_PMD_REGISTER_PARAM_STRING with new param, put expected
> format info there as well?
>
>> +			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;
>> +				}
> I would extract the string to mac logic into its own function, but up to you.
>
>> +
>> +				if (index != 6) {
>> +					RTE_LOG(ERR, PMD, " failure in MAC value (%s) at %u\n",
>> +						macByte, index + 1);
>> +					return -1;
> And this is not related to this patch, but just as reminder, when a virtual
> driver probe fails vdev bus stops probing with an error, so all remaining
> virtual devices are not probed, in case one might want to fix ;)
>
>> +				}
>> +
>> +				RTE_LOG(DEBUG, PMD, " User MAC (%s) considered\n",
> "User defined MAC" ? And can you please add an port identifier, port_id or
> device name or interface name, for the case there are multiple tap device to
> identify which one get user defined MAC.
>
>> +					value);
>> +				*(int *)extra_args = 2;
>> +			} else {
>> +				RTE_LOG(ERR, PMD, " Mismatch on format for (%s)\n",
>> +					value);
>> +				return -1;
>> +			}
>> +		}
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>>

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

[Varghese, Vipin]

On 11/30/2017 11:49 AM, Vipin Varghese wrote:
> 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).

Overall lgtm, please check a few comments below.

> 
> 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) {

Instead of hardcoded type values 1 & 2, can you please use macros?
>> Ok, Adding MACROs MAC_STRING_NULL, MAC_STRING_FIXED and MAC_STRING_USER

>  		/* 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 */

s/recieved/received
>> Ok

> +		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);

Is this log needs to be in INFO level, since there is already and info level log when MAC set, what about making this debug?
>> Ok

> +
> +			/* desired format aa:bb:cc:dd:ee:ff:11 */

Can you please update RTE_PMD_REGISTER_PARAM_STRING with new param, put expected format info there as well?
>> Ok

> +			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;
> +				}

I would extract the string to mac logic into its own function, but up to you.
>> not done

> +
> +				if (index != 6) {
> +					RTE_LOG(ERR, PMD, " failure in MAC value (%s) at %u\n",
> +						macByte, index + 1);
> +					return -1;

And this is not related to this patch, but just as reminder, when a virtual driver probe fails vdev bus stops probing with an error, so all remaining virtual devices are not probed, in case one might want to fix ;)

> +				}
> +
> +				RTE_LOG(DEBUG, PMD, " User MAC (%s) considered\n",

"User defined MAC" ? And can you please add an port identifier, port_id or device name or interface name, for the case there are multiple tap device to identify which one get user defined MAC.
VV> not done, each device in DPDK rte_eal_init is sequentially scanned and probed, hence log starts with 'iface,[parameters]', then probe, create and init is called for TAP PMD one after another. 
With minimalistic logging style the current format is kept. 

> +					value);
> +				*(int *)extra_args = 2;
> +			} else {
> +				RTE_LOG(ERR, PMD, " Mismatch on format for (%s)\n",
> +					value);
> +				return -1;
> +			}
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> 

[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 ...

[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

[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.