[dpdk-dev,v2,1/2] net/pcap: physical interface MAC support
Checks
Commit Message
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(-)
Comments
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
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
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
>
> >> 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
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
>
@@ -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
@@ -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;
}