From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wiles, Keith" Subject: Re: [PATCH] net/tap: add support for fixed mac addresses Date: Tue, 11 Apr 2017 15:42:32 +0000 Message-ID: References: <86B786CB-214B-4F44-857C-2FB61341CD7A@intel.com> <5c2f65e8087fffc62c03b0c36b01a3058f203254.1491923112.git.pascal.mazon@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: Pascal Mazon Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id BD1A658F6 for ; Tue, 11 Apr 2017 17:42:35 +0200 (CEST) In-Reply-To: <5c2f65e8087fffc62c03b0c36b01a3058f203254.1491923112.git.pascal.mazon@6wind.com> Content-Language: en-US Content-ID: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > On Apr 11, 2017, at 10:09 AM, Pascal Mazon wrote= : >=20 > Support for a fixed MAC address for testing with the last octet > incrementing by one for each interface defined with the new 'mac=3Dfixed' > string on the --vdev option. The default option is still to randomize > the MAC address for each tap interface. >=20 > Signed-off-by: Keith Wiles > Signed-off-by: Pascal Mazon > --- >=20 > Hi Keith, I've made the changes I suggested. >=20 > It does as expected use a mac incremented by 1 for each new tap device, > without a global variable for fixed_mac_type. >=20 > With the more clean "\0dtap" initialization for mac[], I do think it > looks better, don't you? > I agree my version with \x did little improvement to yours, and would > be a question of style. > What do you say of this patch? >=20 > doc/guides/nics/tap.rst | 13 ++++++++++++- > drivers/net/tap/rte_eth_tap.c | 39 ++++++++++++++++++++++++++++++++++++--= - > 2 files changed, 48 insertions(+), 4 deletions(-) >=20 > diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst > index 5c5ba5357bba..f3ee95d2897b 100644 > --- a/doc/guides/nics/tap.rst > +++ b/doc/guides/nics/tap.rst > @@ -46,7 +46,7 @@ These TAP interfaces can be used with Wireshark or tcpd= ump or Pktgen-DPDK > along with being able to be used as a network connection to the DPDK > application. The method enable one or more interfaces is to use the > ``--vdev=3Dnet_tap0`` option on the DPDK application command line. Each > -``--vdev=3Dnet_tap1`` option give will create an interface named dtap0, = dtap1, > +``--vdev=3Dnet_tap1`` option given will create an interface named dtap0,= dtap1, > and so on. >=20 > The interface name can be changed by adding the ``iface=3Dfoo0``, for exa= mple:: > @@ -58,6 +58,17 @@ needed, but the interface does not enforce that speed,= for example:: >=20 > --vdev=3Dnet_tap0,iface=3Dfoo0,speed=3D25000 >=20 > +Normally the PMD will generate a random MAC address, but when testing or= with > +a static configuration the developer may need a fixed MAC address style. > +Using the option ``mac=3Dfixed`` you can create a fixed known MAC addres= s:: > + > + --vdev=3Dnet_tap0,mac=3Dfixed > + > +The MAC address will have a fixed value with the last octet incrementing= by one > +for each interface string containing ``mac=3Dfixed``. 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]``. > + > It is possible to specify a remote netdevice to capture packets from by a= dding > ``remote=3Dfoo1``, for example:: >=20 > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.= c > index cd82a7e09c75..2092f5d4184b 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -1,7 +1,7 @@ > /*- > * BSD LICENSE > * > - * Copyright(c) 2016 Intel Corporation. All rights reserved. > + * Copyright(c) 2016-2017 Intel Corporation. All rights reserved. > * All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > @@ -71,6 +71,8 @@ > #define ETH_TAP_IFACE_ARG "iface" > #define ETH_TAP_SPEED_ARG "speed" > #define ETH_TAP_REMOTE_ARG "remote" > +#define ETH_TAP_MAC_ARG "mac" > +#define ETH_TAP_MAC_FIXED "fixed" >=20 > #define FLOWER_KERNEL_VERSION KERNEL_VERSION(4, 2, 0) > #define FLOWER_VLAN_KERNEL_VERSION KERNEL_VERSION(4, 9, 0) > @@ -81,6 +83,7 @@ static const char *valid_arguments[] =3D { > ETH_TAP_IFACE_ARG, > ETH_TAP_SPEED_ARG, > ETH_TAP_REMOTE_ARG, > + ETH_TAP_MAC_ARG, > NULL > }; >=20 > @@ -1131,7 +1134,8 @@ tap_kernel_support(struct pmd_internals *pmd) > } >=20 > static int > -eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface) > +eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface, > + int fixed_mac_type) > { > int numa_node =3D rte_socket_id(); > struct rte_eth_dev *dev =3D NULL; > @@ -1229,6 +1233,13 @@ eth_dev_tap_create(const char *name, char *tap_nam= e, char *remote_iface) > } > rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data, > ETHER_ADDR_LEN); > + } else if (fixed_mac_type) { > + /* fixed mac =3D 00:64:74:61:70: */ > + static int iface_idx; > + char mac[ETHER_ADDR_LEN] =3D "\0dtap"; > + > + mac[ETHER_ADDR_LEN - 1] =3D iface_idx++; > + rte_memcpy(&pmd->eth_addr, mac, ETHER_ADDR_LEN); > } else { > eth_random_addr((uint8_t *)&pmd->eth_addr); > } This does not show up as a change in the patch, I thought it should. > @@ -1285,6 +1296,17 @@ set_remote_iface(const char *key __rte_unused, > return 0; > } >=20 > +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 =3D 1; > + return 0; > +} > + > /* Open a TAP interface device. > */ > static int > @@ -1295,6 +1317,7 @@ rte_pmd_tap_probe(const char *name, const char *par= ams) > int speed; > char tap_name[RTE_ETH_NAME_MAX_LEN]; > char remote_iface[RTE_ETH_NAME_MAX_LEN]; > + int fixed_mac_type =3D 0; >=20 > speed =3D ETH_SPEED_NUM_10G; > snprintf(tap_name, sizeof(tap_name), "%s%d", > @@ -1332,6 +1355,15 @@ rte_pmd_tap_probe(const char *name, const char *pa= rams) > if (ret =3D=3D -1) > goto leave; > } > + > + if (rte_kvargs_count(kvlist, ETH_TAP_MAC_ARG) =3D=3D 1) { > + ret =3D rte_kvargs_process(kvlist, > + ETH_TAP_MAC_ARG, > + &set_mac_type, > + &fixed_mac_type); > + if (ret =3D=3D -1) > + goto leave; > + } > } > } > pmd_link.link_speed =3D speed; > @@ -1339,7 +1371,7 @@ rte_pmd_tap_probe(const char *name, const char *par= ams) > RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n", > name, tap_name); >=20 > - ret =3D eth_dev_tap_create(name, tap_name, remote_iface); > + ret =3D eth_dev_tap_create(name, tap_name, remote_iface, fixed_mac_type= ); >=20 > leave: > if (ret =3D=3D -1) { > @@ -1397,4 +1429,5 @@ RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap); > RTE_PMD_REGISTER_PARAM_STRING(net_tap, > ETH_TAP_IFACE_ARG "=3D " > ETH_TAP_SPEED_ARG "=3D " > + ETH_TAP_MAC_ARG "=3D" ETH_TAP_MAC_FIXED " " > ETH_TAP_REMOTE_ARG "=3D"); > --=20 > 2.12.0.306.g4a9b9b3 >=20 Regards, Keith