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]> wrote: > > > On 26 March 2015 at 17:14, Bill Fischofer <[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] >> > 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]> >>> --- >>> 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] >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> >> _______________________________________________ >> lng-odp mailing list >> [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
