On 12/21/2017 4:01 PM, Vipin Varghese wrote:
> Added fixes for user TAP user MAC
> 1) user format to RTE_PMD_REGISTER_PARAM_STRING
> 2) TAP to the RTE_LOG in absence of dynamic RTE_LOG
> 3) Boundary case for MAC string added
> > ---------------------------------------------------------------
>
> Other Details:
> 1) not to extract "string to mac" conversion to its own function
> 2) set_mac_type does not take any pmd or device name
> 3) Segault for boundary cases 'mac="01:01:01:01:01:01', not found
> 4) Added user MAC format string
Hi Vipin,
Patch version changes shouldn't be part of git commit. But it is good to put
them after "---" in commit log. Can you please update the commit log?
Also please add maintainer to the cc/to .
>
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
<...>
> - strncpy(macTemp, value, 18);
> -
> - macByte = strtok(macTemp, ":");
> - while ((macByte != NULL) &&
> - (strspn(macByte, "0123456789ABCDEFabcdef")) &&
> - (strspn((macByte + 1), "0123456789ABCDEFabcdef")) &&
> - (strlen(macByte) == 2)) {
> - user_mac[index] = strtoul(macByte, NULL, 16);
> - macByte = strtok(NULL, ":");
> + strncpy(mac_temp, value, 18);
> + mac_temp[19] = "\0";
This cause a build error [1], should be used '\0':
[1]
...drivers/net/tap/rte_eth_tap.c: In function ‘set_mac_type’:
...drivers/net/tap/rte_eth_tap.c:1467:18: error: assignment makes integer from
pointer without a cast [-Werror=int-conversion]
mac_temp[19] = "\0";
^
<....>
> - ETH_TAP_IFACE_ARG "=<string> "
> - ETH_TAP_SPEED_ARG "=<int> "
> - ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
> - ETH_TAP_REMOTE_ARG "=<string>");
> + ETH_TAP_IFACE_ARG "=<string> "
> + ETH_TAP_SPEED_ARG "=<int> "
> + ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
> + ETH_TAP_REMOTE_ARG "=<string>");
This also build a build error [2] because of how ETH_TAP_USER_MAC_FMT defined [3].
[2]
.../drivers/net/tap/rte_eth_tap.c: At top level:
.../drivers/net/tap/rte_eth_tap.c:45:33: error: called object is not a function
or function pointer
#define ETH_TAP_IFACE_ARG "iface"
^
.../build/include/rte_dev.h:265:25: note: in definition of macro
‘RTE_PMD_REGISTER_PARAM_STRING’
__attribute__((used)) = str
^~~
.../drivers/net/tap/rte_eth_tap.c:1628:2: note: in expansion of macro
‘ETH_TAP_IFACE_ARG’
ETH_TAP_IFACE_ARG "=<string> "
^~~~~~~~~~~~~~~~~
.../drivers/net/tap/rte_eth_tap.c:1630:65: error: expected ‘,’ or ‘;’ before
string constant
ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
^
.../build/include/rte_dev.h:265:25: note: in definition of macro
‘RTE_PMD_REGISTER_PARAM_STRING’
__attribute__((used)) = str
^~~
[3]
#define ETH_TAP_USER_MAC_FMT ("xx:xx:xx:xx:xx:xx")
parenthesis ...
@@ -81,6 +81,11 @@
#define FLOWER_KERNEL_VERSION KERNEL_VERSION(4, 2, 0)
#define FLOWER_VLAN_KERNEL_VERSION KERNEL_VERSION(4, 9, 0)
+#define MAC_STRING_NULL (0)
+#define MAC_STRING_FIXED (1)
+#define MAC_STRING_USER (2)
+#define ETH_TAP_USER_MAC_FMT ("xx:xx:xx:xx:xx:xx")
+
static unsigned char user_mac[ETHER_ADDR_LEN];
static struct rte_vdev_driver pmd_tap_drv;
@@ -1293,17 +1298,18 @@ enum ioctl_mode {
pmd->txq[i].fd = -1;
}
- if (fixed_mac_type == 1) {
+ if (fixed_mac_type == MAC_STRING_FIXED) {
/* fixed mac = 00:64:74:61:70:<iface_idx> */
static int iface_idx;
char mac[ETHER_ADDR_LEN] = "\0dtap";
mac[ETHER_ADDR_LEN - 1] = iface_idx++;
rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN);
- } else if (fixed_mac_type == 2) {
- /* user mac is recieved */
+ } else if (fixed_mac_type == MAC_STRING_USER) {
+ /* user mac is received */
RTE_LOG(INFO, PMD,
- "Using user MAC (%02x:%02x:%02x:%02x:%02x:%02x)\n",
+ "%s; Using user MAC (%02x:%02x:%02x:%02x:%02x:%02x) argument\n",
+ pmd->name,
user_mac[0], user_mac[1], user_mac[2],
user_mac[3], user_mac[4], user_mac[5]);
rte_memcpy(&pmd->eth_addr, user_mac, ETHER_ADDR_LEN);
@@ -1476,53 +1482,49 @@ enum ioctl_mode {
}
static int
-set_mac_type(const char *key __rte_unused,
- const char *value,
- void *extra_args)
+set_mac_type(const char *key __rte_unused, const char *value, void *extra_args)
{
- char macTemp[20], *macByte = NULL;
+ char mac_temp[20] = {0}, *mac_byte = NULL;
unsigned int index = 0;
if (value) {
+ RTE_LOG(DEBUG, PMD, "TAP user MAC (%s) to set.\n", value);
+
if (!strncasecmp(ETH_TAP_MAC_FIXED, value,
strlen(ETH_TAP_MAC_FIXED))) {
- *(int *)extra_args = 1;
+ *(int *)extra_args = MAC_STRING_FIXED;
} else {
- RTE_LOG(INFO, PMD, "User shared MAC param (%s)\n",
- value);
-
/* desired format aa:bb:cc:dd:ee:ff:11 */
if (strlen(value) == 17) {
- strncpy(macTemp, value, 18);
-
- macByte = strtok(macTemp, ":");
- while ((macByte != NULL) &&
- (strspn(macByte, "0123456789ABCDEFabcdef")) &&
- (strspn((macByte + 1), "0123456789ABCDEFabcdef")) &&
- (strlen(macByte) == 2)) {
- user_mac[index] = strtoul(macByte, NULL, 16);
- macByte = strtok(NULL, ":");
+ strncpy(mac_temp, value, 18);
+ mac_temp[19] = "\0";
+ mac_byte = strtok(mac_temp, ":");
+
+ while ((mac_byte != NULL) &&
+ (strspn(mac_byte, "0123456789ABCDEFabcdef")) &&
+ (strspn((mac_byte + 1), "0123456789ABCDEFabcdef")) &&
+ (strlen(mac_byte) == 2)) {
+ user_mac[index] = strtoul(mac_byte, NULL, 16);
+ mac_byte = strtok(NULL, ":");
index += 1;
}
- if (index != 6) {
- RTE_LOG(ERR, PMD, " failure in MAC value (%s) at %u\n",
- macByte, index + 1);
- return -1;
- }
+ if (index != 6)
+ goto error;
- RTE_LOG(DEBUG, PMD, " User MAC (%s) considered\n",
- value);
- *(int *)extra_args = 2;
- } else {
- RTE_LOG(ERR, PMD, " Mismatch on format for (%s)\n",
- value);
- return -1;
- }
+ *(int *)extra_args = MAC_STRING_USER;
+ } else
+ goto error;
}
+ RTE_LOG(DEBUG, PMD, "TAP user MAC (%s) considered\n", value);
}
return 0;
+
+error:
+ RTE_LOG(ERR, PMD, "TAP user MAC (%s) is not in format (%s)\n",
+ value, ETH_TAP_USER_MAC_FMT);
+ return -1;
}
/* Open a TAP interface device.
@@ -1536,7 +1538,7 @@ enum ioctl_mode {
int speed;
char tap_name[RTE_ETH_NAME_MAX_LEN];
char remote_iface[RTE_ETH_NAME_MAX_LEN];
- int fixed_mac_type = 0;
+ int fixed_mac_type = MAC_STRING_NULL;
name = rte_vdev_device_name(dev);
params = rte_vdev_device_args(dev);
@@ -1656,7 +1658,7 @@ enum ioctl_mode {
RTE_PMD_REGISTER_VDEV(net_tap, pmd_tap_drv);
RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
RTE_PMD_REGISTER_PARAM_STRING(net_tap,
- ETH_TAP_IFACE_ARG "=<string> "
- ETH_TAP_SPEED_ARG "=<int> "
- ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " "
- ETH_TAP_REMOTE_ARG "=<string>");
+ ETH_TAP_IFACE_ARG "=<string> "
+ ETH_TAP_SPEED_ARG "=<int> "
+ ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " "
+ ETH_TAP_REMOTE_ARG "=<string>");