[dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support

[dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support

[Juhamatti Kuusisaari]

Support for PCAP MAC address using physical interface MAC.
Support for getting proper link status, speed and duplex.

Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
---
 config/common_base              |  1 +
 drivers/net/pcap/rte_eth_pcap.c | 52 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/config/common_base b/config/common_base
index c2b0d91..9804585 100644
--- a/config/common_base
+++ b/config/common_base
@@ -410,6 +410,7 @@ CONFIG_RTE_LIBRTE_PMD_NULL=y
 # Compile software PMD backed by PCAP files
 #
 CONFIG_RTE_LIBRTE_PMD_PCAP=n
+CONFIG_RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT=n
 
 #
 # Compile example software rings based PMD
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index c1571e1..d2aba1c 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -7,6 +7,13 @@
 #include <time.h>
 
 #include <net/if.h>
+#ifdef RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <string.h>
+#include <linux/ethtool.h>
+#include <linux/sockios.h>
+#endif /* RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT */
 
 #include <pcap.h>
 
@@ -67,6 +74,10 @@ struct pmd_internals {
 	struct pcap_tx_queue tx_queue[RTE_PMD_PCAP_MAX_QUEUES];
 	int if_index;
 	int single_iface;
+#ifdef RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT
+	const char *if_name;
+	int if_fd;
+#endif /* RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT */
 };
 
 struct pmd_devargs {
@@ -602,6 +613,27 @@ static int
 eth_link_update(struct rte_eth_dev *dev __rte_unused,
 		int wait_to_complete __rte_unused)
 {
+#ifdef RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT
+	struct ifreq ifr;
+	struct ethtool_cmd cmd;
+	struct pmd_internals *internals = dev->data->dev_private;
+
+	if (internals->if_name && (internals->if_fd != -1)) {
+		/* Get link status, speed and duplex from the underlying interface */
+		strncpy(ifr.ifr_name, internals->if_name, sizeof(ifr.ifr_name)-1);
+		ifr.ifr_name[sizeof(ifr.ifr_name)-1] = 0;
+		if (!ioctl(internals->if_fd, SIOCGIFFLAGS, &ifr))
+			dev->data->dev_link.link_status = (ifr.ifr_flags & IFF_UP) ? 1 : 0;
+
+		cmd.cmd = ETHTOOL_GSET;
+		ifr.ifr_data = (void *)&cmd;
+		if (!ioctl(internals->if_fd, SIOCETHTOOL, &ifr)) {
+			dev->data->dev_link.link_speed = ethtool_cmd_speed(&cmd);
+			dev->data->dev_link.link_duplex =
+				cmd.duplex ? ETH_LINK_FULL_DUPLEX : ETH_LINK_HALF_DUPLEX;
+		}
+	}
+#endif /* RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT */
 	return 0;
 }
 
@@ -866,8 +898,26 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
 
 	if (pair == NULL)
 		(*internals)->if_index = 0;
-	else
+	else {
 		(*internals)->if_index = if_nametoindex(pair->value);
+#ifdef RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT
+		/* Use real interface mac addr, save name and fd for eth_link_update() */
+		(*internals)->if_name = strdup(pair->value);
+		(*internals)->if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+		if ((*internals)->if_fd != -1) {
+			struct ifreq ifr;
+			strncpy(ifr.ifr_name, pair->value, sizeof(ifr.ifr_name)-1);
+			ifr.ifr_name[sizeof(ifr.ifr_name)-1] = 0;
+			if (!ioctl((*internals)->if_fd, SIOCGIFHWADDR, &ifr)) {
+				(*eth_dev)->data->mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0, vdev->device.numa_node);
+				rte_memcpy((*eth_dev)->data->mac_addrs, ifr.ifr_addr.sa_data, ETHER_ADDR_LEN);
+			}
+		}
+#endif /* RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT */
+	}
+#ifdef RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT
+	eth_link_update((*eth_dev), 0);
+#endif /* RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT */
 
 	return 0;
 }
-- 
2.8.1

[dpdk-dev] [PATCH v2 2/2] net/pcap: physical interface MAC support

[Juhamatti Kuusisaari]

Support for PCAP MAC address using physical interface MAC.
Support for getting proper link status, speed and duplex.

Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
---
 v2:
 * Fix whitespace
---
 drivers/net/pcap/rte_eth_pcap.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index d2aba1c..4d6db55 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -619,18 +619,22 @@ eth_link_update(struct rte_eth_dev *dev __rte_unused,
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	if (internals->if_name && (internals->if_fd != -1)) {
-		/* Get link status, speed and duplex from the underlying interface */
-		strncpy(ifr.ifr_name, internals->if_name, sizeof(ifr.ifr_name)-1);
-		ifr.ifr_name[sizeof(ifr.ifr_name)-1] = 0;
+		/* Get link status, speed and duplex of the interface. */
+		strncpy(ifr.ifr_name, internals->if_name,
+				sizeof(ifr.ifr_name) - 1);
+		ifr.ifr_name[sizeof(ifr.ifr_name) - 1] = 0;
 		if (!ioctl(internals->if_fd, SIOCGIFFLAGS, &ifr))
-			dev->data->dev_link.link_status = (ifr.ifr_flags & IFF_UP) ? 1 : 0;
+			dev->data->dev_link.link_status =
+				(ifr.ifr_flags & IFF_UP) ? 1 : 0;
 
 		cmd.cmd = ETHTOOL_GSET;
 		ifr.ifr_data = (void *)&cmd;
 		if (!ioctl(internals->if_fd, SIOCETHTOOL, &ifr)) {
-			dev->data->dev_link.link_speed = ethtool_cmd_speed(&cmd);
+			dev->data->dev_link.link_speed =
+				ethtool_cmd_speed(&cmd);
 			dev->data->dev_link.link_duplex =
-				cmd.duplex ? ETH_LINK_FULL_DUPLEX : ETH_LINK_HALF_DUPLEX;
+				cmd.duplex ?
+				ETH_LINK_FULL_DUPLEX : ETH_LINK_HALF_DUPLEX;
 		}
 	}
 #endif /* RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT */
@@ -896,21 +900,30 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
 			break;
 	}
 
-	if (pair == NULL)
+	if (pair == NULL) {
 		(*internals)->if_index = 0;
-	else {
+	} else {
 		(*internals)->if_index = if_nametoindex(pair->value);
 #ifdef RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT
-		/* Use real interface mac addr, save name and fd for eth_link_update() */
+		/*
+		 * Use real interface mac addr, save name and
+		 * file descriptor for the update.
+		 */
 		(*internals)->if_name = strdup(pair->value);
 		(*internals)->if_fd = socket(AF_INET, SOCK_DGRAM, 0);
 		if ((*internals)->if_fd != -1) {
 			struct ifreq ifr;
-			strncpy(ifr.ifr_name, pair->value, sizeof(ifr.ifr_name)-1);
-			ifr.ifr_name[sizeof(ifr.ifr_name)-1] = 0;
+			strncpy(ifr.ifr_name, pair->value,
+					sizeof(ifr.ifr_name) - 1);
+			ifr.ifr_name[sizeof(ifr.ifr_name) - 1] = 0;
 			if (!ioctl((*internals)->if_fd, SIOCGIFHWADDR, &ifr)) {
-				(*eth_dev)->data->mac_addrs = rte_zmalloc_socket(NULL, ETHER_ADDR_LEN, 0, vdev->device.numa_node);
-				rte_memcpy((*eth_dev)->data->mac_addrs, ifr.ifr_addr.sa_data, ETHER_ADDR_LEN);
+				(*eth_dev)->data->mac_addrs =
+					rte_zmalloc_socket(NULL,
+							ETHER_ADDR_LEN, 0,
+							vdev->device.numa_node);
+				rte_memcpy((*eth_dev)->data->mac_addrs,
+						ifr.ifr_addr.sa_data,
+						ETHER_ADDR_LEN);
 			}
 		}
 #endif /* RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT */
-- 
2.8.1

Re: [dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support

[Ferruh Yigit]

On 4/17/2018 1:53 PM, Juhamatti Kuusisaari wrote:
> Support for PCAP MAC address using physical interface MAC.
> Support for getting proper link status, speed and duplex.
> 
> Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
> ---
>  config/common_base              |  1 +
>  drivers/net/pcap/rte_eth_pcap.c | 52 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/config/common_base b/config/common_base
> index c2b0d91..9804585 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -410,6 +410,7 @@ CONFIG_RTE_LIBRTE_PMD_NULL=y
>  # Compile software PMD backed by PCAP files
>  #
>  CONFIG_RTE_LIBRTE_PMD_PCAP=n
> +CONFIG_RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT=n

Hi Juhamatti,

Why a build time config option for this? Can we make it a runtime devarg?
Overall we are trying to reduce config options already and this seems no need to
be build time option at all.

btw, this is a little late in release cycle, so lets target this patch for next
release.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support

[Kuusisaari, Juhamatti]

Hello Ferruh,

> On 4/17/2018 1:53 PM, Juhamatti Kuusisaari wrote:
> > Support for PCAP MAC address using physical interface MAC.
> > Support for getting proper link status, speed and duplex.
> >
> > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
> > ---
> >  config/common_base              |  1 +
> >  drivers/net/pcap/rte_eth_pcap.c | 52
> > ++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/config/common_base b/config/common_base index
> > c2b0d91..9804585 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -410,6 +410,7 @@ CONFIG_RTE_LIBRTE_PMD_NULL=y  # Compile
> software
> > PMD backed by PCAP files  #  CONFIG_RTE_LIBRTE_PMD_PCAP=n
> > +CONFIG_RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT=n
> 
> Hi Juhamatti,
> 
> Why a build time config option for this? Can we make it a runtime devarg?

Sure, we can make it a devarg. Or do we even need that? Are there a lot of test dependencies that would need to be fixed if we have it enabled by default?

> Overall we are trying to reduce config options already and this seems no
> need to be build time option at all.
> 
> btw, this is a little late in release cycle, so lets target this patch for next
> release.

The patch is on top of net-next, this should be just fine.

> Thanks,
> ferruh

Thanks,
--
 Juhamatti

Re: [dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support

[Ferruh Yigit]

On 4/18/2018 5:35 AM, Kuusisaari, Juhamatti wrote:
> Hello Ferruh,
> 
>> On 4/17/2018 1:53 PM, Juhamatti Kuusisaari wrote:
>>> Support for PCAP MAC address using physical interface MAC.
>>> Support for getting proper link status, speed and duplex.
>>>
>>> Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
>>> ---
>>>  config/common_base              |  1 +
>>>  drivers/net/pcap/rte_eth_pcap.c | 52
>>> ++++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 52 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/config/common_base b/config/common_base index
>>> c2b0d91..9804585 100644
>>> --- a/config/common_base
>>> +++ b/config/common_base
>>> @@ -410,6 +410,7 @@ CONFIG_RTE_LIBRTE_PMD_NULL=y  # Compile
>> software
>>> PMD backed by PCAP files  #  CONFIG_RTE_LIBRTE_PMD_PCAP=n
>>> +CONFIG_RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT=n
>>
>> Hi Juhamatti,
>>
>> Why a build time config option for this? Can we make it a runtime devarg?
> 
> Sure, we can make it a devarg. Or do we even need that? Are there a lot of test dependencies that would need to be fixed if we have it enabled by default?

Not test dependencies but this may be overkill for some usecases, I prefer
making this dynamically configurable, no strong opinion though.

> 
>> Overall we are trying to reduce config options already and this seems no
>> need to be build time option at all.
>>
>> btw, this is a little late in release cycle, so lets target this patch for next
>> release.
> 
> The patch is on top of net-next, this should be just fine.

Perhaps we should rename the sub-tree :) because this is not happening first
time. next-net is not for next release, as it has been Linux, it is for this
release but just a sub-tree for net PMDs.

> 
>> Thanks,
>> ferruh
> 
> Thanks,
> --
>  Juhamatti
> 

Re: [dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support

[Kuusisaari, Juhamatti]

> >> Why a build time config option for this? Can we make it a runtime devarg?
> >
> > Sure, we can make it a devarg. Or do we even need that? Are there a lot of
> test dependencies that would need to be fixed if we have it enabled by
> default?
> 
> Not test dependencies but this may be overkill for some usecases, I prefer
> making this dynamically configurable, no strong opinion though.

OK, I'll take a look at this and craft a new version.
 
> >
> >> Overall we are trying to reduce config options already and this seems
> >> no need to be build time option at all.
> >>
> >> btw, this is a little late in release cycle, so lets target this
> >> patch for next release.
> >
> > The patch is on top of net-next, this should be just fine.
> 
> Perhaps we should rename the sub-tree :) because this is not happening first
> time. next-net is not for next release, as it has been Linux, it is for this
> release but just a sub-tree for net PMDs.

Aha, while reading the docs it says: "All sub-repositories are merged into main repository for -rc1 and -rc2", so I kind of thought this sub-repo is going to the next release, as you have rc2 out already.

> >
> >> Thanks,
> >> ferruh
> >

Thanks,
--
 Juhamatti

Re: [dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support

[Ferruh Yigit]

On 4/19/2018 6:16 AM, Kuusisaari, Juhamatti wrote:
>>>> Why a build time config option for this? Can we make it a runtime devarg?
>>>
>>> Sure, we can make it a devarg. Or do we even need that? Are there a lot of
>> test dependencies that would need to be fixed if we have it enabled by
>> default?
>>
>> Not test dependencies but this may be overkill for some usecases, I prefer
>> making this dynamically configurable, no strong opinion though.
> 
> OK, I'll take a look at this and craft a new version.
>  
>>>
>>>> Overall we are trying to reduce config options already and this seems
>>>> no need to be build time option at all.
>>>>
>>>> btw, this is a little late in release cycle, so lets target this
>>>> patch for next release.
>>>
>>> The patch is on top of net-next, this should be just fine.
>>
>> Perhaps we should rename the sub-tree :) because this is not happening first
>> time. next-net is not for next release, as it has been Linux, it is for this
>> release but just a sub-tree for net PMDs.
> 
> Aha, while reading the docs it says: "All sub-repositories are merged into main repository for -rc1 and -rc2", so I kind of thought this sub-repo is going to the next release, as you have rc2 out already.

rc2 is not out yet, we are still working for rc1, which should be soon.

> 
>>>
>>>> Thanks,
>>>> ferruh
>>>
> 
> Thanks,
> --
>  Juhamatti
>