DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v5] net/pcap: physical interface MAC address support
@ 2018-09-06 16:56 Juhamatti Kuusisaari
  2018-09-10 12:03 ` Ferruh Yigit
  2018-09-10 16:55 ` [dpdk-dev] [PATCH v6] " Juhamatti Kuusisaari
  0 siblings, 2 replies; 11+ messages in thread
From: Juhamatti Kuusisaari @ 2018-09-06 16:56 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Juhamatti Kuusisaari

Support for PCAP physical interface MAC with phy_mac=1 devarg.

Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
---
 doc/guides/rel_notes/release_18_11.rst |   4 +
 drivers/net/pcap/rte_eth_pcap.c        | 119 +++++++++++++++++++++++--
 2 files changed, 118 insertions(+), 5 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 3ae6b3f58..70966740a 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -54,6 +54,10 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **Added a devarg to use PCAP interface physical MAC address.**
+  A new devarg ``phy_mac`` was introduced to allow users to use physical
+  MAC address of the selected PCAP interface.
+
 
 API Changes
 -----------
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index e8810a171..8917c4c4d 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -7,6 +7,14 @@
 #include <time.h>
 
 #include <net/if.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+#ifdef __FreeBSD__
+#include <sys/sysctl.h>
+#include <net/if_dl.h>
+#endif
 
 #include <pcap.h>
 
@@ -17,6 +25,7 @@
 #include <rte_malloc.h>
 #include <rte_mbuf.h>
 #include <rte_bus_vdev.h>
+#include <rte_string_fns.h>
 
 #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
 #define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
@@ -29,6 +38,7 @@
 #define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
 #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
 #define ETH_PCAP_IFACE_ARG    "iface"
+#define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
 
 #define ETH_PCAP_ARG_MAXLEN	64
 
@@ -87,6 +97,7 @@ static const char *valid_arguments[] = {
 	ETH_PCAP_RX_IFACE_IN_ARG,
 	ETH_PCAP_TX_IFACE_ARG,
 	ETH_PCAP_IFACE_ARG,
+	ETH_PCAP_PHY_MAC_ARG,
 	NULL
 };
 
@@ -904,12 +915,79 @@ pmd_init_internals(struct rte_vdev_device *vdev,
 	return 0;
 }
 
+static void eth_pcap_update_mac(const char *if_name, struct rte_eth_dev **eth_dev,
+		const unsigned int numa_node)
+{
+	void *mac_addrs;
+	PMD_LOG(INFO, "Setting phy MAC for %s\n",
+			if_name);
+#ifndef __FreeBSD__
+	int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (if_fd != -1) {
+		struct ifreq ifr;
+		strlcpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+		if (!ioctl(if_fd, SIOCGIFHWADDR, &ifr)) {
+			mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN,
+					0, numa_node);
+			if (mac_addrs) {
+				(*eth_dev)->data->mac_addrs = mac_addrs;
+				rte_memcpy((*eth_dev)->data->mac_addrs,
+						ifr.ifr_addr.sa_data,
+						ETHER_ADDR_LEN);
+			}
+		}
+		close(if_fd);
+	}
+#else
+	int mib[6];
+	size_t len = 0;
+	char *buf = NULL;
+
+	mib[0] = CTL_NET;
+	mib[1] = AF_ROUTE;
+	mib[2] = 0;
+	mib[3] = AF_LINK;
+	mib[4] = NET_RT_IFLIST;
+	mib[5] = if_nametoindex(if_name);
+
+	if (sysctl(mib, 6, NULL, &len, NULL, 0) < 0)
+		return;
+
+	if (len > 0) {
+		struct if_msghdr	*ifm;
+		struct sockaddr_dl	*sdl;
+
+		buf = rte_zmalloc_socket(NULL, len,
+				0, numa_node);
+		if (buf) {
+			if (sysctl(mib, 6, buf, &len, NULL, 0) < 0) {
+				rte_free(buf);
+				return;
+			}
+
+			ifm = (struct if_msghdr *)buf;
+			sdl = (struct sockaddr_dl *)(ifm + 1);
+			mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN,
+					0, numa_node);
+			if (mac_addrs) {
+				(*eth_dev)->data->mac_addrs = mac_addrs;
+				rte_memcpy((*eth_dev)->data->mac_addrs,
+						LLADDR(sdl),
+						ETHER_ADDR_LEN);
+			}
+		}
+	}
+	if (buf)
+		rte_free(buf);
+#endif
+}
+
 static int
 eth_from_pcaps_common(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
 		struct rte_kvargs *kvlist, struct pmd_internals **internals,
-		struct rte_eth_dev **eth_dev)
+		const int phy_mac, struct rte_eth_dev **eth_dev)
 {
 	struct rte_kvargs_pair *pair = NULL;
 	unsigned int k_idx;
@@ -955,6 +1033,9 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
 	else
 		(*internals)->if_index = if_nametoindex(pair->value);
 
+	if (phy_mac && pair)
+		eth_pcap_update_mac(pair->value, eth_dev, vdev->device.numa_node);
+
 	return 0;
 }
 
@@ -962,7 +1043,7 @@ static int
 eth_from_pcaps(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
-		struct rte_kvargs *kvlist, int single_iface,
+		struct rte_kvargs *kvlist, int single_iface, int phy_mac,
 		unsigned int using_dumpers)
 {
 	struct pmd_internals *internals = NULL;
@@ -970,7 +1051,7 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	int ret;
 
 	ret = eth_from_pcaps_common(vdev, rx_queues, nb_rx_queues,
-		tx_queues, nb_tx_queues, kvlist, &internals, &eth_dev);
+			tx_queues, nb_tx_queues, kvlist, &internals, phy_mac, &eth_dev);
 
 	if (ret < 0)
 		return ret;
@@ -989,6 +1070,22 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	return 0;
 }
 
+static int
+select_phy_mac(const char *key, const char *value, void *extra_args)
+{
+	if (extra_args && strcmp(key, ETH_PCAP_PHY_MAC_ARG) == 0) {
+		const int phy_mac = atoi(value);
+		int *enable_phy_mac = extra_args;
+
+		if (phy_mac != 0 && phy_mac != 1)
+			PMD_LOG(WARNING, "Value should be 0 or 1, set it as 1!");
+
+		if (phy_mac)
+			*enable_phy_mac = 1;
+	}
+	return 0;
+}
+
 static int
 pmd_pcap_probe(struct rte_vdev_device *dev)
 {
@@ -999,6 +1096,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 	struct pmd_devargs dumpers = {0};
 	struct rte_eth_dev *eth_dev;
 	int single_iface = 0;
+	int phy_mac = 0;
 	int ret;
 
 	name = rte_vdev_device_name(dev);
@@ -1026,6 +1124,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 	if (kvlist == NULL)
 		return -1;
 
+	/*
+	 * We check whether we want to use phy MAC of pcap interface.
+	 */
+	if (rte_kvargs_count(kvlist, ETH_PCAP_PHY_MAC_ARG)) {
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
+				&select_phy_mac, &phy_mac);
+		if (ret < 0)
+			goto free_kvlist;
+	}
+
 	/*
 	 * If iface argument is passed we open the NICs and use them for
 	 * reading / writing
@@ -1084,7 +1192,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 
 create_eth:
 	ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
-		dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
+			dumpers.num_of_queue, kvlist, single_iface, phy_mac, is_tx_pcap);
 
 free_kvlist:
 	rte_kvargs_free(kvlist);
@@ -1128,7 +1236,8 @@ RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
 	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
 	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
 	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
-	ETH_PCAP_IFACE_ARG "=<ifc>");
+	ETH_PCAP_IFACE_ARG "=<ifc> "
+	ETH_PCAP_PHY_MAC_ARG "=<int>");
 
 RTE_INIT(eth_pcap_init_log)
 {
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v5] net/pcap: physical interface MAC address support
  2018-09-06 16:56 [dpdk-dev] [PATCH v5] net/pcap: physical interface MAC address support Juhamatti Kuusisaari
@ 2018-09-10 12:03 ` Ferruh Yigit
  2018-09-10 17:20   ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
  2018-09-10 16:55 ` [dpdk-dev] [PATCH v6] " Juhamatti Kuusisaari
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2018-09-10 12:03 UTC (permalink / raw)
  To: Juhamatti Kuusisaari; +Cc: dev

On 9/6/2018 5:56 PM, Juhamatti Kuusisaari wrote:
> Support for PCAP physical interface MAC with phy_mac=1 devarg.
> 
> Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>

Hi Juhamatti,

Thanks for the patch.

Can you please add a little more details into commit log that why this new
feature is enabled, what is the use case?

Also can you please send new version of the patches as a reply to previous one,
- via "--in-reply-to" git send-email argument. And a history after commit log
what changed in new version. These helps tracing the multiple versions.

> ---
>  doc/guides/rel_notes/release_18_11.rst |   4 +

Please update doc/guides/nics/pcap_ring.rst to document new devargs.

>  drivers/net/pcap/rte_eth_pcap.c        | 119 +++++++++++++++++++++++--
>  2 files changed, 118 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
> index 3ae6b3f58..70966740a 100644
> --- a/doc/guides/rel_notes/release_18_11.rst
> +++ b/doc/guides/rel_notes/release_18_11.rst
> @@ -54,6 +54,10 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =========================================================
>  
> +* **Added a devarg to use PCAP interface physical MAC address.**
> +  A new devarg ``phy_mac`` was introduced to allow users to use physical
> +  MAC address of the selected PCAP interface.
> +
>  
>  API Changes
>  -----------
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index e8810a171..8917c4c4d 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -7,6 +7,14 @@
>  #include <time.h>
>  
>  #include <net/if.h>
> +#include <sys/socket.h>
> +#include <sys/ioctl.h>
> +#include <unistd.h>
> +
> +#ifdef __FreeBSD__
> +#include <sys/sysctl.h>
> +#include <net/if_dl.h>
> +#endif
>  
>  #include <pcap.h>
>  
> @@ -17,6 +25,7 @@
>  #include <rte_malloc.h>
>  #include <rte_mbuf.h>
>  #include <rte_bus_vdev.h>
> +#include <rte_string_fns.h>
>  
>  #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
>  #define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
> @@ -29,6 +38,7 @@
>  #define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
>  #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
>  #define ETH_PCAP_IFACE_ARG    "iface"
> +#define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
>  
>  #define ETH_PCAP_ARG_MAXLEN	64
>  
> @@ -87,6 +97,7 @@ static const char *valid_arguments[] = {
>  	ETH_PCAP_RX_IFACE_IN_ARG,
>  	ETH_PCAP_TX_IFACE_ARG,
>  	ETH_PCAP_IFACE_ARG,
> +	ETH_PCAP_PHY_MAC_ARG,
>  	NULL
>  };
>  
> @@ -904,12 +915,79 @@ pmd_init_internals(struct rte_vdev_device *vdev,
>  	return 0;
>  }
>  
> +static void eth_pcap_update_mac(const char *if_name, struct rte_eth_dev **eth_dev,
> +		const unsigned int numa_node)

Please follow convention, to have return type in a separate line above.

> +{
> +	void *mac_addrs;
> +	PMD_LOG(INFO, "Setting phy MAC for %s\n",
> +			if_name);

This can be single line. And please leave an empty line between variable
declaration and code.

> +#ifndef __FreeBSD__
> +	int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
> +	if (if_fd != -1) {

To reduce the indentation of rest of the code, it helps to reverse the logic,
exit on failure and continue without indentation.

> +		struct ifreq ifr;
> +		strlcpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
> +		if (!ioctl(if_fd, SIOCGIFHWADDR, &ifr)) {
> +			mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN,
> +					0, numa_node);
> +			if (mac_addrs) {
> +				(*eth_dev)->data->mac_addrs = mac_addrs;
> +				rte_memcpy((*eth_dev)->data->mac_addrs,
> +						ifr.ifr_addr.sa_data,
> +						ETHER_ADDR_LEN);
> +			}
> +		}
> +		close(if_fd);
> +	}
> +#else

There is an assumption that it is either Linux or FreeBSD, that is true for now
but Windows is coming :) Better to be safe here and do an "else if" for FreeBSD.

> +	int mib[6];
> +	size_t len = 0;
> +	char *buf = NULL;
> +
> +	mib[0] = CTL_NET;
> +	mib[1] = AF_ROUTE;
> +	mib[2] = 0;
> +	mib[3] = AF_LINK;
> +	mib[4] = NET_RT_IFLIST;
> +	mib[5] = if_nametoindex(if_name);
> +
> +	if (sysctl(mib, 6, NULL, &len, NULL, 0) < 0)
> +		return;
> +
> +	if (len > 0) {
> +		struct if_msghdr	*ifm;
> +		struct sockaddr_dl	*sdl;
> +
> +		buf = rte_zmalloc_socket(NULL, len,
> +				0, numa_node);

This is an interim buffer, no need to use rte_zmalloc_socket API, rte_malloc or
regular malloc will do the same.
Also no need to break the line.

> +		if (buf) {
> +			if (sysctl(mib, 6, buf, &len, NULL, 0) < 0) {
> +				rte_free(buf);
> +				return;
> +			}
> +
> +			ifm = (struct if_msghdr *)buf;
> +			sdl = (struct sockaddr_dl *)(ifm + 1);
> +			mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN,
> +					0, numa_node);
> +			if (mac_addrs) {
> +				(*eth_dev)->data->mac_addrs = mac_addrs;

Just as a heads up, right now the mac_addrs points to global variable so no need
to worry about it, but please beware of a patch does unique mac address for each
interface.
For that case will need to free the mac_addrs before this assignment, based on
which patch goes in, you may need to update this part.

Patch https://patches.dpdk.org/patch/44170/

> +				rte_memcpy((*eth_dev)->data->mac_addrs,
> +						LLADDR(sdl),
> +						ETHER_ADDR_LEN);
> +			}
> +		}
> +	}
> +	if (buf)
> +		rte_free(buf);
> +#endif
> +}
> +
>  static int
>  eth_from_pcaps_common(struct rte_vdev_device *vdev,
>  		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
>  		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
>  		struct rte_kvargs *kvlist, struct pmd_internals **internals,
> -		struct rte_eth_dev **eth_dev)
> +		const int phy_mac, struct rte_eth_dev **eth_dev)
>  {
>  	struct rte_kvargs_pair *pair = NULL;
>  	unsigned int k_idx;
> @@ -955,6 +1033,9 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
>  	else
>  		(*internals)->if_index = if_nametoindex(pair->value);
>  
> +	if (phy_mac && pair)
> +		eth_pcap_update_mac(pair->value, eth_dev, vdev->device.numa_node);

This is a little misleading.

pair will be set only for ETH_PCAP_IFACE_ARG ("iface") argument, it won't work
with rx_iface[_in]/tx_iface arguments. Like below two are same:
1) iface=lo
2) rx_iface=lo,tx_iface=lo

But this code will work only with 1) not 2), better to mention in document.


Also pcap supports multiple queue by repeating devarg, like:
rx_iface=eth0,rx_iface=eth1,rx_iface=eth2,tx_pcap=tx.pcap
This will create tree Rx queues, one for each interface.

Right now "iface" argument is hardcoded to single queue, but without a good
reason, when it changes in the future to support multiple queue, ethdev will use
the MAC from first one, better to clarify in the doc update, or perhaps in the
code comment too.


> +
>  	return 0;
>  }
>  
> @@ -962,7 +1043,7 @@ static int
>  eth_from_pcaps(struct rte_vdev_device *vdev,
>  		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
>  		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
> -		struct rte_kvargs *kvlist, int single_iface,
> +		struct rte_kvargs *kvlist, int single_iface, int phy_mac,
>  		unsigned int using_dumpers)
>  {
>  	struct pmd_internals *internals = NULL;
> @@ -970,7 +1051,7 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
>  	int ret;
>  
>  	ret = eth_from_pcaps_common(vdev, rx_queues, nb_rx_queues,
> -		tx_queues, nb_tx_queues, kvlist, &internals, &eth_dev);
> +			tx_queues, nb_tx_queues, kvlist, &internals, phy_mac, &eth_dev);

Please fix ./devtools/checkpatches.sh warnings.

>  
>  	if (ret < 0)
>  		return ret;
> @@ -989,6 +1070,22 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
>  	return 0;
>  }
>  
> +static int
> +select_phy_mac(const char *key, const char *value, void *extra_args)
> +{
> +	if (extra_args && strcmp(key, ETH_PCAP_PHY_MAC_ARG) == 0) {

no need strcmp, it has been done by rte_kvargs_process().

> +		const int phy_mac = atoi(value);
> +		int *enable_phy_mac = extra_args;
> +
> +		if (phy_mac != 0 && phy_mac != 1)
> +			PMD_LOG(WARNING, "Value should be 0 or 1, set it as 1!");

According below logic value doesn't matter, which seems OK, what about removing
the log? You should document expected values in pcap doc.

> +
> +		if (phy_mac)
> +			*enable_phy_mac = 1;
> +	}
> +	return 0;
> +}
> +
>  static int
>  pmd_pcap_probe(struct rte_vdev_device *dev)
>  {
> @@ -999,6 +1096,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  	struct pmd_devargs dumpers = {0};
>  	struct rte_eth_dev *eth_dev;
>  	int single_iface = 0;
> +	int phy_mac = 0;
>  	int ret;
>  
>  	name = rte_vdev_device_name(dev);
> @@ -1026,6 +1124,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  	if (kvlist == NULL)
>  		return -1;
>  
> +	/*
> +	 * We check whether we want to use phy MAC of pcap interface.
> +	 */
> +	if (rte_kvargs_count(kvlist, ETH_PCAP_PHY_MAC_ARG)) {
> +		ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
> +				&select_phy_mac, &phy_mac);
> +		if (ret < 0)
> +			goto free_kvlist;
> +	}
> +
>  	/*
>  	 * If iface argument is passed we open the NICs and use them for
>  	 * reading / writing
> @@ -1084,7 +1192,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  
>  create_eth:
>  	ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
> -		dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
> +			dumpers.num_of_queue, kvlist, single_iface, phy_mac, is_tx_pcap);
>  
>  free_kvlist:
>  	rte_kvargs_free(kvlist);
> @@ -1128,7 +1236,8 @@ RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
>  	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
>  	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
>  	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
> -	ETH_PCAP_IFACE_ARG "=<ifc>");
> +	ETH_PCAP_IFACE_ARG "=<ifc> "
> +	ETH_PCAP_PHY_MAC_ARG "=<int>");
>  
>  RTE_INIT(eth_pcap_init_log)
>  {
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v6] net/pcap: physical interface MAC address support
  2018-09-06 16:56 [dpdk-dev] [PATCH v5] net/pcap: physical interface MAC address support Juhamatti Kuusisaari
  2018-09-10 12:03 ` Ferruh Yigit
@ 2018-09-10 16:55 ` Juhamatti Kuusisaari
  2018-09-11 15:35   ` Ferruh Yigit
  2018-10-05 20:27   ` [dpdk-dev] [PATCH v7] " Ferruh Yigit
  1 sibling, 2 replies; 11+ messages in thread
From: Juhamatti Kuusisaari @ 2018-09-10 16:55 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Juhamatti Kuusisaari

At the moment, PCAP interfaces use dummy MAC by default. This change
adds support for selecting PCAP physical interface MAC with phy_mac=1
devarg. This allows to setup packet flows using the physical interface
MAC.

Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>

---
 v6:
  * Review changes:
    * Clarified devarg applicability (applies to iface-only)
    * Introduced detailed documentation for the devarg
    * More checkpatches-fixes
 v5-v3:
  * FreeBSD related compilation and checkpatches-fixes
 v2:
  * FreeBSD support introduced
  * Release notes added
 v1:
  * phy_mac=1 devarg support
---
 doc/guides/nics/pcap_ring.rst          |   9 ++
 doc/guides/rel_notes/release_18_11.rst |   4 +
 drivers/net/pcap/rte_eth_pcap.c        | 120 +++++++++++++++++++++++--
 3 files changed, 128 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst
index 879e5430f..604ca4300 100644
--- a/doc/guides/nics/pcap_ring.rst
+++ b/doc/guides/nics/pcap_ring.rst
@@ -96,6 +96,15 @@ The different stream types are:
 
         iface=eth0
 
+Runtime Config Options
+^^^^^^^^^^^^^^^^^^^^^^
+
+- ``Use PCAP interface physical MAC``
+
+ In case ``iface=`` configuration is set, user may want to use the selected interface's physical MAC
+ address. This can be done with a ``devarg`` ``phy_mac``, for example::
+ iface=eth0,phy_mac=1
+
 Examples of Usage
 ^^^^^^^^^^^^^^^^^
 
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 3ae6b3f58..70966740a 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -54,6 +54,10 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **Added a devarg to use PCAP interface physical MAC address.**
+  A new devarg ``phy_mac`` was introduced to allow users to use physical
+  MAC address of the selected PCAP interface.
+
 
 API Changes
 -----------
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index e8810a171..96ff61781 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -7,6 +7,14 @@
 #include <time.h>
 
 #include <net/if.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+#if defined(__FreeBSD__)
+#include <sys/sysctl.h>
+#include <net/if_dl.h>
+#endif
 
 #include <pcap.h>
 
@@ -17,6 +25,7 @@
 #include <rte_malloc.h>
 #include <rte_mbuf.h>
 #include <rte_bus_vdev.h>
+#include <rte_string_fns.h>
 
 #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
 #define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
@@ -29,6 +38,7 @@
 #define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
 #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
 #define ETH_PCAP_IFACE_ARG    "iface"
+#define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
 
 #define ETH_PCAP_ARG_MAXLEN	64
 
@@ -87,6 +97,7 @@ static const char *valid_arguments[] = {
 	ETH_PCAP_RX_IFACE_IN_ARG,
 	ETH_PCAP_TX_IFACE_ARG,
 	ETH_PCAP_IFACE_ARG,
+	ETH_PCAP_PHY_MAC_ARG,
 	NULL
 };
 
@@ -904,12 +915,80 @@ pmd_init_internals(struct rte_vdev_device *vdev,
 	return 0;
 }
 
+static void
+eth_pcap_update_mac(const char *if_name, struct rte_eth_dev **eth_dev,
+		const unsigned int numa_node)
+{
+	void *mac_addrs;
+	struct ifreq ifr;
+
+	PMD_LOG(INFO, "Setting phy MAC for %s\n", if_name);
+#if !defined(__FreeBSD__)
+	int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (if_fd == -1)
+		return;
+
+	strlcpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+	if (!ioctl(if_fd, SIOCGIFHWADDR, &ifr)) {
+		mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN,
+				0, numa_node);
+		if (mac_addrs) {
+			(*eth_dev)->data->mac_addrs = mac_addrs;
+			rte_memcpy((*eth_dev)->data->mac_addrs,
+					ifr.ifr_addr.sa_data,
+					ETHER_ADDR_LEN);
+		}
+	}
+	close(if_fd);
+#elif defined(__FreeBSD__)
+	int mib[6];
+	size_t len = 0;
+	char *buf = NULL;
+
+	mib[0] = CTL_NET;
+	mib[1] = AF_ROUTE;
+	mib[2] = 0;
+	mib[3] = AF_LINK;
+	mib[4] = NET_RT_IFLIST;
+	mib[5] = if_nametoindex(if_name);
+
+	if (sysctl(mib, 6, NULL, &len, NULL, 0) < 0)
+		return;
+
+	if (len > 0) {
+		struct if_msghdr	*ifm;
+		struct sockaddr_dl	*sdl;
+
+		buf = rte_malloc(NULL, len, 0);
+		if (buf) {
+			if (sysctl(mib, 6, buf, &len, NULL, 0) < 0) {
+				rte_free(buf);
+				return;
+			}
+
+			ifm = (struct if_msghdr *)buf;
+			sdl = (struct sockaddr_dl *)(ifm + 1);
+			mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN,
+					0, numa_node);
+			if (mac_addrs) {
+				(*eth_dev)->data->mac_addrs = mac_addrs;
+				rte_memcpy((*eth_dev)->data->mac_addrs,
+						LLADDR(sdl),
+						ETHER_ADDR_LEN);
+			}
+		}
+	}
+	if (buf)
+		rte_free(buf);
+#endif
+}
+
 static int
 eth_from_pcaps_common(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
 		struct rte_kvargs *kvlist, struct pmd_internals **internals,
-		struct rte_eth_dev **eth_dev)
+		const int phy_mac, struct rte_eth_dev **eth_dev)
 {
 	struct rte_kvargs_pair *pair = NULL;
 	unsigned int k_idx;
@@ -955,6 +1034,10 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
 	else
 		(*internals)->if_index = if_nametoindex(pair->value);
 
+	if (phy_mac && pair) /* phy_mac arg is applied only to iface */
+		eth_pcap_update_mac(pair->value, eth_dev,
+				vdev->device.numa_node);
+
 	return 0;
 }
 
@@ -962,7 +1045,7 @@ static int
 eth_from_pcaps(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
-		struct rte_kvargs *kvlist, int single_iface,
+		struct rte_kvargs *kvlist, int single_iface, int phy_mac,
 		unsigned int using_dumpers)
 {
 	struct pmd_internals *internals = NULL;
@@ -970,7 +1053,8 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	int ret;
 
 	ret = eth_from_pcaps_common(vdev, rx_queues, nb_rx_queues,
-		tx_queues, nb_tx_queues, kvlist, &internals, &eth_dev);
+			tx_queues, nb_tx_queues, kvlist,
+			&internals, phy_mac, &eth_dev);
 
 	if (ret < 0)
 		return ret;
@@ -989,6 +1073,19 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	return 0;
 }
 
+static int
+select_phy_mac(const char *key, const char *value, void *extra_args)
+{
+	if (extra_args) {
+		const int phy_mac = atoi(value);
+		int *enable_phy_mac = extra_args;
+
+		if (phy_mac)
+			*enable_phy_mac = 1;
+	}
+	return 0;
+}
+
 static int
 pmd_pcap_probe(struct rte_vdev_device *dev)
 {
@@ -999,6 +1096,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 	struct pmd_devargs dumpers = {0};
 	struct rte_eth_dev *eth_dev;
 	int single_iface = 0;
+	int phy_mac = 0;
 	int ret;
 
 	name = rte_vdev_device_name(dev);
@@ -1031,6 +1129,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 	 * reading / writing
 	 */
 	if (rte_kvargs_count(kvlist, ETH_PCAP_IFACE_ARG) == 1) {
+		/*
+		 * We check first whether we want to use phy MAC of the PCAP
+		 * interface.
+		 */
+		if (rte_kvargs_count(kvlist, ETH_PCAP_PHY_MAC_ARG)) {
+			ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
+					&select_phy_mac, &phy_mac);
+			if (ret < 0)
+				goto free_kvlist;
+		}
 
 		ret = rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG,
 				&open_rx_tx_iface, &pcaps);
@@ -1084,7 +1192,8 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 
 create_eth:
 	ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
-		dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
+			dumpers.num_of_queue, kvlist, single_iface,
+			phy_mac, is_tx_pcap);
 
 free_kvlist:
 	rte_kvargs_free(kvlist);
@@ -1128,7 +1237,8 @@ RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
 	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
 	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
 	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
-	ETH_PCAP_IFACE_ARG "=<ifc>");
+	ETH_PCAP_IFACE_ARG "=<ifc> "
+	ETH_PCAP_PHY_MAC_ARG "=<int>");
 
 RTE_INIT(eth_pcap_init_log)
 {
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v5] net/pcap: physical interface MAC address support
  2018-09-10 12:03 ` Ferruh Yigit
@ 2018-09-10 17:20   ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
  0 siblings, 0 replies; 11+ messages in thread
From: Kuusisaari, Juhamatti (Coriant - FI/Espoo) @ 2018-09-10 17:20 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev


Hello Ferruh,

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, September 10, 2018 3:04 PM
>
> Hi Juhamatti,
> 
> Thanks for the patch.
> 
> Can you please add a little more details into commit log that why this new
> feature is enabled, what is the use case?
> 
> Also can you please send new version of the patches as a reply to previous
> one,
> - via "--in-reply-to" git send-email argument. And a history after commit log
> what changed in new version. These helps tracing the multiple versions.

Thanks for the excellent comments and guidance. I did the changes and v6 has been sent to Patchwork queue. The use case is really just to use PCAP interface as one of the interfaces of the application and thus allowing to use also the physical MAC address of the interface. I tried to elaborate this a bit more, I hope it is at least better now.

Seems that I cannot easily get rid of __FreeBSD__ define checkpatches-nag. I assumed that it isn't really mandatory as I can see the same define used elsewhere.

BR,
--
 Juhamatti


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v6] net/pcap: physical interface MAC address support
  2018-09-10 16:55 ` [dpdk-dev] [PATCH v6] " Juhamatti Kuusisaari
@ 2018-09-11 15:35   ` Ferruh Yigit
  2018-10-01  7:30     ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
  2018-10-05 20:27   ` [dpdk-dev] [PATCH v7] " Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2018-09-11 15:35 UTC (permalink / raw)
  To: Juhamatti Kuusisaari; +Cc: dev

On 9/10/2018 5:55 PM, Juhamatti Kuusisaari wrote:
> At the moment, PCAP interfaces use dummy MAC by default. This change
> adds support for selecting PCAP physical interface MAC with phy_mac=1
> devarg. This allows to setup packet flows using the physical interface
> MAC.
> 
> Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
> 
> ---
>  v6:
>   * Review changes:
>     * Clarified devarg applicability (applies to iface-only)
>     * Introduced detailed documentation for the devarg
>     * More checkpatches-fixes
>  v5-v3:
>   * FreeBSD related compilation and checkpatches-fixes
>  v2:
>   * FreeBSD support introduced
>   * Release notes added
>  v1:
>   * phy_mac=1 devarg support

<...>

> +#elif defined(__FreeBSD__)

Just to double check did you check/verify below code on FreeBSD?

<...>

> @@ -955,6 +1034,10 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
>  	else
>  		(*internals)->if_index = if_nametoindex(pair->value);
>  
> +	if (phy_mac && pair) /* phy_mac arg is applied only to iface */

Having this comment is good, but "iface" is so generic, it may be confusing for
beyond this context, what about "only if iface devarg provided" kind of detail?

<...>

> @@ -989,6 +1073,19 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
>  	return 0;
>  }
>  
> +static int
> +select_phy_mac(const char *key, const char *value, void *extra_args)
> +{
> +	if (extra_args) {
> +		const int phy_mac = atoi(value);
> +		int *enable_phy_mac = extra_args;
> +
> +		if (phy_mac)
> +			*enable_phy_mac = 1;
> +	}
> +	return 0;
> +}

This is causing build error because of "key" not used, needs __rte_unused marker.

<...>

> @@ -1031,6 +1129,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  	 * reading / writing
>  	 */
>  	if (rte_kvargs_count(kvlist, ETH_PCAP_IFACE_ARG) == 1) {
> +		/*
> +		 * We check first whether we want to use phy MAC of the PCAP
> +		 * interface.
> +		 */
> +		if (rte_kvargs_count(kvlist, ETH_PCAP_PHY_MAC_ARG)) {

Do you need count check at all?

> +			ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
> +					&select_phy_mac, &phy_mac);
> +			if (ret < 0)
> +				goto free_kvlist;
> +		}

I would prefer to see this block below ETH_PCAP_IFACE_ARG check, this block is
for "iface", so it makes more sense to me first verify it, later verify phy_mac

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v6] net/pcap: physical interface MAC address support
  2018-09-11 15:35   ` Ferruh Yigit
@ 2018-10-01  7:30     ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
  0 siblings, 0 replies; 11+ messages in thread
From: Kuusisaari, Juhamatti (Coriant - FI/Espoo) @ 2018-10-01  7:30 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev


Hello Ferruh,

> On 9/10/2018 5:55 PM, Juhamatti Kuusisaari wrote:
> > At the moment, PCAP interfaces use dummy MAC by default. This change
> > adds support for selecting PCAP physical interface MAC with phy_mac=1
> > devarg. This allows to setup packet flows using the physical interface
> > MAC.
> >
> > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
> >
> > ---
> >  v6:
> >   * Review changes:
> >     * Clarified devarg applicability (applies to iface-only)
> >     * Introduced detailed documentation for the devarg
> >     * More checkpatches-fixes
> >  v5-v3:
> >   * FreeBSD related compilation and checkpatches-fixes
> >  v2:
> >   * FreeBSD support introduced
> >   * Release notes added
> >  v1:
> >   * phy_mac=1 devarg support
> 
> <...>
> 
> > +#elif defined(__FreeBSD__)
> 
> Just to double check did you check/verify below code on FreeBSD?

I did run a manual test of the added code and seems to work. I think it is still better to run DPDK on it to make sure everything works, so I'll do that before sending next revision.

> 
> <...>
> 
> > @@ -955,6 +1034,10 @@ eth_from_pcaps_common(struct
> rte_vdev_device *vdev,
> >  	else
> >  		(*internals)->if_index = if_nametoindex(pair->value);
> >
> > +	if (phy_mac && pair) /* phy_mac arg is applied only to iface */
> 
> Having this comment is good, but "iface" is so generic, it may be confusing for
> beyond this context, what about "only if iface devarg provided" kind of detail?

Yes, let's elaborate that.

> <...>
> 
> > @@ -989,6 +1073,19 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
> >  	return 0;
> >  }
> >
> > +static int
> > +select_phy_mac(const char *key, const char *value, void *extra_args)
> > +{
> > +	if (extra_args) {
> > +		const int phy_mac = atoi(value);
> > +		int *enable_phy_mac = extra_args;
> > +
> > +		if (phy_mac)
> > +			*enable_phy_mac = 1;
> > +	}
> > +	return 0;
> > +}
> 
> This is causing build error because of "key" not used, needs __rte_unused
> marker.

Yes, need to fix this.

> <...>
> 
> > @@ -1031,6 +1129,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
> >  	 * reading / writing
> >  	 */
> >  	if (rte_kvargs_count(kvlist, ETH_PCAP_IFACE_ARG) == 1) {
> > +		/*
> > +		 * We check first whether we want to use phy MAC of the
> PCAP
> > +		 * interface.
> > +		 */
> > +		if (rte_kvargs_count(kvlist, ETH_PCAP_PHY_MAC_ARG)) {
> 
> Do you need count check at all?

I think it is needed, as the arg may not exist there. At least to me calling process directly does not feel right, even if it would work.

> > +			ret = rte_kvargs_process(kvlist,
> ETH_PCAP_PHY_MAC_ARG,
> > +					&select_phy_mac, &phy_mac);
> > +			if (ret < 0)
> > +				goto free_kvlist;
> > +		}
> 
> I would prefer to see this block below ETH_PCAP_IFACE_ARG check, this
> block is
> for "iface", so it makes more sense to me first verify it, later verify phy_mac

Sure, I'll add it.

As there is already pcap-related MAC address patch merged, I'll need to do a rebase and recheck this patch functionality on top of that. It will take some time (which is a scarce resource at the moment), so I'll try to get this in again after 18.11.

Thanks,
--
 Juhamatti



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v7] net/pcap: physical interface MAC address support
  2018-09-10 16:55 ` [dpdk-dev] [PATCH v6] " Juhamatti Kuusisaari
  2018-09-11 15:35   ` Ferruh Yigit
@ 2018-10-05 20:27   ` Ferruh Yigit
  2018-10-06  0:49     ` [dpdk-dev] [PATCH v8] " Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2018-10-05 20:27 UTC (permalink / raw)
  To: Bruce Richardson, John McNamara, Marko Kovacevic
  Cc: dev, Ferruh Yigit, Juhamatti Kuusisaari, Juhamatti Kuusisaari

From: Juhamatti Kuusisaari <Juhamatti.Kuusisaari@coriant.com>

At the moment, PCAP interfaces use dummy MAC by default. This change
adds support for selecting PCAP physical interface MAC with phy_mac=1
devarg. This allows to setup packet flows using the physical interface
MAC.

Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v7:
* Add internal->phy_mac to be able to free data->mac_addrs
* code review comments applied
* doc format updates
* NOTE: FreeBSD functionality not tested
---
 doc/guides/nics/pcap_ring.rst          |  10 ++
 doc/guides/rel_notes/release_18_11.rst |   4 +
 drivers/net/pcap/rte_eth_pcap.c        | 142 ++++++++++++++++++++++++-
 3 files changed, 151 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst
index 879e5430f..c1ef9196b 100644
--- a/doc/guides/nics/pcap_ring.rst
+++ b/doc/guides/nics/pcap_ring.rst
@@ -96,6 +96,16 @@ The different stream types are:
 
         iface=eth0
 
+Runtime Config Options
+^^^^^^^^^^^^^^^^^^^^^^
+
+- Use PCAP interface physical MAC
+
+ In case ``iface=`` configuration is set, user may want to use the selected interface's physical MAC
+ address. This can be done with a ``devarg`` ``phy_mac``, for example::
+
+   --vdev 'net_pcap0,iface=eth0,phy_mac=1'
+
 Examples of Usage
 ^^^^^^^^^^^^^^^^^
 
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 89ca3317f..491d97f13 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -98,6 +98,10 @@ New Features
   * Support for runtime Rx and Tx queues setup.
   * Support multicast MAC address set.
 
+* **Added a devarg to use PCAP interface physical MAC address.**
+  A new devarg ``phy_mac`` was introduced to allow users to use physical
+  MAC address of the selected PCAP interface.
+
 * **Added Event Ethernet Tx Adapter.**
 
   Added event ethernet Tx adapter library that  provides configuration and
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 8736010f0..2b60e7b64 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -7,6 +7,14 @@
 #include <time.h>
 
 #include <net/if.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+#if defined(RTE_EXEC_ENV_BSDAPP)
+#include <sys/sysctl.h>
+#include <net/if_dl.h>
+#endif
 
 #include <pcap.h>
 
@@ -17,6 +25,7 @@
 #include <rte_malloc.h>
 #include <rte_mbuf.h>
 #include <rte_bus_vdev.h>
+#include <rte_string_fns.h>
 
 #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
 #define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
@@ -29,6 +38,7 @@
 #define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
 #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
 #define ETH_PCAP_IFACE_ARG    "iface"
+#define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
 
 #define ETH_PCAP_ARG_MAXLEN	64
 
@@ -70,6 +80,7 @@ struct pmd_internals {
 	struct ether_addr eth_addr;
 	int if_index;
 	int single_iface;
+	int phy_mac;
 };
 
 struct pmd_devargs {
@@ -89,6 +100,7 @@ static const char *valid_arguments[] = {
 	ETH_PCAP_RX_IFACE_IN_ARG,
 	ETH_PCAP_TX_IFACE_ARG,
 	ETH_PCAP_IFACE_ARG,
+	ETH_PCAP_PHY_MAC_ARG,
 	NULL
 };
 
@@ -859,6 +871,20 @@ open_tx_iface(const char *key, const char *value, void *extra_args)
 	return open_iface(key, value, extra_args);
 }
 
+static int
+select_phy_mac(const char *key __rte_unused, const char *value,
+		void *extra_args)
+{
+	if (extra_args) {
+		const int phy_mac = atoi(value);
+		int *enable_phy_mac = extra_args;
+
+		if (phy_mac)
+			*enable_phy_mac = 1;
+	}
+	return 0;
+}
+
 static struct rte_vdev_driver pmd_pcap_drv;
 
 static int
@@ -894,6 +920,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
 	(*internals)->eth_addr = (struct ether_addr) {
 		.addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
 	};
+	(*internals)->phy_mac = 0;
 	data = (*eth_dev)->data;
 	data->nb_rx_queues = (uint16_t)nb_rx_queues;
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
@@ -909,12 +936,96 @@ pmd_init_internals(struct rte_vdev_device *vdev,
 	return 0;
 }
 
+static int
+eth_pcap_update_mac(const char *if_name, struct rte_eth_dev *eth_dev,
+		const unsigned int numa_node)
+{
+#if defined(RTE_EXEC_ENV_LINUXAPP)
+	void *mac_addrs;
+	struct ifreq ifr;
+	int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+	if (if_fd == -1)
+		return -1;
+
+	rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+	if (ioctl(if_fd, SIOCGIFHWADDR, &ifr)) {
+		close(if_fd);
+		return -1;
+	}
+
+	mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0, numa_node);
+	if (!mac_addrs) {
+		close(if_fd);
+		return -1;
+	}
+
+	PMD_LOG(INFO, "Setting phy MAC for %s", if_name);
+	eth_dev->data->mac_addrs = mac_addrs;
+	rte_memcpy(eth_dev->data->mac_addrs[0].addr_bytes,
+			ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
+
+	close(if_fd);
+
+	return 0;
+
+#elif defined(RTE_EXEC_ENV_BSDAPP)
+	void *mac_addrs;
+	struct if_msghdr *ifm;
+	struct sockaddr_dl *sdl;
+	int mib[6];
+	size_t len = 0;
+	char *buf;
+
+	mib[0] = CTL_NET;
+	mib[1] = AF_ROUTE;
+	mib[2] = 0;
+	mib[3] = AF_LINK;
+	mib[4] = NET_RT_IFLIST;
+	mib[5] = if_nametoindex(if_name);
+
+	if (sysctl(mib, 6, NULL, &len, NULL, 0) < 0)
+		return -1;
+
+	if (len == 0)
+		return -1;
+
+	buf = rte_malloc(NULL, len, 0);
+	if (!buf)
+		return -1;
+
+	if (sysctl(mib, 6, buf, &len, NULL, 0) < 0) {
+		rte_free(buf);
+		return -1;
+	}
+	ifm = (struct if_msghdr *)buf;
+	sdl = (struct sockaddr_dl *)(ifm + 1);
+
+	mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0, numa_node);
+	if (!mac_addrs) {
+		rte_free(buf);
+		return -1;
+	}
+
+	PMD_LOG(INFO, "Setting phy MAC for %s", if_name);
+	eth_dev->data->mac_addrs = mac_addrs;
+	rte_memcpy(eth_dev->data->mac_addrs[0].addr_bytes,
+			LLADDR(sdl), ETHER_ADDR_LEN);
+
+	rte_free(buf);
+
+	return 0;
+#else
+	return -1;
+#endif
+}
+
 static int
 eth_from_pcaps_common(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
 		struct rte_kvargs *kvlist, struct pmd_internals **internals,
-		struct rte_eth_dev **eth_dev)
+		const int phy_mac, struct rte_eth_dev **eth_dev)
 {
 	struct rte_kvargs_pair *pair = NULL;
 	unsigned int k_idx;
@@ -960,6 +1071,14 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
 	else
 		(*internals)->if_index = if_nametoindex(pair->value);
 
+	/* phy_mac arg is applied only only if "iface" devarg is provided */
+	if (phy_mac && pair) {
+		int ret = eth_pcap_update_mac(pair->value, *eth_dev,
+				vdev->device.numa_node);
+		if (ret == 0)
+			(*internals)->phy_mac = 1;
+	}
+
 	return 0;
 }
 
@@ -967,7 +1086,7 @@ static int
 eth_from_pcaps(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
-		struct rte_kvargs *kvlist, int single_iface,
+		struct rte_kvargs *kvlist, int single_iface, int phy_mac,
 		unsigned int using_dumpers)
 {
 	struct pmd_internals *internals = NULL;
@@ -975,7 +1094,8 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	int ret;
 
 	ret = eth_from_pcaps_common(vdev, rx_queues, nb_rx_queues,
-		tx_queues, nb_tx_queues, kvlist, &internals, &eth_dev);
+			tx_queues, nb_tx_queues, kvlist,
+			&internals, phy_mac, &eth_dev);
 
 	if (ret < 0)
 		return ret;
@@ -1004,6 +1124,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 	struct pmd_devargs dumpers = {0};
 	struct rte_eth_dev *eth_dev;
 	int single_iface = 0;
+	int phy_mac = 0;
 	int ret;
 
 	name = rte_vdev_device_name(dev);
@@ -1039,7 +1160,11 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 
 		ret = rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG,
 				&open_rx_tx_iface, &pcaps);
+		if (ret < 0)
+			goto free_kvlist;
 
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
+				&select_phy_mac, &phy_mac);
 		if (ret < 0)
 			goto free_kvlist;
 
@@ -1089,7 +1214,8 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 
 create_eth:
 	ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
-		dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
+			dumpers.num_of_queue, kvlist, single_iface,
+			phy_mac, is_tx_pcap);
 
 free_kvlist:
 	rte_kvargs_free(kvlist);
@@ -1100,6 +1226,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 static int
 pmd_pcap_remove(struct rte_vdev_device *dev)
 {
+	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
 
 	PMD_LOG(INFO, "Closing pcap ethdev on numa socket %d",
@@ -1113,6 +1240,10 @@ pmd_pcap_remove(struct rte_vdev_device *dev)
 	if (eth_dev == NULL)
 		return -1;
 
+	internals = eth_dev->data->dev_private;
+	if (internals && internals->phy_mac)
+		rte_free(eth_dev->data->mac_addrs);
+
 	rte_free(eth_dev->data->dev_private);
 
 	rte_eth_dev_release_port(eth_dev);
@@ -1133,7 +1264,8 @@ RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
 	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
 	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
 	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
-	ETH_PCAP_IFACE_ARG "=<ifc>");
+	ETH_PCAP_IFACE_ARG "=<ifc> "
+	ETH_PCAP_PHY_MAC_ARG "=<int>");
 
 RTE_INIT(eth_pcap_init_log)
 {
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v8] net/pcap: physical interface MAC address support
  2018-10-05 20:27   ` [dpdk-dev] [PATCH v7] " Ferruh Yigit
@ 2018-10-06  0:49     ` Ferruh Yigit
  2018-10-08 12:14       ` Ferruh Yigit
  2018-10-09  4:30       ` Kuusisaari, Juhamatti (Infinera - FI/Espoo)
  0 siblings, 2 replies; 11+ messages in thread
From: Ferruh Yigit @ 2018-10-06  0:49 UTC (permalink / raw)
  To: Bruce Richardson, John McNamara, Marko Kovacevic
  Cc: dev, Ferruh Yigit, Juhamatti Kuusisaari, Juhamatti Kuusisaari

From: Juhamatti Kuusisaari <Juhamatti.Kuusisaari@coriant.com>

At the moment, PCAP interfaces use dummy MAC by default. This change
adds support for selecting PCAP physical interface MAC with phy_mac=1
devarg. This allows to setup packet flows using the physical interface
MAC.

Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v7:
* Add internal->phy_mac to be able to free data->mac_addrs
* code review comments applied
* doc format updates
* NOTE: FreeBSD functionality not tested

v8:
* don't access kvlist internals directly
* store phy_mac in pmd_devargs instead of passing as arg to function
---
 doc/guides/nics/pcap_ring.rst          |  10 ++
 doc/guides/rel_notes/release_18_11.rst |   4 +
 drivers/net/pcap/rte_eth_pcap.c        | 163 +++++++++++++++++++++----
 3 files changed, 156 insertions(+), 21 deletions(-)

diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst
index 879e5430f..c1ef9196b 100644
--- a/doc/guides/nics/pcap_ring.rst
+++ b/doc/guides/nics/pcap_ring.rst
@@ -96,6 +96,16 @@ The different stream types are:
 
         iface=eth0
 
+Runtime Config Options
+^^^^^^^^^^^^^^^^^^^^^^
+
+- Use PCAP interface physical MAC
+
+ In case ``iface=`` configuration is set, user may want to use the selected interface's physical MAC
+ address. This can be done with a ``devarg`` ``phy_mac``, for example::
+
+   --vdev 'net_pcap0,iface=eth0,phy_mac=1'
+
 Examples of Usage
 ^^^^^^^^^^^^^^^^^
 
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 89ca3317f..491d97f13 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -98,6 +98,10 @@ New Features
   * Support for runtime Rx and Tx queues setup.
   * Support multicast MAC address set.
 
+* **Added a devarg to use PCAP interface physical MAC address.**
+  A new devarg ``phy_mac`` was introduced to allow users to use physical
+  MAC address of the selected PCAP interface.
+
 * **Added Event Ethernet Tx Adapter.**
 
   Added event ethernet Tx adapter library that  provides configuration and
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 8736010f0..51d405116 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -7,6 +7,14 @@
 #include <time.h>
 
 #include <net/if.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+#if defined(RTE_EXEC_ENV_BSDAPP)
+#include <sys/sysctl.h>
+#include <net/if_dl.h>
+#endif
 
 #include <pcap.h>
 
@@ -17,6 +25,7 @@
 #include <rte_malloc.h>
 #include <rte_mbuf.h>
 #include <rte_bus_vdev.h>
+#include <rte_string_fns.h>
 
 #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
 #define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
@@ -29,6 +38,7 @@
 #define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
 #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
 #define ETH_PCAP_IFACE_ARG    "iface"
+#define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
 
 #define ETH_PCAP_ARG_MAXLEN	64
 
@@ -70,6 +80,7 @@ struct pmd_internals {
 	struct ether_addr eth_addr;
 	int if_index;
 	int single_iface;
+	int phy_mac;
 };
 
 struct pmd_devargs {
@@ -80,6 +91,7 @@ struct pmd_devargs {
 		const char *name;
 		const char *type;
 	} queue[RTE_PMD_PCAP_MAX_QUEUES];
+	int phy_mac;
 };
 
 static const char *valid_arguments[] = {
@@ -89,6 +101,7 @@ static const char *valid_arguments[] = {
 	ETH_PCAP_RX_IFACE_IN_ARG,
 	ETH_PCAP_TX_IFACE_ARG,
 	ETH_PCAP_IFACE_ARG,
+	ETH_PCAP_PHY_MAC_ARG,
 	NULL
 };
 
@@ -859,6 +872,20 @@ open_tx_iface(const char *key, const char *value, void *extra_args)
 	return open_iface(key, value, extra_args);
 }
 
+static int
+select_phy_mac(const char *key __rte_unused, const char *value,
+		void *extra_args)
+{
+	if (extra_args) {
+		const int phy_mac = atoi(value);
+		int *enable_phy_mac = extra_args;
+
+		if (phy_mac)
+			*enable_phy_mac = 1;
+	}
+	return 0;
+}
+
 static struct rte_vdev_driver pmd_pcap_drv;
 
 static int
@@ -894,6 +921,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
 	(*internals)->eth_addr = (struct ether_addr) {
 		.addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
 	};
+	(*internals)->phy_mac = 0;
 	data = (*eth_dev)->data;
 	data->nb_rx_queues = (uint16_t)nb_rx_queues;
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
@@ -909,15 +937,96 @@ pmd_init_internals(struct rte_vdev_device *vdev,
 	return 0;
 }
 
+static int
+eth_pcap_update_mac(const char *if_name, struct rte_eth_dev *eth_dev,
+		const unsigned int numa_node)
+{
+#if defined(RTE_EXEC_ENV_LINUXAPP)
+	void *mac_addrs;
+	struct ifreq ifr;
+	int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+	if (if_fd == -1)
+		return -1;
+
+	rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+	if (ioctl(if_fd, SIOCGIFHWADDR, &ifr)) {
+		close(if_fd);
+		return -1;
+	}
+
+	mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0, numa_node);
+	if (!mac_addrs) {
+		close(if_fd);
+		return -1;
+	}
+
+	PMD_LOG(INFO, "Setting phy MAC for %s", if_name);
+	eth_dev->data->mac_addrs = mac_addrs;
+	rte_memcpy(eth_dev->data->mac_addrs[0].addr_bytes,
+			ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
+
+	close(if_fd);
+
+	return 0;
+
+#elif defined(RTE_EXEC_ENV_BSDAPP)
+	void *mac_addrs;
+	struct if_msghdr *ifm;
+	struct sockaddr_dl *sdl;
+	int mib[6];
+	size_t len = 0;
+	char *buf;
+
+	mib[0] = CTL_NET;
+	mib[1] = AF_ROUTE;
+	mib[2] = 0;
+	mib[3] = AF_LINK;
+	mib[4] = NET_RT_IFLIST;
+	mib[5] = if_nametoindex(if_name);
+
+	if (sysctl(mib, 6, NULL, &len, NULL, 0) < 0)
+		return -1;
+
+	if (len == 0)
+		return -1;
+
+	buf = rte_malloc(NULL, len, 0);
+	if (!buf)
+		return -1;
+
+	if (sysctl(mib, 6, buf, &len, NULL, 0) < 0) {
+		rte_free(buf);
+		return -1;
+	}
+	ifm = (struct if_msghdr *)buf;
+	sdl = (struct sockaddr_dl *)(ifm + 1);
+
+	mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0, numa_node);
+	if (!mac_addrs) {
+		rte_free(buf);
+		return -1;
+	}
+
+	PMD_LOG(INFO, "Setting phy MAC for %s", if_name);
+	eth_dev->data->mac_addrs = mac_addrs;
+	rte_memcpy(eth_dev->data->mac_addrs[0].addr_bytes,
+			LLADDR(sdl), ETHER_ADDR_LEN);
+
+	rte_free(buf);
+
+	return 0;
+#else
+	return -1;
+#endif
+}
+
 static int
 eth_from_pcaps_common(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
-		struct rte_kvargs *kvlist, struct pmd_internals **internals,
-		struct rte_eth_dev **eth_dev)
+		struct pmd_internals **internals, struct rte_eth_dev **eth_dev)
 {
-	struct rte_kvargs_pair *pair = NULL;
-	unsigned int k_idx;
 	unsigned int i;
 
 	/* do some parameter checking */
@@ -949,17 +1058,6 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
 		snprintf(tx->type, sizeof(tx->type), "%s", queue->type);
 	}
 
-	for (k_idx = 0; k_idx < kvlist->count; k_idx++) {
-		pair = &kvlist->pairs[k_idx];
-		if (strstr(pair->key, ETH_PCAP_IFACE_ARG) != NULL)
-			break;
-	}
-
-	if (pair == NULL)
-		(*internals)->if_index = 0;
-	else
-		(*internals)->if_index = if_nametoindex(pair->value);
-
 	return 0;
 }
 
@@ -967,15 +1065,14 @@ static int
 eth_from_pcaps(struct rte_vdev_device *vdev,
 		struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
 		struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
-		struct rte_kvargs *kvlist, int single_iface,
-		unsigned int using_dumpers)
+		int single_iface, unsigned int using_dumpers)
 {
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
 	int ret;
 
 	ret = eth_from_pcaps_common(vdev, rx_queues, nb_rx_queues,
-		tx_queues, nb_tx_queues, kvlist, &internals, &eth_dev);
+		tx_queues, nb_tx_queues, &internals, &eth_dev);
 
 	if (ret < 0)
 		return ret;
@@ -983,6 +1080,18 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	/* store weather we are using a single interface for rx/tx or not */
 	internals->single_iface = single_iface;
 
+	if (single_iface) {
+		internals->if_index = if_nametoindex(rx_queues->queue[0].name);
+
+		/* phy_mac arg is applied only only if "iface" devarg is provided */
+		if (rx_queues->phy_mac) {
+			int ret = eth_pcap_update_mac(rx_queues->queue[0].name,
+					eth_dev, vdev->device.numa_node);
+			if (ret == 0)
+				internals->phy_mac = 1;
+		}
+	}
+
 	eth_dev->rx_pkt_burst = eth_pcap_rx;
 
 	if (using_dumpers)
@@ -1039,12 +1148,18 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 
 		ret = rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG,
 				&open_rx_tx_iface, &pcaps);
-
 		if (ret < 0)
 			goto free_kvlist;
 
 		dumpers.queue[0] = pcaps.queue[0];
 
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
+				&select_phy_mac, &pcaps.phy_mac);
+		if (ret < 0)
+			goto free_kvlist;
+
+		dumpers.phy_mac = pcaps.phy_mac;
+
 		single_iface = 1;
 		pcaps.num_of_queue = 1;
 		dumpers.num_of_queue = 1;
@@ -1089,7 +1204,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 
 create_eth:
 	ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
-		dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
+		dumpers.num_of_queue, single_iface, is_tx_pcap);
 
 free_kvlist:
 	rte_kvargs_free(kvlist);
@@ -1100,6 +1215,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 static int
 pmd_pcap_remove(struct rte_vdev_device *dev)
 {
+	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
 
 	PMD_LOG(INFO, "Closing pcap ethdev on numa socket %d",
@@ -1113,6 +1229,10 @@ pmd_pcap_remove(struct rte_vdev_device *dev)
 	if (eth_dev == NULL)
 		return -1;
 
+	internals = eth_dev->data->dev_private;
+	if (internals && internals->phy_mac)
+		rte_free(eth_dev->data->mac_addrs);
+
 	rte_free(eth_dev->data->dev_private);
 
 	rte_eth_dev_release_port(eth_dev);
@@ -1133,7 +1253,8 @@ RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
 	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
 	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
 	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
-	ETH_PCAP_IFACE_ARG "=<ifc>");
+	ETH_PCAP_IFACE_ARG "=<ifc> "
+	ETH_PCAP_PHY_MAC_ARG "=<int>");
 
 RTE_INIT(eth_pcap_init_log)
 {
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v8] net/pcap: physical interface MAC address support
  2018-10-06  0:49     ` [dpdk-dev] [PATCH v8] " Ferruh Yigit
@ 2018-10-08 12:14       ` Ferruh Yigit
  2018-10-09  4:30       ` Kuusisaari, Juhamatti (Infinera - FI/Espoo)
  1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2018-10-08 12:14 UTC (permalink / raw)
  To: Bruce Richardson, John McNamara, Marko Kovacevic
  Cc: dev, Juhamatti Kuusisaari

On 10/6/2018 1:49 AM, Ferruh Yigit wrote:
> From: Juhamatti Kuusisaari <Juhamatti.Kuusisaari@coriant.com>
> 
> At the moment, PCAP interfaces use dummy MAC by default. This change
> adds support for selecting PCAP physical interface MAC with phy_mac=1
> devarg. This allows to setup packet flows using the physical interface
> MAC.
> 
> Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> v7:
> * Add internal->phy_mac to be able to free data->mac_addrs
> * code review comments applied
> * doc format updates
> * NOTE: FreeBSD functionality not tested

Able to test on FreeBSD, works fine.

> 
> v8:
> * don't access kvlist internals directly
> * store phy_mac in pmd_devargs instead of passing as arg to function

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v8] net/pcap: physical interface MAC address support
  2018-10-06  0:49     ` [dpdk-dev] [PATCH v8] " Ferruh Yigit
  2018-10-08 12:14       ` Ferruh Yigit
@ 2018-10-09  4:30       ` Kuusisaari, Juhamatti (Infinera - FI/Espoo)
  2018-10-09 12:04         ` Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Kuusisaari, Juhamatti (Infinera - FI/Espoo) @ 2018-10-09  4:30 UTC (permalink / raw)
  To: dev



> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Saturday, October 06, 2018 3:50 AM
> To: Bruce Richardson <bruce.richardson@intel.com>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; Ferruh Yigit <ferruh.yigit@intel.com>; Juhamatti
> Kuusisaari <Juhamatti.Kuusisaari@coriant.com>; Juhamatti Kuusisaari
> <juhamatti.kuusisaari@coriant.com>
> Subject: [PATCH v8] net/pcap: physical interface MAC address support
> 
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you recognize the sender and know the
> content is safe.
> 
> 
> From: Juhamatti Kuusisaari <Juhamatti.Kuusisaari@coriant.com>
> 
> At the moment, PCAP interfaces use dummy MAC by default. This change
> adds support for selecting PCAP physical interface MAC with phy_mac=1
> devarg. This allows to setup packet flows using the physical interface
> MAC.
> 
> Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> v7:
> * Add internal->phy_mac to be able to free data->mac_addrs
> * code review comments applied
> * doc format updates
> * NOTE: FreeBSD functionality not tested
> 
> v8:
> * don't access kvlist internals directly
> * store phy_mac in pmd_devargs instead of passing as arg to function
> ---
>  doc/guides/nics/pcap_ring.rst          |  10 ++
>  doc/guides/rel_notes/release_18_11.rst |   4 +
>  drivers/net/pcap/rte_eth_pcap.c        | 163 +++++++++++++++++++++----
>  3 files changed, 156 insertions(+), 21 deletions(-)
> 
> diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst
> index 879e5430f..c1ef9196b 100644
> --- a/doc/guides/nics/pcap_ring.rst
> +++ b/doc/guides/nics/pcap_ring.rst
> @@ -96,6 +96,16 @@ The different stream types are:
> 
>          iface=eth0
> 
> +Runtime Config Options
> +^^^^^^^^^^^^^^^^^^^^^^
> +
> +- Use PCAP interface physical MAC
> +
> + In case ``iface=`` configuration is set, user may want to use the selected
> interface's physical MAC
> + address. This can be done with a ``devarg`` ``phy_mac``, for example::
> +
> +   --vdev 'net_pcap0,iface=eth0,phy_mac=1'
> +
>  Examples of Usage
>  ^^^^^^^^^^^^^^^^^
> 
> diff --git a/doc/guides/rel_notes/release_18_11.rst
> b/doc/guides/rel_notes/release_18_11.rst
> index 89ca3317f..491d97f13 100644
> --- a/doc/guides/rel_notes/release_18_11.rst
> +++ b/doc/guides/rel_notes/release_18_11.rst
> @@ -98,6 +98,10 @@ New Features
>    * Support for runtime Rx and Tx queues setup.
>    * Support multicast MAC address set.
> 
> +* **Added a devarg to use PCAP interface physical MAC address.**
> +  A new devarg ``phy_mac`` was introduced to allow users to use physical
> +  MAC address of the selected PCAP interface.
> +
>  * **Added Event Ethernet Tx Adapter.**
> 
>    Added event ethernet Tx adapter library that  provides configuration and
> diff --git a/drivers/net/pcap/rte_eth_pcap.c
> b/drivers/net/pcap/rte_eth_pcap.c
> index 8736010f0..51d405116 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -7,6 +7,14 @@
>  #include <time.h>
> 
>  #include <net/if.h>
> +#include <sys/socket.h>
> +#include <sys/ioctl.h>
> +#include <unistd.h>
> +
> +#if defined(RTE_EXEC_ENV_BSDAPP)
> +#include <sys/sysctl.h>
> +#include <net/if_dl.h>
> +#endif
> 
>  #include <pcap.h>
> 
> @@ -17,6 +25,7 @@
>  #include <rte_malloc.h>
>  #include <rte_mbuf.h>
>  #include <rte_bus_vdev.h>
> +#include <rte_string_fns.h>
> 
>  #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
>  #define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
> @@ -29,6 +38,7 @@
>  #define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
>  #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
>  #define ETH_PCAP_IFACE_ARG    "iface"
> +#define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
> 
>  #define ETH_PCAP_ARG_MAXLEN    64
> 
> @@ -70,6 +80,7 @@ struct pmd_internals {
>         struct ether_addr eth_addr;
>         int if_index;
>         int single_iface;
> +       int phy_mac;
>  };
> 
>  struct pmd_devargs {
> @@ -80,6 +91,7 @@ struct pmd_devargs {
>                 const char *name;
>                 const char *type;
>         } queue[RTE_PMD_PCAP_MAX_QUEUES];
> +       int phy_mac;
>  };
> 
>  static const char *valid_arguments[] = {
> @@ -89,6 +101,7 @@ static const char *valid_arguments[] = {
>         ETH_PCAP_RX_IFACE_IN_ARG,
>         ETH_PCAP_TX_IFACE_ARG,
>         ETH_PCAP_IFACE_ARG,
> +       ETH_PCAP_PHY_MAC_ARG,
>         NULL
>  };
> 
> @@ -859,6 +872,20 @@ open_tx_iface(const char *key, const char *value,
> void *extra_args)
>         return open_iface(key, value, extra_args);
>  }
> 
> +static int
> +select_phy_mac(const char *key __rte_unused, const char *value,
> +               void *extra_args)
> +{
> +       if (extra_args) {
> +               const int phy_mac = atoi(value);
> +               int *enable_phy_mac = extra_args;
> +
> +               if (phy_mac)
> +                       *enable_phy_mac = 1;
> +       }
> +       return 0;
> +}
> +
>  static struct rte_vdev_driver pmd_pcap_drv;
> 
>  static int
> @@ -894,6 +921,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
>         (*internals)->eth_addr = (struct ether_addr) {
>                 .addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
>         };
> +       (*internals)->phy_mac = 0;
>         data = (*eth_dev)->data;
>         data->nb_rx_queues = (uint16_t)nb_rx_queues;
>         data->nb_tx_queues = (uint16_t)nb_tx_queues;
> @@ -909,15 +937,96 @@ pmd_init_internals(struct rte_vdev_device *vdev,
>         return 0;
>  }
> 
> +static int
> +eth_pcap_update_mac(const char *if_name, struct rte_eth_dev *eth_dev,
> +               const unsigned int numa_node)
> +{
> +#if defined(RTE_EXEC_ENV_LINUXAPP)
> +       void *mac_addrs;
> +       struct ifreq ifr;
> +       int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
> +
> +       if (if_fd == -1)
> +               return -1;
> +
> +       rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
> +       if (ioctl(if_fd, SIOCGIFHWADDR, &ifr)) {
> +               close(if_fd);
> +               return -1;
> +       }
> +
> +       mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0,
> numa_node);
> +       if (!mac_addrs) {
> +               close(if_fd);
> +               return -1;
> +       }
> +
> +       PMD_LOG(INFO, "Setting phy MAC for %s", if_name);
> +       eth_dev->data->mac_addrs = mac_addrs;
> +       rte_memcpy(eth_dev->data->mac_addrs[0].addr_bytes,
> +                       ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
> +
> +       close(if_fd);
> +
> +       return 0;
> +
> +#elif defined(RTE_EXEC_ENV_BSDAPP)
> +       void *mac_addrs;
> +       struct if_msghdr *ifm;
> +       struct sockaddr_dl *sdl;
> +       int mib[6];
> +       size_t len = 0;
> +       char *buf;
> +
> +       mib[0] = CTL_NET;
> +       mib[1] = AF_ROUTE;
> +       mib[2] = 0;
> +       mib[3] = AF_LINK;
> +       mib[4] = NET_RT_IFLIST;
> +       mib[5] = if_nametoindex(if_name);
> +
> +       if (sysctl(mib, 6, NULL, &len, NULL, 0) < 0)
> +               return -1;
> +
> +       if (len == 0)
> +               return -1;
> +
> +       buf = rte_malloc(NULL, len, 0);
> +       if (!buf)
> +               return -1;
> +
> +       if (sysctl(mib, 6, buf, &len, NULL, 0) < 0) {
> +               rte_free(buf);
> +               return -1;
> +       }
> +       ifm = (struct if_msghdr *)buf;
> +       sdl = (struct sockaddr_dl *)(ifm + 1);
> +
> +       mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0,
> numa_node);
> +       if (!mac_addrs) {
> +               rte_free(buf);
> +               return -1;
> +       }
> +
> +       PMD_LOG(INFO, "Setting phy MAC for %s", if_name);
> +       eth_dev->data->mac_addrs = mac_addrs;
> +       rte_memcpy(eth_dev->data->mac_addrs[0].addr_bytes,
> +                       LLADDR(sdl), ETHER_ADDR_LEN);
> +
> +       rte_free(buf);
> +
> +       return 0;
> +#else
> +       return -1;
> +#endif
> +}
> +
>  static int
>  eth_from_pcaps_common(struct rte_vdev_device *vdev,
>                 struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
>                 struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
> -               struct rte_kvargs *kvlist, struct pmd_internals **internals,
> -               struct rte_eth_dev **eth_dev)
> +               struct pmd_internals **internals, struct rte_eth_dev **eth_dev)
>  {
> -       struct rte_kvargs_pair *pair = NULL;
> -       unsigned int k_idx;
>         unsigned int i;
> 
>         /* do some parameter checking */
> @@ -949,17 +1058,6 @@ eth_from_pcaps_common(struct rte_vdev_device
> *vdev,
>                 snprintf(tx->type, sizeof(tx->type), "%s", queue->type);
>         }
> 
> -       for (k_idx = 0; k_idx < kvlist->count; k_idx++) {
> -               pair = &kvlist->pairs[k_idx];
> -               if (strstr(pair->key, ETH_PCAP_IFACE_ARG) != NULL)
> -                       break;
> -       }
> -
> -       if (pair == NULL)
> -               (*internals)->if_index = 0;
> -       else
> -               (*internals)->if_index = if_nametoindex(pair->value);
> -
>         return 0;
>  }
> 
> @@ -967,15 +1065,14 @@ static int
>  eth_from_pcaps(struct rte_vdev_device *vdev,
>                 struct pmd_devargs *rx_queues, const unsigned int nb_rx_queues,
>                 struct pmd_devargs *tx_queues, const unsigned int nb_tx_queues,
> -               struct rte_kvargs *kvlist, int single_iface,
> -               unsigned int using_dumpers)
> +               int single_iface, unsigned int using_dumpers)
>  {
>         struct pmd_internals *internals = NULL;
>         struct rte_eth_dev *eth_dev = NULL;
>         int ret;
> 
>         ret = eth_from_pcaps_common(vdev, rx_queues, nb_rx_queues,
> -               tx_queues, nb_tx_queues, kvlist, &internals, &eth_dev);
> +               tx_queues, nb_tx_queues, &internals, &eth_dev);
> 
>         if (ret < 0)
>                 return ret;
> @@ -983,6 +1080,18 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
>         /* store weather we are using a single interface for rx/tx or not */
>         internals->single_iface = single_iface;
> 
> +       if (single_iface) {
> +               internals->if_index = if_nametoindex(rx_queues->queue[0].name);
> +
> +               /* phy_mac arg is applied only only if "iface" devarg is provided */
> +               if (rx_queues->phy_mac) {
> +                       int ret = eth_pcap_update_mac(rx_queues->queue[0].name,
> +                                       eth_dev, vdev->device.numa_node);
> +                       if (ret == 0)
> +                               internals->phy_mac = 1;
> +               }
> +       }
> +
>         eth_dev->rx_pkt_burst = eth_pcap_rx;
> 
>         if (using_dumpers)
> @@ -1039,12 +1148,18 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
> 
>                 ret = rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG,
>                                 &open_rx_tx_iface, &pcaps);
> -
>                 if (ret < 0)
>                         goto free_kvlist;
> 
>                 dumpers.queue[0] = pcaps.queue[0];
> 
> +               ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
> +                               &select_phy_mac, &pcaps.phy_mac);
> +               if (ret < 0)
> +                       goto free_kvlist;
> +
> +               dumpers.phy_mac = pcaps.phy_mac;
> +
>                 single_iface = 1;
>                 pcaps.num_of_queue = 1;
>                 dumpers.num_of_queue = 1;
> @@ -1089,7 +1204,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
> 
>  create_eth:
>         ret = eth_from_pcaps(dev, &pcaps, pcaps.num_of_queue, &dumpers,
> -               dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
> +               dumpers.num_of_queue, single_iface, is_tx_pcap);
> 
>  free_kvlist:
>         rte_kvargs_free(kvlist);
> @@ -1100,6 +1215,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  static int
>  pmd_pcap_remove(struct rte_vdev_device *dev)
>  {
> +       struct pmd_internals *internals = NULL;
>         struct rte_eth_dev *eth_dev = NULL;
> 
>         PMD_LOG(INFO, "Closing pcap ethdev on numa socket %d",
> @@ -1113,6 +1229,10 @@ pmd_pcap_remove(struct rte_vdev_device *dev)
>         if (eth_dev == NULL)
>                 return -1;
> 
> +       internals = eth_dev->data->dev_private;
> +       if (internals && internals->phy_mac)
> +               rte_free(eth_dev->data->mac_addrs);
> +
>         rte_free(eth_dev->data->dev_private);
> 
>         rte_eth_dev_release_port(eth_dev);
> @@ -1133,7 +1253,8 @@ RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
>         ETH_PCAP_RX_IFACE_ARG "=<ifc> "
>         ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
>         ETH_PCAP_TX_IFACE_ARG "=<ifc> "
> -       ETH_PCAP_IFACE_ARG "=<ifc>");
> +       ETH_PCAP_IFACE_ARG "=<ifc> "
> +       ETH_PCAP_PHY_MAC_ARG "=<int>");

Acked-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>

>  RTE_INIT(eth_pcap_init_log)
>  {
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v8] net/pcap: physical interface MAC address support
  2018-10-09  4:30       ` Kuusisaari, Juhamatti (Infinera - FI/Espoo)
@ 2018-10-09 12:04         ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2018-10-09 12:04 UTC (permalink / raw)
  To: Kuusisaari, Juhamatti (Infinera - FI/Espoo), dev

On 10/9/2018 5:30 AM, Kuusisaari, Juhamatti (Infinera - FI/Espoo) wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Saturday, October 06, 2018 3:50 AM
>> To: Bruce Richardson <bruce.richardson@intel.com>; John McNamara
>> <john.mcnamara@intel.com>; Marko Kovacevic
>> <marko.kovacevic@intel.com>
>> Cc: dev@dpdk.org; Ferruh Yigit <ferruh.yigit@intel.com>; Juhamatti
>> Kuusisaari <Juhamatti.Kuusisaari@coriant.com>; Juhamatti Kuusisaari
>> <juhamatti.kuusisaari@coriant.com>
>> Subject: [PATCH v8] net/pcap: physical interface MAC address support
>>
>> CAUTION: This email originated from outside of the organization. Do not click
>> links or open attachments unless you recognize the sender and know the
>> content is safe.
>>
>>
>> From: Juhamatti Kuusisaari <Juhamatti.Kuusisaari@coriant.com>
>>
>> At the moment, PCAP interfaces use dummy MAC by default. This change
>> adds support for selecting PCAP physical interface MAC with phy_mac=1
>> devarg. This allows to setup packet flows using the physical interface
>> MAC.
>>
>> Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

> Acked-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-10-09 12:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-06 16:56 [dpdk-dev] [PATCH v5] net/pcap: physical interface MAC address support Juhamatti Kuusisaari
2018-09-10 12:03 ` Ferruh Yigit
2018-09-10 17:20   ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
2018-09-10 16:55 ` [dpdk-dev] [PATCH v6] " Juhamatti Kuusisaari
2018-09-11 15:35   ` Ferruh Yigit
2018-10-01  7:30     ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
2018-10-05 20:27   ` [dpdk-dev] [PATCH v7] " Ferruh Yigit
2018-10-06  0:49     ` [dpdk-dev] [PATCH v8] " Ferruh Yigit
2018-10-08 12:14       ` Ferruh Yigit
2018-10-09  4:30       ` Kuusisaari, Juhamatti (Infinera - FI/Espoo)
2018-10-09 12:04         ` Ferruh Yigit

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).