On 03/27/15 17:11, Stuart Haslam wrote:
It doesn't break non-jumbo tests, in fact all tests pass and I didn't
*notice* any adverse effects, I found the issue by inspection.
The problem is that when generating non-jumbo packets (which most of
the tests do) the magic2 marker is written beyond the end of a
calloc'd block into un-allocated memory. The receiver doesn't check
magic2 for non-jumbo packets.
The issue could be trivially avoided with the simpler change below,
but I wasn't happy with that as we'd still have a pointer to a
structure that's much larger than the memory allocated.
Stuart.
Yes this change is ok. Did that the same as
https://bugs.linaro.org/show_bug.cgi?id=1365
?
Maxim.
diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
index e022c33..5ea45e4 100644
--- a/test/validation/odp_pktio.c
+++ b/test/validation/odp_pktio.c
@@ -95,7 +95,8 @@ static int pktio_pkt_set_seq(odp_packet_t pkt)
CU_ASSERT_FATAL(data != NULL);
data->head.magic = TEST_SEQ_MAGIC;
- data->magic2 = TEST_SEQ_MAGIC;
+ if (test_jumbo)
+ data->magic2 = TEST_SEQ_MAGIC;
data->head.seq = tstseq;
odp_packet_copydata_in(pkt, l4_off+ODPH_UDPHDR_LEN,
On 27 March 2015 at 13:15, Bill Fischofer <[email protected]
<mailto:[email protected]>> wrote:
If I understand Stuart's patch correctly, he's saying that the
introduction of jumbo frame support in v1.0.1 broke non-jumbo
processing. Since that's a critical function I'd think we'd want
to fix that bug ASAP in v1.0.2.
On Fri, Mar 27, 2015 at 7:04 AM, Mike Holmes
<[email protected] <mailto:[email protected]>> wrote:
On 26 March 2015 at 17:14, Bill Fischofer
<[email protected] <mailto:[email protected]>>
wrote:
Sounds like we should hold v1.0.2 for this bug fix?
Maxim: I think this review is properly yours as you did
the jumbo frame add for v1.0.1.
Bill can you describe the use case than for holding, if we
have a strong case for it then we can hold.
Normal procedure is that whatever makes the cut is there and
the release goes out unless there is a regression introduced.
On Thu, Mar 26, 2015 at 12:52 PM, Stuart Haslam
<[email protected]
<mailto:[email protected]>> wrote:
A magic number, used to ensure jumbo frames are
transmitted and received
in full, is written to the end of a packet buffer at a
fixed offset of
9170 bytes. However for non-jumbo tests this is way
beyond the end of
the allocated buffer.
Signed-off-by: Stuart Haslam <[email protected]
<mailto:[email protected]>>
---
test/validation/odp_pktio.c | 84
++++++++++++++++++++++++++++-----------------
1 file changed, 52 insertions(+), 32 deletions(-)
diff --git a/test/validation/odp_pktio.c
b/test/validation/odp_pktio.c
index e022c33..d653da4 100644
--- a/test/validation/odp_pktio.c
+++ b/test/validation/odp_pktio.c
@@ -44,7 +44,6 @@ typedef struct ODP_PACKED {
/** structure of test packet UDP payload */
typedef struct ODP_PACKED {
- pkt_head_t head;
char data[PKT_BUF_JUMBO_MAX_PAYLOAD -
sizeof(pkt_head_t) -
sizeof(uint32be_t)];
uint32be_t magic2;
@@ -74,33 +73,47 @@ static void
pktio_pkt_set_macs(odp_packet_t pkt,
static uint32_t pkt_payload_len(void)
{
- return test_jumbo ? sizeof(pkt_test_data_t) :
sizeof(pkt_head_t);
+ uint32_t len;
+
+ len = sizeof(pkt_head_t);
+
+ if (test_jumbo)
+ len += sizeof(pkt_test_data_t);
+
+ return len;
}
static int pktio_pkt_set_seq(odp_packet_t pkt)
{
static uint32_t tstseq;
- size_t l4_off;
- pkt_test_data_t *data;
- uint32_t len = pkt_payload_len();
-
+ size_t off;
+ pkt_head_t head;
- l4_off = odp_packet_l4_offset(pkt);
- if (!l4_off) {
+ off = odp_packet_l4_offset(pkt);
+ if (!off) {
CU_FAIL("packet L4 offset not set");
return -1;
}
- data = calloc(1, len);
- CU_ASSERT_FATAL(data != NULL);
+ head.magic = TEST_SEQ_MAGIC;
+ head.seq = tstseq;
+
+ off += ODPH_UDPHDR_LEN;
+ odp_packet_copydata_in(pkt, off, sizeof(head), &head);
+
+ if (test_jumbo) {
+ pkt_test_data_t *data;
+
+ data = calloc(1, sizeof(*data));
+ CU_ASSERT_FATAL(data != NULL);
- data->head.magic = TEST_SEQ_MAGIC;
- data->magic2 = TEST_SEQ_MAGIC;
- data->head.seq = tstseq;
+ data->magic2 = TEST_SEQ_MAGIC;
+
+ off += sizeof(head);
+ odp_packet_copydata_in(pkt, off, sizeof(*data), data);
+ free(data);
+ }
- odp_packet_copydata_in(pkt, l4_off+ODPH_UDPHDR_LEN,
- len, data);
- free(data);
tstseq++;
return 0;
@@ -108,30 +121,37 @@ static int
pktio_pkt_set_seq(odp_packet_t pkt)
static uint32_t pktio_pkt_seq(odp_packet_t pkt)
{
- size_t l4_off;
+ size_t off;
uint32_t seq = TEST_SEQ_INVALID;
- pkt_test_data_t *data;
- uint32_t len = pkt_payload_len();
+ pkt_head_t head;
- l4_off = odp_packet_l4_offset(pkt);
- if (l4_off == ODP_PACKET_OFFSET_INVALID)
+ off = odp_packet_l4_offset(pkt);
+ if (off == ODP_PACKET_OFFSET_INVALID)
return TEST_SEQ_INVALID;
- data = calloc(1, len);
- CU_ASSERT_FATAL(data != NULL);
+ off += ODPH_UDPHDR_LEN;
+ odp_packet_copydata_out(pkt, off, sizeof(head), &head);
- odp_packet_copydata_out(pkt, l4_off+ODPH_UDPHDR_LEN,
- len, data);
+ if (head.magic != TEST_SEQ_MAGIC)
+ return TEST_SEQ_INVALID;
- if (data->head.magic == TEST_SEQ_MAGIC) {
- if (test_jumbo && data->magic2 !=
TEST_SEQ_MAGIC) {
- free(data);
- return TEST_SEQ_INVALID;
- }
- seq = data->head.seq;
+ seq = head.seq;
+
+ if (test_jumbo) {
+ pkt_test_data_t *data;
+
+ data = calloc(1, sizeof(*data));
+ CU_ASSERT_FATAL(data != NULL);
+
+ off += sizeof(head);
+ odp_packet_copydata_out(pkt, off, sizeof(*data), data);
+
+ if (data->magic2 != TEST_SEQ_MAGIC)
+ seq = TEST_SEQ_INVALID;
+
+ free(data);
}
- free(data);
return seq;
}
--
2.1.1
_______________________________________________
lng-odp mailing list
[email protected] <mailto:[email protected]>
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected] <mailto:[email protected]>
http://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
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
