* [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args
@ 2018-01-31 18:22 Vipin Varghese
2018-02-02 9:16 ` Pascal Mazon
2018-02-05 15:54 ` [dpdk-dev] [PATCH v2] " Pascal Mazon
0 siblings, 2 replies; 23+ messages in thread
From: Vipin Varghese @ 2018-01-31 18:22 UTC (permalink / raw)
To: dev; +Cc: pascal.mazon, ferruh.yigit, deepak.k.jain, 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args
2018-01-31 18:22 [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args Vipin Varghese
@ 2018-02-02 9:16 ` Pascal Mazon
2018-02-02 9:49 ` Varghese, Vipin
2018-02-05 15:54 ` [dpdk-dev] [PATCH v2] " Pascal Mazon
1 sibling, 1 reply; 23+ messages in thread
From: Pascal Mazon @ 2018-02-02 9:16 UTC (permalink / raw)
To: Vipin Varghese, dev; +Cc: ferruh.yigit, deepak.k.jain
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>");
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args
2018-02-02 9:16 ` Pascal Mazon
@ 2018-02-02 9:49 ` Varghese, Vipin
2018-02-05 8:23 ` Varghese, Vipin
0 siblings, 1 reply; 23+ messages in thread
From: Varghese, Vipin @ 2018-02-02 9:49 UTC (permalink / raw)
To: Pascal Mazon, dev; +Cc: Yigit, Ferruh, Jain, Deepak K
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>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args
2018-02-02 9:49 ` Varghese, Vipin
@ 2018-02-05 8:23 ` Varghese, Vipin
0 siblings, 0 replies; 23+ messages in thread
From: Varghese, Vipin @ 2018-02-05 8:23 UTC (permalink / raw)
To: Varghese, Vipin, Pascal Mazon, dev; +Cc: Yigit, Ferruh, Jain, Deepak K
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>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
2018-01-31 18:22 [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args Vipin Varghese
2018-02-02 9:16 ` Pascal Mazon
@ 2018-02-05 15:54 ` Pascal Mazon
2018-02-05 15:58 ` Varghese, Vipin
` (2 more replies)
1 sibling, 3 replies; 23+ messages in thread
From: Pascal Mazon @ 2018-02-05 15:54 UTC (permalink / raw)
To: dev, vipin.varghese; +Cc: ferruh.yigit, deepak.k.jain, 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
2018-02-05 15:54 ` [dpdk-dev] [PATCH v2] " Pascal Mazon
@ 2018-02-05 15:58 ` Varghese, Vipin
2018-02-05 18:13 ` Varghese, Vipin
2018-02-05 18:42 ` Ferruh Yigit
2018-02-12 14:44 ` [dpdk-dev] [PATCH v1] " Vipin Varghese
2 siblings, 1 reply; 23+ messages in thread
From: Varghese, Vipin @ 2018-02-05 15:58 UTC (permalink / raw)
To: Pascal Mazon, dev; +Cc: Yigit, Ferruh, Jain, Deepak K
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
2018-02-05 15:58 ` Varghese, Vipin
@ 2018-02-05 18:13 ` Varghese, Vipin
0 siblings, 0 replies; 23+ messages in thread
From: Varghese, Vipin @ 2018-02-05 18:13 UTC (permalink / raw)
To: Varghese, Vipin, Pascal Mazon, dev; +Cc: Yigit, Ferruh, Jain, Deepak K
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
2018-02-05 15:54 ` [dpdk-dev] [PATCH v2] " Pascal Mazon
2018-02-05 15:58 ` Varghese, Vipin
@ 2018-02-05 18:42 ` Ferruh Yigit
2018-02-12 14:44 ` [dpdk-dev] [PATCH v1] " Vipin Varghese
2 siblings, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2018-02-05 18:42 UTC (permalink / raw)
To: Pascal Mazon, dev, vipin.varghese; +Cc: deepak.k.jain
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?
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args
2018-02-05 15:54 ` [dpdk-dev] [PATCH v2] " Pascal Mazon
2018-02-05 15:58 ` Varghese, Vipin
2018-02-05 18:42 ` Ferruh Yigit
@ 2018-02-12 14:44 ` Vipin Varghese
2018-02-22 11:52 ` Pascal Mazon
` (2 more replies)
2 siblings, 3 replies; 23+ messages in thread
From: Vipin Varghese @ 2018-02-12 14:44 UTC (permalink / raw)
To: dev, pascal.mazon; +Cc: ferruh.yigit, deepak.k.jain, 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args
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-12 20:21 ` [dpdk-dev] [PATCH v2] " Vipin Varghese
2 siblings, 0 replies; 23+ messages in thread
From: Pascal Mazon @ 2018-02-22 11:52 UTC (permalink / raw)
To: Vipin Varghese, dev; +Cc: ferruh.yigit, deepak.k.jain
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>");
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args
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
2018-03-12 20:21 ` [dpdk-dev] [PATCH v2] " Vipin Varghese
2 siblings, 1 reply; 23+ messages in thread
From: Ferruh Yigit @ 2018-03-06 16:41 UTC (permalink / raw)
To: Vipin Varghese, dev, pascal.mazon; +Cc: deepak.k.jain
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/tap: allow user MAC to be passed as args
2018-03-06 16:41 ` Ferruh Yigit
@ 2018-03-07 9:44 ` Varghese, Vipin
0 siblings, 0 replies; 23+ messages in thread
From: Varghese, Vipin @ 2018-03-07 9:44 UTC (permalink / raw)
To: Yigit, Ferruh, dev, pascal.mazon; +Cc: Jain, Deepak K
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
2018-03-12 20:21 ` [dpdk-dev] [PATCH v2] " Vipin Varghese
@ 2018-03-12 15:08 ` Ferruh Yigit
2018-03-12 15:38 ` Varghese, Vipin
2018-03-12 17:33 ` Stephen Hemminger
2018-03-12 21:53 ` [dpdk-dev] [PATCH v3] " Vipin Varghese
2 siblings, 1 reply; 23+ messages in thread
From: Ferruh Yigit @ 2018-03-12 15:08 UTC (permalink / raw)
To: Vipin Varghese, dev, pascal.mazon
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?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
2018-03-12 15:08 ` Ferruh Yigit
@ 2018-03-12 15:38 ` Varghese, Vipin
0 siblings, 0 replies; 23+ messages in thread
From: Varghese, Vipin @ 2018-03-12 15:38 UTC (permalink / raw)
To: Yigit, Ferruh, dev, pascal.mazon
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?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
2018-03-12 20:21 ` [dpdk-dev] [PATCH v2] " Vipin Varghese
2018-03-12 15:08 ` Ferruh Yigit
@ 2018-03-12 17:33 ` Stephen Hemminger
2018-03-12 21:53 ` [dpdk-dev] [PATCH v3] " Vipin Varghese
2 siblings, 0 replies; 23+ messages in thread
From: Stephen Hemminger @ 2018-03-12 17:33 UTC (permalink / raw)
To: Vipin Varghese; +Cc: dev, ferruh.yigit, pascal.mazon
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...
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/tap: allow user MAC to be passed as args
2018-03-12 21:53 ` [dpdk-dev] [PATCH v3] " Vipin Varghese
@ 2018-03-12 17:45 ` Ferruh Yigit
2018-03-16 14:22 ` Ferruh Yigit
0 siblings, 1 reply; 23+ messages in thread
From: Ferruh Yigit @ 2018-03-12 17:45 UTC (permalink / raw)
To: Vipin Varghese, dev, pascal.mazon
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>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v2] net/tap: allow user MAC to be passed as args
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-12 20:21 ` Vipin Varghese
2018-03-12 15:08 ` Ferruh Yigit
` (2 more replies)
2 siblings, 3 replies; 23+ messages in thread
From: Vipin Varghese @ 2018-03-12 20:21 UTC (permalink / raw)
To: dev, ferruh.yigit, pascal.mazon; +Cc: 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v3] net/tap: allow user MAC to be passed as args
2018-03-12 20:21 ` [dpdk-dev] [PATCH v2] " Vipin Varghese
2018-03-12 15:08 ` Ferruh Yigit
2018-03-12 17:33 ` Stephen Hemminger
@ 2018-03-12 21:53 ` Vipin Varghese
2018-03-12 17:45 ` Ferruh Yigit
2 siblings, 1 reply; 23+ messages in thread
From: Vipin Varghese @ 2018-03-12 21:53 UTC (permalink / raw)
To: dev, ferruh.yigit, pascal.mazon; +Cc: 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/tap: allow user MAC to be passed as args
2018-03-12 17:45 ` Ferruh Yigit
@ 2018-03-16 14:22 ` Ferruh Yigit
0 siblings, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2018-03-16 14:22 UTC (permalink / raw)
To: Vipin Varghese, dev, pascal.mazon
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/tap: allow user mac to be passed as args
2017-12-07 20:11 ` Ferruh Yigit
2017-12-08 10:14 ` Pascal Mazon
@ 2017-12-16 2:21 ` Varghese, Vipin
1 sibling, 0 replies; 23+ messages in thread
From: Varghese, Vipin @ 2017-12-16 2:21 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: Hunt, David, Jain, Deepak K
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;
> }
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/tap: allow user mac to be passed as args
2017-12-07 20:11 ` Ferruh Yigit
@ 2017-12-08 10:14 ` Pascal Mazon
2017-12-16 2:21 ` Varghese, Vipin
1 sibling, 0 replies; 23+ messages in thread
From: Pascal Mazon @ 2017-12-08 10:14 UTC (permalink / raw)
To: dev
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;
>> }
>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/tap: allow user mac to be passed as args
2017-11-30 19:49 [dpdk-dev] [PATCH v1] net/tap: allow user mac " Vipin Varghese
@ 2017-12-07 20:11 ` Ferruh Yigit
2017-12-08 10:14 ` Pascal Mazon
2017-12-16 2:21 ` Varghese, Vipin
0 siblings, 2 replies; 23+ messages in thread
From: Ferruh Yigit @ 2017-12-07 20:11 UTC (permalink / raw)
To: Vipin Varghese, dev; +Cc: david.hunt, deepak.k.jain
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;
> }
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v1] net/tap: allow user mac to be passed as args
@ 2017-11-30 19:49 Vipin Varghese
2017-12-07 20:11 ` Ferruh Yigit
0 siblings, 1 reply; 23+ messages in thread
From: Vipin Varghese @ 2017-11-30 19:49 UTC (permalink / raw)
To: dev; +Cc: david.hunt, deepak.k.jain, 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
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2018-03-16 14:22 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-31 18:22 [dpdk-dev] [PATCH] net/tap: allow user MAC to be passed as args Vipin Varghese
2018-02-02 9:16 ` Pascal Mazon
2018-02-02 9:49 ` Varghese, Vipin
2018-02-05 8:23 ` Varghese, Vipin
2018-02-05 15:54 ` [dpdk-dev] [PATCH v2] " Pascal Mazon
2018-02-05 15:58 ` Varghese, Vipin
2018-02-05 18:13 ` Varghese, Vipin
2018-02-05 18:42 ` Ferruh Yigit
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
2018-03-12 20:21 ` [dpdk-dev] [PATCH v2] " Vipin Varghese
2018-03-12 15:08 ` Ferruh Yigit
2018-03-12 15:38 ` Varghese, Vipin
2018-03-12 17:33 ` Stephen Hemminger
2018-03-12 21:53 ` [dpdk-dev] [PATCH v3] " Vipin Varghese
2018-03-12 17:45 ` Ferruh Yigit
2018-03-16 14:22 ` Ferruh Yigit
-- strict thread matches above, loose matches on Subject: below --
2017-11-30 19:49 [dpdk-dev] [PATCH v1] net/tap: allow user mac " Vipin Varghese
2017-12-07 20:11 ` Ferruh Yigit
2017-12-08 10:14 ` Pascal Mazon
2017-12-16 2:21 ` Varghese, Vipin
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).