linux-generic: pktio: support multiple loop interfaces

Message ID 1452872765-14197-1-git-send-email-stuart.haslam@linaro.org
State New
Headers show

Commit Message

Stuart Haslam Jan. 15, 2016, 3:46 p.m.
The current implementation supports only a single loop interface named
"loop", but it's sometimes useful to use multiple loopback interfaces to
avoid the need to multiplex based on packet data.

Allow interfaces named "loop" or "loopN" where supported numbers for N
are 0-254.

Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org>
---
 .../linux-generic/include/odp_packet_io_internal.h |  1 +
 platform/linux-generic/pktio/loop.c                | 40 +++++++++++++++++-----
 2 files changed, 32 insertions(+), 9 deletions(-)

Comments

Maxim Uvarov Jan. 19, 2016, 1:05 p.m. | #1
Bala, as I remember you also said that Cavium has several loop device. 
Looks like it's reasonable
to update API then:

API
  * @note The device name "loop" is a reserved name for a loopback 
device used
  *     for testing purposes.

Maxim.

On 01/15/2016 18:46, Stuart Haslam wrote:
> The current implementation supports only a single loop interface named
> "loop", but it's sometimes useful to use multiple loopback interfaces to
> avoid the need to multiplex based on packet data.
>
> Allow interfaces named "loop" or "loopN" where supported numbers for N
> are 0-254.
>
> Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org>
> ---
>   .../linux-generic/include/odp_packet_io_internal.h |  1 +
>   platform/linux-generic/pktio/loop.c                | 40 +++++++++++++++++-----
>   2 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h
> index 4d73952..1645670 100644
> --- a/platform/linux-generic/include/odp_packet_io_internal.h
> +++ b/platform/linux-generic/include/odp_packet_io_internal.h
> @@ -45,6 +45,7 @@ struct pktio_if_ops;
>   typedef struct {
>   	odp_queue_t loopq;		/**< loopback queue for "loop" device */
>   	odp_bool_t promisc;		/**< promiscuous mode state */
> +	unsigned char if_mac[ETH_ALEN];	/**< MAC address for the interface */
>   } pkt_loop_t;
>   
>   #ifdef HAVE_PCAP
> diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/pktio/loop.c
> index 95644c4..911eca8 100644
> --- a/platform/linux-generic/pktio/loop.c
> +++ b/platform/linux-generic/pktio/loop.c
> @@ -19,27 +19,50 @@
>   #include <odp/helper/eth.h>
>   #include <odp/helper/ip.h>
>   
> +#include <ctype.h>
>   #include <errno.h>
>   
> -/* MAC address for the "loop" interface */
> -static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73, 0x01};
> +/* MAC address for the "loop" interface. When a numbered interface is
> + * opened, the number suffix replaces the last digit of the MAC address,
> + * e.g. loop1 has MAC 02:e9:34:80:73:01 */
> +static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73, 0xff};
>   
>   static int loopback_open(odp_pktio_t id, pktio_entry_t *pktio_entry,
>   			 const char *devname, odp_pool_t pool ODP_UNUSED)
>   {
> -	if (strcmp(devname, "loop"))
> +	char loopq_name[ODP_QUEUE_NAME_LEN];
> +	int loop_num = -1;
> +
> +	if (strncmp(devname, "loop", 4))
>   		return -1;
>   
> -	char loopq_name[ODP_QUEUE_NAME_LEN];
> +	devname += strlen("loop");
> +
> +	/* valid names are loop[0-254] */
> +	if (*devname != '\0') {
> +		const char *p = devname;
>   
> -	snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64 "-pktio_loopq",
> -		 odp_pktio_to_u64(id));
> +		while (*p != '\0')
> +			if (!isdigit(*p++))
> +				return -1;
> +
> +		loop_num = atoi(devname);
> +		if (loop_num >= 0xff)
> +			return -1;
> +	}
> +
> +	snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64 "-pktio_loop%s",
> +		 odp_pktio_to_u64(id), devname);
>   	pktio_entry->s.pkt_loop.loopq =
>   		odp_queue_create(loopq_name, ODP_QUEUE_TYPE_POLL, NULL);
>   
>   	if (pktio_entry->s.pkt_loop.loopq == ODP_QUEUE_INVALID)
>   		return -1;
>   
> +	memcpy(pktio_entry->s.pkt_loop.if_mac, pktio_loop_mac, ETH_ALEN);
> +	if (loop_num != -1)
> +		pktio_entry->s.pkt_loop.if_mac[ETH_ALEN-1] = loop_num;
> +
>   	return 0;
>   }
>   
> @@ -104,10 +127,9 @@ static int loopback_mtu_get(pktio_entry_t *pktio_entry ODP_UNUSED)
>   	return INT_MAX;
>   }
>   
> -static int loopback_mac_addr_get(pktio_entry_t *pktio_entry ODP_UNUSED,
> -				 void *mac_addr)
> +static int loopback_mac_addr_get(pktio_entry_t *pktio_entry, void *mac_addr)
>   {
> -	memcpy(mac_addr, pktio_loop_mac, ETH_ALEN);
> +	memcpy(mac_addr, pktio_entry->s.pkt_loop.if_mac, ETH_ALEN);
>   	return ETH_ALEN;
>   }
>
Balasubramanian Manoharan Jan. 19, 2016, 1:23 p.m. | #2
The "loop" as a keyword for loop interface was useful to test
validation suite since it needed only one interface but if we need
multiple loopback interface then I wouldn't suggest using "loopX"
keyword but a different mechanism to set an interface as loop.

I believe most platforms will be able to configure an existing
interface as loop so that it will send the packet back into the system
so if we need more than one loop interface it is better to get the
capability of the interface to check whether this particular interface
can support loopback and then set the interface as loop.
Regards,
Bala


On 19 January 2016 at 18:35, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> Bala, as I remember you also said that Cavium has several loop device. Looks
> like it's reasonable
> to update API then:
>
> API
>  * @note The device name "loop" is a reserved name for a loopback device
> used
>  *     for testing purposes.
>
> Maxim.
>
>
> On 01/15/2016 18:46, Stuart Haslam wrote:
>>
>> The current implementation supports only a single loop interface named
>> "loop", but it's sometimes useful to use multiple loopback interfaces to
>> avoid the need to multiplex based on packet data.
>>
>> Allow interfaces named "loop" or "loopN" where supported numbers for N
>> are 0-254.
>>
>> Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org>
>> ---
>>   .../linux-generic/include/odp_packet_io_internal.h |  1 +
>>   platform/linux-generic/pktio/loop.c                | 40
>> +++++++++++++++++-----
>>   2 files changed, 32 insertions(+), 9 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h
>> b/platform/linux-generic/include/odp_packet_io_internal.h
>> index 4d73952..1645670 100644
>> --- a/platform/linux-generic/include/odp_packet_io_internal.h
>> +++ b/platform/linux-generic/include/odp_packet_io_internal.h
>> @@ -45,6 +45,7 @@ struct pktio_if_ops;
>>   typedef struct {
>>         odp_queue_t loopq;              /**< loopback queue for "loop"
>> device */
>>         odp_bool_t promisc;             /**< promiscuous mode state */
>> +       unsigned char if_mac[ETH_ALEN]; /**< MAC address for the interface
>> */
>>   } pkt_loop_t;
>>     #ifdef HAVE_PCAP
>> diff --git a/platform/linux-generic/pktio/loop.c
>> b/platform/linux-generic/pktio/loop.c
>> index 95644c4..911eca8 100644
>> --- a/platform/linux-generic/pktio/loop.c
>> +++ b/platform/linux-generic/pktio/loop.c
>> @@ -19,27 +19,50 @@
>>   #include <odp/helper/eth.h>
>>   #include <odp/helper/ip.h>
>>   +#include <ctype.h>
>>   #include <errno.h>
>>   -/* MAC address for the "loop" interface */
>> -static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73,
>> 0x01};
>> +/* MAC address for the "loop" interface. When a numbered interface is
>> + * opened, the number suffix replaces the last digit of the MAC address,
>> + * e.g. loop1 has MAC 02:e9:34:80:73:01 */
>> +static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73,
>> 0xff};
>>     static int loopback_open(odp_pktio_t id, pktio_entry_t *pktio_entry,
>>                          const char *devname, odp_pool_t pool ODP_UNUSED)
>>   {
>> -       if (strcmp(devname, "loop"))
>> +       char loopq_name[ODP_QUEUE_NAME_LEN];
>> +       int loop_num = -1;
>> +
>> +       if (strncmp(devname, "loop", 4))
>>                 return -1;
>>   -     char loopq_name[ODP_QUEUE_NAME_LEN];
>> +       devname += strlen("loop");
>> +
>> +       /* valid names are loop[0-254] */
>> +       if (*devname != '\0') {
>> +               const char *p = devname;
>>   -     snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64
>> "-pktio_loopq",
>> -                odp_pktio_to_u64(id));
>> +               while (*p != '\0')
>> +                       if (!isdigit(*p++))
>> +                               return -1;
>> +
>> +               loop_num = atoi(devname);
>> +               if (loop_num >= 0xff)
>> +                       return -1;
>> +       }
>> +
>> +       snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64
>> "-pktio_loop%s",
>> +                odp_pktio_to_u64(id), devname);
>>         pktio_entry->s.pkt_loop.loopq =
>>                 odp_queue_create(loopq_name, ODP_QUEUE_TYPE_POLL, NULL);
>>         if (pktio_entry->s.pkt_loop.loopq == ODP_QUEUE_INVALID)
>>                 return -1;
>>   +     memcpy(pktio_entry->s.pkt_loop.if_mac, pktio_loop_mac, ETH_ALEN);
>> +       if (loop_num != -1)
>> +               pktio_entry->s.pkt_loop.if_mac[ETH_ALEN-1] = loop_num;
>> +
>>         return 0;
>>   }
>>   @@ -104,10 +127,9 @@ static int loopback_mtu_get(pktio_entry_t
>> *pktio_entry ODP_UNUSED)
>>         return INT_MAX;
>>   }
>>   -static int loopback_mac_addr_get(pktio_entry_t *pktio_entry ODP_UNUSED,
>> -                                void *mac_addr)
>> +static int loopback_mac_addr_get(pktio_entry_t *pktio_entry, void
>> *mac_addr)
>>   {
>> -       memcpy(mac_addr, pktio_loop_mac, ETH_ALEN);
>> +       memcpy(mac_addr, pktio_entry->s.pkt_loop.if_mac, ETH_ALEN);
>>         return ETH_ALEN;
>>   }
>>
>
>
Mike Holmes Jan. 19, 2016, 2:40 p.m. | #3
On 19 January 2016 at 08:23, Bala Manoharan <bala.manoharan@linaro.org>
wrote:

> The "loop" as a keyword for loop interface was useful to test

> validation suite since it needed only one interface but if we need

> multiple loopback interface then I wouldn't suggest using "loopX"

> keyword but a different mechanism to set an interface as loop.

>

> I believe most platforms will be able to configure an existing

> interface as loop so that it will send the packet back into the system

> so if we need more than one loop interface it is better to get the

> capability of the interface to check whether this particular interface

> can support loopback and then set the interface as loop.

> Regards,

> Bala

>


Being able to set an real interface to its loop mode which can face in both
directions i.e. external loopback for external equipment testing and
internally to test an application is needed in real systems.
Loopbacks can also appear at intermediate steps in the process path, maybe
before and after classifier if that makes sense. We should make it possible
to use those also. These intermediate lop points probably can't be used in
the validation tests becasue they may not exist on all hw - how do we
handle a platform advertising the loops backs it can offer for debug?

But  having multiple HW loopback devices will make generic testing that
much less portable if we are not carefull.
If we go the HW Loopback route, then all platforms will have to export the
interface to be used when calling the validation tests rather than knowing
loopX is available by default.

One thing we need to do is expand the testing out from sunny day and I
think that requires that we can trivially add tests that send packet flows.
This needs to be in a really portable way so that the validation tests can
drive packets though generically, presumably we can only inject traffic
into psuo interfaces and not into the front end of real ones on all HW ?


>

> On 19 January 2016 at 18:35, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> > Bala, as I remember you also said that Cavium has several loop device.

> Looks

> > like it's reasonable

> > to update API then:

> >

> > API

> >  * @note The device name "loop" is a reserved name for a loopback device

> > used

> >  *     for testing purposes.

> >

> > Maxim.

> >

> >

> > On 01/15/2016 18:46, Stuart Haslam wrote:

> >>

> >> The current implementation supports only a single loop interface named

> >> "loop", but it's sometimes useful to use multiple loopback interfaces to

> >> avoid the need to multiplex based on packet data.

> >>

> >> Allow interfaces named "loop" or "loopN" where supported numbers for N

> >> are 0-254.

> >>

> >> Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org>

> >> ---

> >>   .../linux-generic/include/odp_packet_io_internal.h |  1 +

> >>   platform/linux-generic/pktio/loop.c                | 40

> >> +++++++++++++++++-----

> >>   2 files changed, 32 insertions(+), 9 deletions(-)

> >>

> >> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h

> >> b/platform/linux-generic/include/odp_packet_io_internal.h

> >> index 4d73952..1645670 100644

> >> --- a/platform/linux-generic/include/odp_packet_io_internal.h

> >> +++ b/platform/linux-generic/include/odp_packet_io_internal.h

> >> @@ -45,6 +45,7 @@ struct pktio_if_ops;

> >>   typedef struct {

> >>         odp_queue_t loopq;              /**< loopback queue for "loop"

> >> device */

> >>         odp_bool_t promisc;             /**< promiscuous mode state */

> >> +       unsigned char if_mac[ETH_ALEN]; /**< MAC address for the

> interface

> >> */

> >>   } pkt_loop_t;

> >>     #ifdef HAVE_PCAP

> >> diff --git a/platform/linux-generic/pktio/loop.c

> >> b/platform/linux-generic/pktio/loop.c

> >> index 95644c4..911eca8 100644

> >> --- a/platform/linux-generic/pktio/loop.c

> >> +++ b/platform/linux-generic/pktio/loop.c

> >> @@ -19,27 +19,50 @@

> >>   #include <odp/helper/eth.h>

> >>   #include <odp/helper/ip.h>

> >>   +#include <ctype.h>

> >>   #include <errno.h>

> >>   -/* MAC address for the "loop" interface */

> >> -static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73,

> >> 0x01};

> >> +/* MAC address for the "loop" interface. When a numbered interface is

> >> + * opened, the number suffix replaces the last digit of the MAC

> address,

> >> + * e.g. loop1 has MAC 02:e9:34:80:73:01 */

> >> +static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73,

> >> 0xff};

> >>     static int loopback_open(odp_pktio_t id, pktio_entry_t *pktio_entry,

> >>                          const char *devname, odp_pool_t pool

> ODP_UNUSED)

> >>   {

> >> -       if (strcmp(devname, "loop"))

> >> +       char loopq_name[ODP_QUEUE_NAME_LEN];

> >> +       int loop_num = -1;

> >> +

> >> +       if (strncmp(devname, "loop", 4))

> >>                 return -1;

> >>   -     char loopq_name[ODP_QUEUE_NAME_LEN];

> >> +       devname += strlen("loop");

> >> +

> >> +       /* valid names are loop[0-254] */

> >> +       if (*devname != '\0') {

> >> +               const char *p = devname;

> >>   -     snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64

> >> "-pktio_loopq",

> >> -                odp_pktio_to_u64(id));

> >> +               while (*p != '\0')

> >> +                       if (!isdigit(*p++))

> >> +                               return -1;

> >> +

> >> +               loop_num = atoi(devname);

> >> +               if (loop_num >= 0xff)

> >> +                       return -1;

> >> +       }

> >> +

> >> +       snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64

> >> "-pktio_loop%s",

> >> +                odp_pktio_to_u64(id), devname);

> >>         pktio_entry->s.pkt_loop.loopq =

> >>                 odp_queue_create(loopq_name, ODP_QUEUE_TYPE_POLL, NULL);

> >>         if (pktio_entry->s.pkt_loop.loopq == ODP_QUEUE_INVALID)

> >>                 return -1;

> >>   +     memcpy(pktio_entry->s.pkt_loop.if_mac, pktio_loop_mac,

> ETH_ALEN);

> >> +       if (loop_num != -1)

> >> +               pktio_entry->s.pkt_loop.if_mac[ETH_ALEN-1] = loop_num;

> >> +

> >>         return 0;

> >>   }

> >>   @@ -104,10 +127,9 @@ static int loopback_mtu_get(pktio_entry_t

> >> *pktio_entry ODP_UNUSED)

> >>         return INT_MAX;

> >>   }

> >>   -static int loopback_mac_addr_get(pktio_entry_t *pktio_entry

> ODP_UNUSED,

> >> -                                void *mac_addr)

> >> +static int loopback_mac_addr_get(pktio_entry_t *pktio_entry, void

> >> *mac_addr)

> >>   {

> >> -       memcpy(mac_addr, pktio_loop_mac, ETH_ALEN);

> >> +       memcpy(mac_addr, pktio_entry->s.pkt_loop.if_mac, ETH_ALEN);

> >>         return ETH_ALEN;

> >>   }

> >>

> >

> >

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>




-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs

Patch

diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h
index 4d73952..1645670 100644
--- a/platform/linux-generic/include/odp_packet_io_internal.h
+++ b/platform/linux-generic/include/odp_packet_io_internal.h
@@ -45,6 +45,7 @@  struct pktio_if_ops;
 typedef struct {
 	odp_queue_t loopq;		/**< loopback queue for "loop" device */
 	odp_bool_t promisc;		/**< promiscuous mode state */
+	unsigned char if_mac[ETH_ALEN];	/**< MAC address for the interface */
 } pkt_loop_t;
 
 #ifdef HAVE_PCAP
diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/pktio/loop.c
index 95644c4..911eca8 100644
--- a/platform/linux-generic/pktio/loop.c
+++ b/platform/linux-generic/pktio/loop.c
@@ -19,27 +19,50 @@ 
 #include <odp/helper/eth.h>
 #include <odp/helper/ip.h>
 
+#include <ctype.h>
 #include <errno.h>
 
-/* MAC address for the "loop" interface */
-static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73, 0x01};
+/* MAC address for the "loop" interface. When a numbered interface is
+ * opened, the number suffix replaces the last digit of the MAC address,
+ * e.g. loop1 has MAC 02:e9:34:80:73:01 */
+static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73, 0xff};
 
 static int loopback_open(odp_pktio_t id, pktio_entry_t *pktio_entry,
 			 const char *devname, odp_pool_t pool ODP_UNUSED)
 {
-	if (strcmp(devname, "loop"))
+	char loopq_name[ODP_QUEUE_NAME_LEN];
+	int loop_num = -1;
+
+	if (strncmp(devname, "loop", 4))
 		return -1;
 
-	char loopq_name[ODP_QUEUE_NAME_LEN];
+	devname += strlen("loop");
+
+	/* valid names are loop[0-254] */
+	if (*devname != '\0') {
+		const char *p = devname;
 
-	snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64 "-pktio_loopq",
-		 odp_pktio_to_u64(id));
+		while (*p != '\0')
+			if (!isdigit(*p++))
+				return -1;
+
+		loop_num = atoi(devname);
+		if (loop_num >= 0xff)
+			return -1;
+	}
+
+	snprintf(loopq_name, sizeof(loopq_name), "%" PRIu64 "-pktio_loop%s",
+		 odp_pktio_to_u64(id), devname);
 	pktio_entry->s.pkt_loop.loopq =
 		odp_queue_create(loopq_name, ODP_QUEUE_TYPE_POLL, NULL);
 
 	if (pktio_entry->s.pkt_loop.loopq == ODP_QUEUE_INVALID)
 		return -1;
 
+	memcpy(pktio_entry->s.pkt_loop.if_mac, pktio_loop_mac, ETH_ALEN);
+	if (loop_num != -1)
+		pktio_entry->s.pkt_loop.if_mac[ETH_ALEN-1] = loop_num;
+
 	return 0;
 }
 
@@ -104,10 +127,9 @@  static int loopback_mtu_get(pktio_entry_t *pktio_entry ODP_UNUSED)
 	return INT_MAX;
 }
 
-static int loopback_mac_addr_get(pktio_entry_t *pktio_entry ODP_UNUSED,
-				 void *mac_addr)
+static int loopback_mac_addr_get(pktio_entry_t *pktio_entry, void *mac_addr)
 {
-	memcpy(mac_addr, pktio_loop_mac, ETH_ALEN);
+	memcpy(mac_addr, pktio_entry->s.pkt_loop.if_mac, ETH_ALEN);
 	return ETH_ALEN;
 }