diff mbox

[RFC/PATCH,1/2] linux-generic: mmap: jumbo frames support

Message ID 1424357149-14761-2-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Feb. 19, 2015, 2:45 p.m. UTC
Support for jumbo frames for linux-generic with unsegmented buffers.
https://bugs.linaro.org/show_bug.cgi?id=509

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 platform/linux-generic/odp_packet_socket.c | 6 ++++--
 test/validation/odp_pktio.c                | 8 ++++++--
 test/validation/odp_pktio_run              | 4 ++--
 3 files changed, 12 insertions(+), 6 deletions(-)

Comments

Bill Fischofer Feb. 19, 2015, 10:14 p.m. UTC | #1
On Thu, Feb 19, 2015 at 8:45 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Support for jumbo frames for linux-generic with unsegmented buffers.
> https://bugs.linaro.org/show_bug.cgi?id=509
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  platform/linux-generic/odp_packet_socket.c | 6 ++++--
>  test/validation/odp_pktio.c                | 8 ++++++--
>  test/validation/odp_pktio_run              | 4 ++--
>  3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/platform/linux-generic/odp_packet_socket.c
> b/platform/linux-generic/odp_packet_socket.c
> index 41e57c1..3e1451e 100644
> --- a/platform/linux-generic/odp_packet_socket.c
> +++ b/platform/linux-generic/odp_packet_socket.c
> @@ -554,10 +554,12 @@ static inline unsigned pkt_mmap_v2_tx(int sock,
> struct ring *ring,
>                         ppd.v2->tp_h.tp_snaplen = pkt_len;
>                         ppd.v2->tp_h.tp_len = pkt_len;
>
> -                       odp_packet_copydata_out(pkt_table[i], 0, pkt_len,
> +                       ret = odp_packet_copydata_out(pkt_table[i], 0,
> pkt_len,
>                                                 (uint8_t *)ppd.raw +
>                                                 TPACKET2_HDRLEN -
>                                                 sizeof(struct
> sockaddr_ll));
> +                       if (odp_unlikely(0 != ret))
> +                               ODP_ABORT("unable to copy data");
>
>
This particular call cannot return nonzero, so I'm not sure what this check
buys other than adding pathlength.


>                         mmap_tx_user_ready(ppd.raw);
>
> @@ -586,7 +588,7 @@ static inline unsigned pkt_mmap_v2_tx(int sock, struct
> ring *ring,
>  static void mmap_fill_ring(struct ring *ring, unsigned blocks)
>  {
>         ring->req.tp_block_size = getpagesize() << 2;
> -       ring->req.tp_frame_size = TPACKET_ALIGNMENT << 7;
> +       ring->req.tp_frame_size = TPACKET_ALIGNMENT << 8;
>

This is an obscure way of setting tp_frame_size to 4096.  Why not be direct
about it?  Is this in fact the frame size we're using, since elsewhere
PKT_BUF_SIZE is increased to 9000?


>         ring->req.tp_block_nr = blocks;
>
>         ring->req.tp_frame_nr = ring->req.tp_block_size /
> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
> index 3f9de3c..d03f527 100644
> --- a/test/validation/odp_pktio.c
> +++ b/test/validation/odp_pktio.c
> @@ -14,7 +14,7 @@
>  #include <stdlib.h>
>
>  #define PKT_BUF_NUM            32
> -#define PKT_BUF_SIZE           1856
> +#define PKT_BUF_SIZE           9000
>

If the intent is to support jumbo packets these are up to 9*1024 bytes
(9216) in size, not 9000.


>  #define MAX_NUM_IFACES         2
>  #define TEST_SEQ_INVALID       ((uint32_t)~0)
>  #define TEST_SEQ_MAGIC         0x92749451
> @@ -37,6 +37,8 @@ typedef struct {
>  typedef struct {
>         uint32be_t magic;
>         uint32be_t seq;
> +       char data[2500];
>

Where does 2500 come from?  Could use a define or at least a comment here.


> +       uint32be_t magic2;
>  } pkt_test_data_t;
>
>  /** default packet pool */
> @@ -66,6 +68,7 @@ static int pktio_pkt_set_seq(odp_packet_t pkt)
>         pkt_test_data_t data;
>
>         data.magic = TEST_SEQ_MAGIC;
> +       data.magic2 = TEST_SEQ_MAGIC;
>         data.seq   = tstseq;
>
>         l4_off = odp_packet_l4_offset(pkt);
> @@ -92,7 +95,8 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt)
>                 odp_packet_copydata_out(pkt, l4_off+ODPH_UDPHDR_LEN,
>                                         sizeof(data), &data);
>
> -               if (data.magic == TEST_SEQ_MAGIC)
> +               if (data.magic == TEST_SEQ_MAGIC &&
> +                   data.magic2 == TEST_SEQ_MAGIC)
>                         return data.seq;
>         }
>
> diff --git a/test/validation/odp_pktio_run b/test/validation/odp_pktio_run
> index 08288e6..7b80719 100755
> --- a/test/validation/odp_pktio_run
> +++ b/test/validation/odp_pktio_run
> @@ -56,8 +56,8 @@ setup_env1()
>                 echo "pktio: error: unable to create veth pair"
>                 exit $TEST_SKIPPED
>         fi
> -       ip link set $IF0 up
> -       ip link set $IF1 up
> +       ip link set $IF0 mtu 9000 up
> +       ip link set $IF1 mtu 9000 up
>
>         # network needs a little time to come up
>         sleep 1
> --
> 1.8.5.1.163.gd7aced9
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Stuart Haslam Feb. 23, 2015, 10:54 a.m. UTC | #2
On Thu, Feb 19, 2015 at 05:45:48PM +0300, Maxim Uvarov wrote:
> Support for jumbo frames for linux-generic with unsegmented buffers.
> https://bugs.linaro.org/show_bug.cgi?id=509
> 

The subject says this is mmap specific but the test code changes
aren't. I get failures for both BASIC and MMSG sockets..


  Test: pktio poll multi ...FAILED
    1. odp_pktio.c:299  - CU_FAIL("failed to receive transmitted packet")
    2. odp_pktio.c:361  - i == num_pkts
  Test: pktio sched queues ...FAILED
    1. odp_pktio.c:299  - CU_FAIL("failed to receive transmitted packet")
    2. odp_pktio.c:361  - i == num_pkts
  Test: pktio sched multi ...FAILED
    1. odp_pktio.c:299  - CU_FAIL("failed to receive transmitted packet")
    2. odp_pktio.c:361  - i == num_pkts
  Test: pktio mtu ... 9000 passed
  Test: pktio promisc mode ...passed
  Test: pktio mac ...testing mac for pktio-p0
 16:84:A9:44:19:3A passed
  Test: pktio inq_remdef ...passed

Run Summary:    Type  Total    Ran Passed Failed Inactive
              suites      1      1    n/a      0        0
               tests     13     13      9      4        0
             asserts    106    106     98      8      n/a

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  platform/linux-generic/odp_packet_socket.c | 6 ++++--
>  test/validation/odp_pktio.c                | 8 ++++++--
>  test/validation/odp_pktio_run              | 4 ++--
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/platform/linux-generic/odp_packet_socket.c b/platform/linux-generic/odp_packet_socket.c
> index 41e57c1..3e1451e 100644
> --- a/platform/linux-generic/odp_packet_socket.c
> +++ b/platform/linux-generic/odp_packet_socket.c
> @@ -554,10 +554,12 @@ static inline unsigned pkt_mmap_v2_tx(int sock, struct ring *ring,
>  			ppd.v2->tp_h.tp_snaplen = pkt_len;
>  			ppd.v2->tp_h.tp_len = pkt_len;
>  
> -			odp_packet_copydata_out(pkt_table[i], 0, pkt_len,
> +			ret = odp_packet_copydata_out(pkt_table[i], 0, pkt_len,
>  						(uint8_t *)ppd.raw +
>  						TPACKET2_HDRLEN -
>  						sizeof(struct sockaddr_ll));
> +			if (odp_unlikely(0 != ret))
> +				ODP_ABORT("unable to copy data");
>  
>  			mmap_tx_user_ready(ppd.raw);
>  
> @@ -586,7 +588,7 @@ static inline unsigned pkt_mmap_v2_tx(int sock, struct ring *ring,
>  static void mmap_fill_ring(struct ring *ring, unsigned blocks)
>  {
>  	ring->req.tp_block_size = getpagesize() << 2;
> -	ring->req.tp_frame_size = TPACKET_ALIGNMENT << 7;
> +	ring->req.tp_frame_size = TPACKET_ALIGNMENT << 8;

This is changing the frame size from 2048 to 4096, why this number? Does
this mean frames > 4096 can't be handled?

>  	ring->req.tp_block_nr = blocks;
>  
>  	ring->req.tp_frame_nr = ring->req.tp_block_size /
> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
> index 3f9de3c..d03f527 100644
> --- a/test/validation/odp_pktio.c
> +++ b/test/validation/odp_pktio.c
> @@ -14,7 +14,7 @@
>  #include <stdlib.h>
>  
>  #define PKT_BUF_NUM            32
> -#define PKT_BUF_SIZE           1856
> +#define PKT_BUF_SIZE           9000
>  #define MAX_NUM_IFACES         2
>  #define TEST_SEQ_INVALID       ((uint32_t)~0)
>  #define TEST_SEQ_MAGIC         0x92749451
> @@ -37,6 +37,8 @@ typedef struct {
>  typedef struct {
>  	uint32be_t magic;
>  	uint32be_t seq;
> +	char data[2500];
> +	uint32be_t magic2;

Why 2500? you've changed the buffer size to 9000.

Also I don't like that this change means we now use large packets for
all tests, if an interface has problems with jumbo frames it would
cause pretty much all tests to fail. I would be better to leave the
existing tests unmodified and add a new test specifically for jumbos.
Maxim Uvarov Feb. 23, 2015, 1:18 p.m. UTC | #3
On 02/23/2015 01:54 PM, Stuart Haslam wrote:
> On Thu, Feb 19, 2015 at 05:45:48PM +0300, Maxim Uvarov wrote:
>> Support for jumbo frames for linux-generic with unsegmented buffers.
>> https://bugs.linaro.org/show_bug.cgi?id=509
>>
> The subject says this is mmap specific but the test code changes
> aren't. I get failures for both BASIC and MMSG sockets..
>
>
>    Test: pktio poll multi ...FAILED
>      1. odp_pktio.c:299  - CU_FAIL("failed to receive transmitted packet")
>      2. odp_pktio.c:361  - i == num_pkts
>    Test: pktio sched queues ...FAILED
>      1. odp_pktio.c:299  - CU_FAIL("failed to receive transmitted packet")
>      2. odp_pktio.c:361  - i == num_pkts
>    Test: pktio sched multi ...FAILED
>      1. odp_pktio.c:299  - CU_FAIL("failed to receive transmitted packet")
>      2. odp_pktio.c:361  - i == num_pkts
>    Test: pktio mtu ... 9000 passed
>    Test: pktio promisc mode ...passed
>    Test: pktio mac ...testing mac for pktio-p0
>   16:84:A9:44:19:3A passed
>    Test: pktio inq_remdef ...passed
>
> Run Summary:    Type  Total    Ran Passed Failed Inactive
>                suites      1      1    n/a      0        0
>                 tests     13     13      9      4        0
>               asserts    106    106     98      8      n/a

To remove that errors you need to apply second patch.

>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   platform/linux-generic/odp_packet_socket.c | 6 ++++--
>>   test/validation/odp_pktio.c                | 8 ++++++--
>>   test/validation/odp_pktio_run              | 4 ++--
>>   3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_packet_socket.c b/platform/linux-generic/odp_packet_socket.c
>> index 41e57c1..3e1451e 100644
>> --- a/platform/linux-generic/odp_packet_socket.c
>> +++ b/platform/linux-generic/odp_packet_socket.c
>> @@ -554,10 +554,12 @@ static inline unsigned pkt_mmap_v2_tx(int sock, struct ring *ring,
>>   			ppd.v2->tp_h.tp_snaplen = pkt_len;
>>   			ppd.v2->tp_h.tp_len = pkt_len;
>>   
>> -			odp_packet_copydata_out(pkt_table[i], 0, pkt_len,
>> +			ret = odp_packet_copydata_out(pkt_table[i], 0, pkt_len,
>>   						(uint8_t *)ppd.raw +
>>   						TPACKET2_HDRLEN -
>>   						sizeof(struct sockaddr_ll));
>> +			if (odp_unlikely(0 != ret))
>> +				ODP_ABORT("unable to copy data");
>>   
>>   			mmap_tx_user_ready(ppd.raw);
>>   
>> @@ -586,7 +588,7 @@ static inline unsigned pkt_mmap_v2_tx(int sock, struct ring *ring,
>>   static void mmap_fill_ring(struct ring *ring, unsigned blocks)
>>   {
>>   	ring->req.tp_block_size = getpagesize() << 2;
>> -	ring->req.tp_frame_size = TPACKET_ALIGNMENT << 7;
>> +	ring->req.tp_frame_size = TPACKET_ALIGNMENT << 8;
> This is changing the frame size from 2048 to 4096, why this number? Does
> this mean frames > 4096 can't be handled?
>
>>   	ring->req.tp_block_nr = blocks;
>>   
>>   	ring->req.tp_frame_nr = ring->req.tp_block_size /
>> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
>> index 3f9de3c..d03f527 100644
>> --- a/test/validation/odp_pktio.c
>> +++ b/test/validation/odp_pktio.c
>> @@ -14,7 +14,7 @@
>>   #include <stdlib.h>
>>   
>>   #define PKT_BUF_NUM            32
>> -#define PKT_BUF_SIZE           1856
>> +#define PKT_BUF_SIZE           9000
>>   #define MAX_NUM_IFACES         2
>>   #define TEST_SEQ_INVALID       ((uint32_t)~0)
>>   #define TEST_SEQ_MAGIC         0x92749451
>> @@ -37,6 +37,8 @@ typedef struct {
>>   typedef struct {
>>   	uint32be_t magic;
>>   	uint32be_t seq;
>> +	char data[2500];
>> +	uint32be_t magic2;
> Why 2500? you've changed the buffer size to 9000.
Stuart, please review updated patch. I fixed number as Bill noted to 
support 9216 bytes packets.

> Also I don't like that this change means we now use large packets for
> all tests, if an interface has problems with jumbo frames it would
> cause pretty much all tests to fail. I would be better to leave the
> existing tests unmodified and add a new test specifically for jumbos.
>
Ok, I can modify existence test to use small frames and jumbo also.

Maxim.
diff mbox

Patch

diff --git a/platform/linux-generic/odp_packet_socket.c b/platform/linux-generic/odp_packet_socket.c
index 41e57c1..3e1451e 100644
--- a/platform/linux-generic/odp_packet_socket.c
+++ b/platform/linux-generic/odp_packet_socket.c
@@ -554,10 +554,12 @@  static inline unsigned pkt_mmap_v2_tx(int sock, struct ring *ring,
 			ppd.v2->tp_h.tp_snaplen = pkt_len;
 			ppd.v2->tp_h.tp_len = pkt_len;
 
-			odp_packet_copydata_out(pkt_table[i], 0, pkt_len,
+			ret = odp_packet_copydata_out(pkt_table[i], 0, pkt_len,
 						(uint8_t *)ppd.raw +
 						TPACKET2_HDRLEN -
 						sizeof(struct sockaddr_ll));
+			if (odp_unlikely(0 != ret))
+				ODP_ABORT("unable to copy data");
 
 			mmap_tx_user_ready(ppd.raw);
 
@@ -586,7 +588,7 @@  static inline unsigned pkt_mmap_v2_tx(int sock, struct ring *ring,
 static void mmap_fill_ring(struct ring *ring, unsigned blocks)
 {
 	ring->req.tp_block_size = getpagesize() << 2;
-	ring->req.tp_frame_size = TPACKET_ALIGNMENT << 7;
+	ring->req.tp_frame_size = TPACKET_ALIGNMENT << 8;
 	ring->req.tp_block_nr = blocks;
 
 	ring->req.tp_frame_nr = ring->req.tp_block_size /
diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
index 3f9de3c..d03f527 100644
--- a/test/validation/odp_pktio.c
+++ b/test/validation/odp_pktio.c
@@ -14,7 +14,7 @@ 
 #include <stdlib.h>
 
 #define PKT_BUF_NUM            32
-#define PKT_BUF_SIZE           1856
+#define PKT_BUF_SIZE           9000
 #define MAX_NUM_IFACES         2
 #define TEST_SEQ_INVALID       ((uint32_t)~0)
 #define TEST_SEQ_MAGIC         0x92749451
@@ -37,6 +37,8 @@  typedef struct {
 typedef struct {
 	uint32be_t magic;
 	uint32be_t seq;
+	char data[2500];
+	uint32be_t magic2;
 } pkt_test_data_t;
 
 /** default packet pool */
@@ -66,6 +68,7 @@  static int pktio_pkt_set_seq(odp_packet_t pkt)
 	pkt_test_data_t data;
 
 	data.magic = TEST_SEQ_MAGIC;
+	data.magic2 = TEST_SEQ_MAGIC;
 	data.seq   = tstseq;
 
 	l4_off = odp_packet_l4_offset(pkt);
@@ -92,7 +95,8 @@  static uint32_t pktio_pkt_seq(odp_packet_t pkt)
 		odp_packet_copydata_out(pkt, l4_off+ODPH_UDPHDR_LEN,
 					sizeof(data), &data);
 
-		if (data.magic == TEST_SEQ_MAGIC)
+		if (data.magic == TEST_SEQ_MAGIC &&
+		    data.magic2 == TEST_SEQ_MAGIC)
 			return data.seq;
 	}
 
diff --git a/test/validation/odp_pktio_run b/test/validation/odp_pktio_run
index 08288e6..7b80719 100755
--- a/test/validation/odp_pktio_run
+++ b/test/validation/odp_pktio_run
@@ -56,8 +56,8 @@  setup_env1()
 		echo "pktio: error: unable to create veth pair"
 		exit $TEST_SKIPPED
 	fi
-	ip link set $IF0 up
-	ip link set $IF1 up
+	ip link set $IF0 mtu 9000 up
+	ip link set $IF1 mtu 9000 up
 
 	# network needs a little time to come up
 	sleep 1