diff mbox

[PATCHv4,4/6] linux-generic: odp_pktio_open loop support

Message ID 1417619157-13690-5-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Dec. 3, 2014, 3:05 p.m. UTC
Implement pktio device loop device suitable for testing.
Note: lo0 can not be used. Instead of it first upped device
is used.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 platform/linux-generic/odp_packet_io.c | 64 ++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

Maxim Uvarov Dec. 4, 2014, 9:57 a.m. UTC | #1
On 12/03/2014 08:40 PM, Stuart Haslam wrote:
> On Wed, Dec 03, 2014 at 03:05:55PM +0000, Maxim Uvarov wrote:
>> Implement pktio device loop device suitable for testing.
>> Note: lo0 can not be used. Instead of it first upped device
>> is used.
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   platform/linux-generic/odp_packet_io.c | 64 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
>> index 595ba56..900b47c 100644
>> --- a/platform/linux-generic/odp_packet_io.c
>> +++ b/platform/linux-generic/odp_packet_io.c
>> @@ -22,6 +22,7 @@
>>   #include <string.h>
>>   #include <sys/ioctl.h>
>>   #include <linux/if_arp.h>
>> +#include <ifaddrs.h>
>>   
>>   typedef struct {
>>   	pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES];
>> @@ -154,12 +155,75 @@ static int free_pktio_entry(odp_pktio_t id)
>>   	return 0;
>>   }
>>   
>> +static int find_loop_dev(char loop[IFNAMSIZ])
>> +{
>> +	struct ifaddrs *ifap, *ifa;
>> +
>> +	getifaddrs(&ifap);
>> +
>> +	for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
>> +		if (ifa->ifa_addr &&
>> +		    ifa->ifa_addr->sa_family == AF_INET) {
>> +			if ((ifa->ifa_flags & (IFF_LOOPBACK |
>> +					      IFF_POINTOPOINT |
>> +					      IFF_NOARP)) ||
>> +			    (!memcmp(ifa->ifa_name, "lxcbr", 5)) ||
>> +			    (!memcmp(ifa->ifa_name, "wlan", 4)) ||
>> +			    (!memcmp(ifa->ifa_name, "veth", 4)) ||
>> +			    (!memcmp(ifa->ifa_name, "virbr", 5)))
> What are you trying do here?.. it's not returning the "first upped
> interface" as the commit log says.

I'm trying to find device which can be used as loop. I.e. on which I can 
set/get
mac, promisc and etc. Comment in git log does not say that correctly.

>   Also filtering on interface name is
> fragile as names aren't fixed.
I'm trying to avoid predefined names for vlan, virtual bridge and etc. 
Printing flags I see that they have
same flags as eth0. Do not see other solution how to filter them without 
looking for names.



>
> On my desktop machine this returns my main network uplink interface,
> apart from this not being a loopback interface, it means my real network
> traffic is disrupted by the unit tests (e.g. I lose a few seconds of
> traffic while the MTU is being messed with).
Ok, that is not good. On mine I'm on wlan0 while this selects eth0.
I did not include mac address change. In that case I think you will 
loose your connection,
until test restores original value.
> I don't think there's a foolproof way to automatically find a suitable
> device. I would rather that attempting to open a "loop" device failed by
> default (on linux-generic) unless you specified in the environment
> variable a device that must actually be a usable loopback device. In
> other words odp_pktio_open("loop") should return a handle for a loopback
> device, not just some random device that might be useful for a subset of
> unit tests.

Automatic selection is specially for cunit tests. And yes it's not 
really clear
which criteria to follow to select that device. After discussion on 
meeting the
agreement was that it might be any device where we can test our 
functions. Because
cunit runs on development boxes it's ok if connection will disappear for 
some time.

>> +				continue;
>> +			memcpy(loop, ifa->ifa_name, IFNAMSIZ);
>> +			freeifaddrs(ifap);
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	freeifaddrs(ifap);
>> +	return -1;
>> +}
>> +
>>   odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool)
>>   {
>>   	odp_pktio_t id;
>>   	pktio_entry_t *pktio_entry;
>>   	int res;
>>   	int fanout = 1;
>> +	char loop[IFNAMSIZ] = {0};
>> +	char *loop_hint;
>> +
>> +	if (strlen(dev) > IFNAMSIZ) {
> Should be >=
>
>> +		/* ioctl names limitation */
>> +		ODP_ERR("pktio name %s is too big, limit is %d bytes\n",
>> +			dev, IFNAMSIZ);
> I think we need an ODP_PKTIO_NAME_LEN definition.
I thing it's implementation specific. In pktio_entry struct there is 
also array of IFNAMSIZ for linux-generic.

>> +		return ODP_PKTIO_INVALID;
>> +	}
>> +
>> +	/* If hint with ODP_PKTIO_LOOPDEV is provided, use hint,
>> +	 * if not try to find usable device.
>> +	 */
>> +	loop_hint = getenv("ODP_PKTIO_LOOPDEV");
>> +	if (!strncmp(dev, "loop", 4)) {
> strcmp(), otherwise "loop0", "loop1" would all work.

yes, thanks,  first version was for loop0 and I did not take match 
attention to redo it to loop.

>> +		if (loop_hint && (strlen(loop_hint) > 0)) {
>> +			if (strlen(loop_hint) > IFNAMSIZ) {
>> =
>> +				ODP_ERR("pktio name %s is too big, limit is %d bytes\n",
>> +					loop_hint, IFNAMSIZ);
>> +				return ODP_PKTIO_INVALID;
>> +			}
>> +
>> +			memset(loop, 0, IFNAMSIZ);
>> +			memcpy(loop, loop_hint, strlen(loop_hint));
>> +			dev = loop;
>> +			ODP_DBG("pktio rename loop0 to %s\n", loop_hint);
> loop0? also the comment isn't very clear what's happening, perhaps;
>
> "using %s as loopback device"
>
>> +		} else {
>> +			if (!find_loop_dev(loop)) {
>> +				ODP_DBG("pktio rename loop to %s\n", loop);
>> +				dev = loop;
>> +			} else {
>> +				ODP_DBG("pktio: No suitable netdev.\n");
>> +				return ODP_PKTIO_INVALID;
>> +			}
>> +		}
>> +	}
>>   
>>   	id = alloc_lock_pktio_entry();
>>   	if (id == ODP_PKTIO_INVALID) {
>> -- 
>> 1.8.5.1.163.gd7aced9
>>
diff mbox

Patch

diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
index 595ba56..900b47c 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -22,6 +22,7 @@ 
 #include <string.h>
 #include <sys/ioctl.h>
 #include <linux/if_arp.h>
+#include <ifaddrs.h>
 
 typedef struct {
 	pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES];
@@ -154,12 +155,75 @@  static int free_pktio_entry(odp_pktio_t id)
 	return 0;
 }
 
+static int find_loop_dev(char loop[IFNAMSIZ])
+{
+	struct ifaddrs *ifap, *ifa;
+
+	getifaddrs(&ifap);
+
+	for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+		if (ifa->ifa_addr &&
+		    ifa->ifa_addr->sa_family == AF_INET) {
+			if ((ifa->ifa_flags & (IFF_LOOPBACK |
+					      IFF_POINTOPOINT |
+					      IFF_NOARP)) ||
+			    (!memcmp(ifa->ifa_name, "lxcbr", 5)) ||
+			    (!memcmp(ifa->ifa_name, "wlan", 4)) ||
+			    (!memcmp(ifa->ifa_name, "veth", 4)) ||
+			    (!memcmp(ifa->ifa_name, "virbr", 5)))
+				continue;
+			memcpy(loop, ifa->ifa_name, IFNAMSIZ);
+			freeifaddrs(ifap);
+			return 0;
+		}
+	}
+
+	freeifaddrs(ifap);
+	return -1;
+}
+
 odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool)
 {
 	odp_pktio_t id;
 	pktio_entry_t *pktio_entry;
 	int res;
 	int fanout = 1;
+	char loop[IFNAMSIZ] = {0};
+	char *loop_hint;
+
+	if (strlen(dev) > IFNAMSIZ) {
+		/* ioctl names limitation */
+		ODP_ERR("pktio name %s is too big, limit is %d bytes\n",
+			dev, IFNAMSIZ);
+		return ODP_PKTIO_INVALID;
+	}
+
+	/* If hint with ODP_PKTIO_LOOPDEV is provided, use hint,
+	 * if not try to find usable device.
+	 */
+	loop_hint = getenv("ODP_PKTIO_LOOPDEV");
+	if (!strncmp(dev, "loop", 4)) {
+		if (loop_hint && (strlen(loop_hint) > 0)) {
+			if (strlen(loop_hint) > IFNAMSIZ) {
+				ODP_ERR("pktio name %s is too big, limit is %d bytes\n",
+					loop_hint, IFNAMSIZ);
+				return ODP_PKTIO_INVALID;
+			}
+
+			memset(loop, 0, IFNAMSIZ);
+			memcpy(loop, loop_hint, strlen(loop_hint));
+			dev = loop;
+			ODP_DBG("pktio rename loop0 to %s\n", loop_hint);
+		} else {
+			if (!find_loop_dev(loop)) {
+				ODP_DBG("pktio rename loop to %s\n", loop);
+				dev = loop;
+			} else {
+				ODP_DBG("pktio: No suitable netdev.\n");
+				return ODP_PKTIO_INVALID;
+			}
+		}
+	}
 
 	id = alloc_lock_pktio_entry();
 	if (id == ODP_PKTIO_INVALID) {