diff mbox

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

Message ID 1418050713-29694-6-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Dec. 8, 2014, 2:58 p.m. UTC
Implement pktio device loop device suitable for testing.
Note: sometimes lo0 can not be used in case if you need
change / get mac, promisc, mtu for specific device.
Environment variable is added to bind loop to specific device, example:
export ODP_PKTIO_LOOPDEV=eth0

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

Comments

Ciprian Barbu Dec. 9, 2014, 3:28 p.m. UTC | #1
On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> Implement pktio device loop device suitable for testing.
> Note: sometimes lo0 can not be used in case if you need
> change / get mac, promisc, mtu for specific device.
> Environment variable is added to bind loop to specific device, example:
> export ODP_PKTIO_LOOPDEV=eth0
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  platform/linux-generic/odp_packet_io.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
> index e8815cd..8a86421 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];
> @@ -160,6 +161,37 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool)
>         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 (!strcmp(dev, "loop")) {

loop_hint is only used if dev == "loop", no need to waste a getenv for
it otherwise.

> +               if (!loop_hint || (strlen(loop_hint) == 0)) {
> +                       ODP_ERR("Set loop with ODP_PKTIO_LOOPDEV=ethX\n");
> +                       return ODP_PKTIO_INVALID;
> +               }
> +
> +               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 using %s as loopback device\n", loop_hint);
> +       }
>
>         id = alloc_lock_pktio_entry();
>         if (id == ODP_PKTIO_INVALID) {
> --
> 1.8.5.1.163.gd7aced9
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Maxim Uvarov Dec. 9, 2014, 3:33 p.m. UTC | #2
On 12/09/2014 06:28 PM, Ciprian Barbu wrote:
> On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>> Implement pktio device loop device suitable for testing.
>> Note: sometimes lo0 can not be used in case if you need
>> change / get mac, promisc, mtu for specific device.
>> Environment variable is added to bind loop to specific device, example:
>> export ODP_PKTIO_LOOPDEV=eth0
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   platform/linux-generic/odp_packet_io.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
>> index e8815cd..8a86421 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];
>> @@ -160,6 +161,37 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool)
>>          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 (!strcmp(dev, "loop")) {
> loop_hint is only used if dev == "loop", no need to waste a getenv for
> it otherwise.

Nope. You can have this env variable but open other packet i/o. For 
example eth0. From API "loop" is
keyword to open loop device.

Maxim.

>> +               if (!loop_hint || (strlen(loop_hint) == 0)) {
>> +                       ODP_ERR("Set loop with ODP_PKTIO_LOOPDEV=ethX\n");
>> +                       return ODP_PKTIO_INVALID;
>> +               }
>> +
>> +               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 using %s as loopback device\n", loop_hint);
>> +       }
>>
>>          id = alloc_lock_pktio_entry();
>>          if (id == ODP_PKTIO_INVALID) {
>> --
>> 1.8.5.1.163.gd7aced9
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
Ciprian Barbu Dec. 9, 2014, 3:41 p.m. UTC | #3
On Tue, Dec 9, 2014 at 5:33 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 12/09/2014 06:28 PM, Ciprian Barbu wrote:
>>
>> On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org>
>> wrote:
>>>
>>> Implement pktio device loop device suitable for testing.
>>> Note: sometimes lo0 can not be used in case if you need
>>> change / get mac, promisc, mtu for specific device.
>>> Environment variable is added to bind loop to specific device, example:
>>> export ODP_PKTIO_LOOPDEV=eth0
>>>
>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>>> ---
>>>   platform/linux-generic/odp_packet_io.c | 32
>>> ++++++++++++++++++++++++++++++++
>>>   1 file changed, 32 insertions(+)
>>>
>>> diff --git a/platform/linux-generic/odp_packet_io.c
>>> b/platform/linux-generic/odp_packet_io.c
>>> index e8815cd..8a86421 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];
>>> @@ -160,6 +161,37 @@ odp_pktio_t odp_pktio_open(const char *dev,
>>> odp_buffer_pool_t pool)
>>>          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 (!strcmp(dev, "loop")) {
>>
>> loop_hint is only used if dev == "loop", no need to waste a getenv for
>> it otherwise.
>
>
> Nope. You can have this env variable but open other packet i/o. For example
> eth0. From API "loop" is
> keyword to open loop device.

I was trying to say you can move the getenv instruction inside the if
block, because you are not using the loop hint if dev is something
else than "loop".

>
> Maxim.
>
>
>>> +               if (!loop_hint || (strlen(loop_hint) == 0)) {
>>> +                       ODP_ERR("Set loop with
>>> ODP_PKTIO_LOOPDEV=ethX\n");
>>> +                       return ODP_PKTIO_INVALID;
>>> +               }
>>> +
>>> +               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 using %s as loopback device\n",
>>> loop_hint);
>>> +       }
>>>
>>>          id = alloc_lock_pktio_entry();
>>>          if (id == ODP_PKTIO_INVALID) {
>>> --
>>> 1.8.5.1.163.gd7aced9
>>>
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
Maxim Uvarov Dec. 9, 2014, 3:50 p.m. UTC | #4
On 12/09/2014 06:41 PM, Ciprian Barbu wrote:
> On Tue, Dec 9, 2014 at 5:33 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>> On 12/09/2014 06:28 PM, Ciprian Barbu wrote:
>>> On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org>
>>> wrote:
>>>> Implement pktio device loop device suitable for testing.
>>>> Note: sometimes lo0 can not be used in case if you need
>>>> change / get mac, promisc, mtu for specific device.
>>>> Environment variable is added to bind loop to specific device, example:
>>>> export ODP_PKTIO_LOOPDEV=eth0
>>>>
>>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>>>> ---
>>>>    platform/linux-generic/odp_packet_io.c | 32
>>>> ++++++++++++++++++++++++++++++++
>>>>    1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/platform/linux-generic/odp_packet_io.c
>>>> b/platform/linux-generic/odp_packet_io.c
>>>> index e8815cd..8a86421 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];
>>>> @@ -160,6 +161,37 @@ odp_pktio_t odp_pktio_open(const char *dev,
>>>> odp_buffer_pool_t pool)
>>>>           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 (!strcmp(dev, "loop")) {
>>> loop_hint is only used if dev == "loop", no need to waste a getenv for
>>> it otherwise.
>>
>> Nope. You can have this env variable but open other packet i/o. For example
>> eth0. From API "loop" is
>> keyword to open loop device.
> I was trying to say you can move the getenv instruction inside the if
> block, because you are not using the loop hint if dev is something
> else than "loop".

getenv is system call. I would like to compare string and then if needed 
do syscall.

Maxim.


>> Maxim.
>>
>>
>>>> +               if (!loop_hint || (strlen(loop_hint) == 0)) {
>>>> +                       ODP_ERR("Set loop with
>>>> ODP_PKTIO_LOOPDEV=ethX\n");
>>>> +                       return ODP_PKTIO_INVALID;
>>>> +               }
>>>> +
>>>> +               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 using %s as loopback device\n",
>>>> loop_hint);
>>>> +       }
>>>>
>>>>           id = alloc_lock_pktio_entry();
>>>>           if (id == ODP_PKTIO_INVALID) {
>>>> --
>>>> 1.8.5.1.163.gd7aced9
>>>>
>>>>
>>>> _______________________________________________
>>>> lng-odp mailing list
>>>> lng-odp@lists.linaro.org
>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
Ciprian Barbu Dec. 9, 2014, 4:09 p.m. UTC | #5
On Tue, Dec 9, 2014 at 5:50 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 12/09/2014 06:41 PM, Ciprian Barbu wrote:
>>
>> On Tue, Dec 9, 2014 at 5:33 PM, Maxim Uvarov <maxim.uvarov@linaro.org>
>> wrote:
>>>
>>> On 12/09/2014 06:28 PM, Ciprian Barbu wrote:
>>>>
>>>> On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org>
>>>> wrote:
>>>>>
>>>>> Implement pktio device loop device suitable for testing.
>>>>> Note: sometimes lo0 can not be used in case if you need
>>>>> change / get mac, promisc, mtu for specific device.
>>>>> Environment variable is added to bind loop to specific device, example:
>>>>> export ODP_PKTIO_LOOPDEV=eth0
>>>>>
>>>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>>>>> ---
>>>>>    platform/linux-generic/odp_packet_io.c | 32
>>>>> ++++++++++++++++++++++++++++++++
>>>>>    1 file changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/platform/linux-generic/odp_packet_io.c
>>>>> b/platform/linux-generic/odp_packet_io.c
>>>>> index e8815cd..8a86421 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];
>>>>> @@ -160,6 +161,37 @@ odp_pktio_t odp_pktio_open(const char *dev,
>>>>> odp_buffer_pool_t pool)
>>>>>           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 (!strcmp(dev, "loop")) {
>>>>
>>>> loop_hint is only used if dev == "loop", no need to waste a getenv for
>>>> it otherwise.
>>>
>>>
>>> Nope. You can have this env variable but open other packet i/o. For
>>> example
>>> eth0. From API "loop" is
>>> keyword to open loop device.
>>
>> I was trying to say you can move the getenv instruction inside the if
>> block, because you are not using the loop hint if dev is something
>> else than "loop".
>
>
> getenv is system call. I would like to compare string and then if needed do
> syscall.

That's exactly what I'm saying too. Avoid system calls if not
necessary. But this patch always makes the system call.

>
> Maxim.
>
>
>
>>> Maxim.
>>>
>>>
>>>>> +               if (!loop_hint || (strlen(loop_hint) == 0)) {
>>>>> +                       ODP_ERR("Set loop with
>>>>> ODP_PKTIO_LOOPDEV=ethX\n");
>>>>> +                       return ODP_PKTIO_INVALID;
>>>>> +               }
>>>>> +
>>>>> +               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 using %s as loopback device\n",
>>>>> loop_hint);
>>>>> +       }
>>>>>
>>>>>           id = alloc_lock_pktio_entry();
>>>>>           if (id == ODP_PKTIO_INVALID) {
>>>>> --
>>>>> 1.8.5.1.163.gd7aced9
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> lng-odp mailing list
>>>>> lng-odp@lists.linaro.org
>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>>
>
diff mbox

Patch

diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
index e8815cd..8a86421 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];
@@ -160,6 +161,37 @@  odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool)
 	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 (!strcmp(dev, "loop")) {
+		if (!loop_hint || (strlen(loop_hint) == 0)) {
+			ODP_ERR("Set loop with ODP_PKTIO_LOOPDEV=ethX\n");
+			return ODP_PKTIO_INVALID;
+		}
+
+		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 using %s as loopback device\n", loop_hint);
+	}
 
 	id = alloc_lock_pktio_entry();
 	if (id == ODP_PKTIO_INVALID) {