DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Vipin Varghese <vipin.varghese@intel.com>, dev@dpdk.org
Cc: david.hunt@intel.com, deepak.k.jain@intel.com
Subject: Re: [dpdk-dev] [PATCH v1] net/tap: allow user mac to be passed as args
Date: Thu, 7 Dec 2017 12:11:39 -0800	[thread overview]
Message-ID: <bda3f6d6-7799-a4b5-2385-526cc1803654@intel.com> (raw)
In-Reply-To: <1512071396-10653-1-git-send-email-vipin.varghese@intel.com>

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

  reply	other threads:[~2017-12-07 20:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-30 19:49 Vipin Varghese
2017-12-07 20:11 ` Ferruh Yigit [this message]
2017-12-08 10:14   ` Pascal Mazon
2017-12-16  2:21   ` Varghese, Vipin
2017-12-21 16:01 ` [dpdk-dev] [PATCH v2] " Vipin Varghese
2018-01-16 11:32   ` Ferruh Yigit
2018-02-05 15:54 [dpdk-dev] [PATCH v2] net/tap: allow user MAC " Pascal Mazon
2018-02-12 14:44 ` [dpdk-dev] [PATCH v1] " Vipin Varghese
2018-02-22 11:52   ` Pascal Mazon
2018-03-06 16:41   ` Ferruh Yigit
2018-03-07  9:44     ` Varghese, Vipin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bda3f6d6-7799-a4b5-2385-526cc1803654@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=david.hunt@intel.com \
    --cc=deepak.k.jain@intel.com \
    --cc=dev@dpdk.org \
    --cc=vipin.varghese@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).