From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id 110CC1E33 for ; Fri, 2 Feb 2018 10:16:31 +0100 (CET) Received: by mail-wm0-f68.google.com with SMTP id v123so10831590wmd.5 for ; Fri, 02 Feb 2018 01:16:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=16KgoofgvBEeHIg4VyoSgzy5Wv2tXAmaG8E50m07BNo=; b=cHX2oeZiMr/utTak3yIvztXHlyIbxjDhB+l77xu0lqvizE9JA7n3FvHrOWZ9VC/2br RglXQhvWT+ToOmsvlUS/OCc+EmaxPqNCOtCxLko/4kb+P8AKSjWj7TbhOM/YtP52ngER FPaJ8capS3YjYvvbflO08aR9kqAqz9M0Nx0ttMJtk//Otgqw6FhLLnbTj2XLJN3rro0L J3geyl274k4KNsk0c2dzZbQ1M1RGb2SMtU7bG8lUKw0ZDpSL0vIyZ/wUoWbc5WhxhEhl kknxnhtPNy1IewIBBgh5AMVj3qrIIhTJKQqiTgCAnsCiMOXh03iwLjpgYWQTOrE0YwPt xQbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=16KgoofgvBEeHIg4VyoSgzy5Wv2tXAmaG8E50m07BNo=; b=VX+4/0NQ8PKtQ8jpyWoWMBUGI72wlRNyIzfyQwvNRKCQUtPNszcvGjaQebtOAaGBNd SSBiTsj/WSqCk0T+XDsWBMfAk8aSOTued8hhwmwEDcEbntujt38AjGq+u9bszR47jogi pS2wmGharw2SdfayPuYHuTSURobTyUrG3tmDYftUmTpAYiXPJ81KeDuIt756PS+J2GDG LLj8efJ4kvnZmL65VcmZq3Yg8SJgjO2+HbfIpNzXz6UtToavxECo7BjbVhfuVflWpdu2 goe+QoQF35az4Gqh+kAgKfkhVPBfEDSh7imoVAbemwXg9b1lmN3nGu0AnwBNhGOCElta 5s+Q== X-Gm-Message-State: AKwxytcuTsCPHuLAEcMNAr27bq8kYTG49U7IJFhHk6IjC9iXufa+TWlO 5JW2mEJCsx6dkFWZDUMNpdj9/g== X-Google-Smtp-Source: AH8x225JQNMYX+6rCfoQDGS8TWP63Qpj3aOD3Kzotwd1RvNkUDV0iChmeck3iSbV9gPbAU1zMEjVWQ== X-Received: by 10.80.246.21 with SMTP id c21mr64507942edn.271.1517562990637; Fri, 02 Feb 2018 01:16:30 -0800 (PST) Received: from [192.168.1.58] (82.107.69.91.rev.sfr.net. [91.69.107.82]) by smtp.gmail.com with ESMTPSA id a5sm1282122edm.38.2018.02.02.01.16.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Feb 2018 01:16:29 -0800 (PST) To: Vipin Varghese , dev@dpdk.org Cc: ferruh.yigit@intel.com, deepak.k.jain@intel.com References: <1517422961-21284-1-git-send-email-vipin.varghese@intel.com> From: Pascal Mazon Message-ID: <021c3202-fa37-ba37-fc7d-96eb62e4ff95@6wind.com> Date: Fri, 2 Feb 2018 10:16:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <1517422961-21284-1-git-send-email-vipin.varghese@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Feb 2018 09:16:31 -0000 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 > --- > 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: */ > 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 "= " > ETH_TAP_SPEED_ARG "= " > - ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " " > + ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_ARG_FMT > ETH_TAP_REMOTE_ARG "=");