Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
Hi Ilya, I had completely missed your previous patch. You could also remove the two errno value checks in pktio_test_send_failure(). They are not defined in the API, so they should not be tested. -Matias > -Original Message- > From: EXT Ilya Maximets [mailto:i.maxim...@samsung.com] > Sent: Tuesday, December 08, 2015 12:44 PM > To: Elo, Matias (Nokia - FI/Espoo); lng- > o...@lists.linaro.org > Subject: Re: validation: pktio: fix start_stop and send_failure tests > > Matias, > please check out my patch ( https://lists.linaro.org/pipermail/lng-odp/2015- > December/018057.html ) > from my 'TAP pktio' patch set. > > Best regards, Ilya Maximets. > > On 08.12.2015 13:36, Elo, Matias (Nokia - FI/Espoo) wrote: > > In both tests set MAC addresses to test packets to enable > > testing with real pktio interfaces. > > > > In pktio_test_send_failure() don't check for errno values > > as they are not defined in the API. > > > > Signed-off-by: Matias Elo > > --- > > test/validation/pktio/pktio.c | 26 +- > > 1 file changed, 17 insertions(+), 9 deletions(-) > > > > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > > index 50a6d8a..4ded924 100644 > > --- a/test/validation/pktio/pktio.c > > +++ b/test/validation/pktio/pktio.c > > @@ -695,6 +695,7 @@ void pktio_test_inq(void) > > static void pktio_test_start_stop(void) > > { > > odp_pktio_t pktio[MAX_NUM_IFACES]; > > + pktio_info_t pktio_info[MAX_NUM_IFACES]; > > odp_packet_t pkt; > > odp_event_t tx_ev[1000]; > > odp_event_t ev; > > @@ -705,6 +706,7 @@ static void pktio_test_start_stop(void) > > for (i = 0; i < num_ifaces; i++) { > > pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); > > CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); > > + pktio_info[i].id = pktio[i]; > > create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); > > } > > > > @@ -724,6 +726,8 @@ static void pktio_test_start_stop(void) > > if (pkt == ODP_PACKET_INVALID) > > break; > > pktio_init_packet(pkt); > > + pktio_pkt_set_macs(pkt, _info[0], > > + _info[1]); > > tx_ev[alloc] = odp_packet_to_event(pkt); > > } > > > > @@ -775,6 +779,9 @@ static void pktio_test_start_stop(void) > > if (pkt == ODP_PACKET_INVALID) > > break; > > pktio_init_packet(pkt); > > + if (num_ifaces > 1) > > + pktio_pkt_set_macs(pkt, _info[0], > > + _info[1]); > > tx_ev[alloc] = odp_packet_to_event(pkt); > > } > > > > @@ -849,13 +856,14 @@ static void pktio_test_send_failure(void) > > odp_pool_param_t pool_params; > > odp_pool_t pkt_pool; > > int long_pkt_idx = TX_BATCH_LEN / 2; > > - pktio_info_t info_rx; > > + pktio_info_t info_tx, info_rx; > > > > pktio_tx = create_pktio(0, ODP_PKTIN_MODE_RECV); > > if (pktio_tx == ODP_PKTIO_INVALID) { > > CU_FAIL("failed to open pktio"); > > return; > > } > > + info_tx.id = pktio_tx; > > > > /* read the MTU from the transmit interface */ > > mtu = odp_pktio_mtu(pktio_tx); > > @@ -880,6 +888,10 @@ static void pktio_test_send_failure(void) > > } else { > > pktio_rx = pktio_tx; > > } > > + info_rx.id = pktio_rx; > > + info_rx.outq = ODP_QUEUE_INVALID; > > + info_rx.inq = ODP_QUEUE_INVALID; > > + info_rx.in_mode = ODP_PKTIN_MODE_RECV; > > > > /* generate a batch of packets with a single overly long packet > > * in the middle */ > > @@ -898,6 +910,8 @@ static void pktio_test_send_failure(void) > > pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); > > if (pkt_seq[i] == TEST_SEQ_INVALID) > > break; > > + > > + pktio_pkt_set_macs(pkt_tbl[i], _tx, _rx); > > } > > alloc_pkts = i; > > > > @@ -907,12 +921,6 @@ static void pktio_test_send_failure(void) > > odp_errno_zero(); > > ret = odp_pktio_send(pktio_tx, pkt_tbl, TX_BATCH_LEN); > > CU_ASSERT(ret == long_pkt_idx); > > - CU_ASSERT(odp_errno() == 0); > > - > > - info_rx.id = pktio_rx; > > - info_rx.outq = ODP_QUEUE_INVALID; > > - info_rx.inq = ODP_QUEUE_INVALID; > > - info_rx.in_mode = ODP_PKTIN_MODE_RECV; > > > > for (i = 0; i < ret; ++i) { > > pkt_tbl[i] = wait_for_packet(_rx, pkt_seq[i], > > @@ -929,9 +937,8 @@ static void pktio_test_send_failure(void) > > _tbl[long_pkt_idx], > > TX_BATCH_LEN - long_pkt_idx); > > CU_ASSERT(ret == -1); > > - CU_ASSERT(odp_errno() != 0); > > } else { > > -
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
Matias, I think, better to make separate patch for that. It will not intersect with mine in case of only 2 deletions. Would you prepare it? Best regards, Ilya Maximets. On 08.12.2015 15:21, Elo, Matias (Nokia - FI/Espoo) wrote: > Hi Ilya, > > I had completely missed your previous patch. You could also remove the two > errno value checks in pktio_test_send_failure(). They are not defined in the > API, so they should not be tested. > > -Matias > >> -Original Message- >> From: EXT Ilya Maximets [mailto:i.maxim...@samsung.com] >> Sent: Tuesday, December 08, 2015 12:44 PM >> To: Elo, Matias (Nokia - FI/Espoo); lng- >> o...@lists.linaro.org >> Subject: Re: validation: pktio: fix start_stop and send_failure tests >> >> Matias, >> please check out my patch ( https://lists.linaro.org/pipermail/lng-odp/2015- >> December/018057.html ) >> from my 'TAP pktio' patch set. >> >> Best regards, Ilya Maximets. >> >> On 08.12.2015 13:36, Elo, Matias (Nokia - FI/Espoo) wrote: >>> In both tests set MAC addresses to test packets to enable >>> testing with real pktio interfaces. >>> >>> In pktio_test_send_failure() don't check for errno values >>> as they are not defined in the API. >>> >>> Signed-off-by: Matias Elo >>> --- >>> test/validation/pktio/pktio.c | 26 +- >>> 1 file changed, 17 insertions(+), 9 deletions(-) >>> >>> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c >>> index 50a6d8a..4ded924 100644 >>> --- a/test/validation/pktio/pktio.c >>> +++ b/test/validation/pktio/pktio.c >>> @@ -695,6 +695,7 @@ void pktio_test_inq(void) >>> static void pktio_test_start_stop(void) >>> { >>> odp_pktio_t pktio[MAX_NUM_IFACES]; >>> + pktio_info_t pktio_info[MAX_NUM_IFACES]; >>> odp_packet_t pkt; >>> odp_event_t tx_ev[1000]; >>> odp_event_t ev; >>> @@ -705,6 +706,7 @@ static void pktio_test_start_stop(void) >>> for (i = 0; i < num_ifaces; i++) { >>> pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); >>> CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); >>> + pktio_info[i].id = pktio[i]; >>> create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); >>> } >>> >>> @@ -724,6 +726,8 @@ static void pktio_test_start_stop(void) >>> if (pkt == ODP_PACKET_INVALID) >>> break; >>> pktio_init_packet(pkt); >>> + pktio_pkt_set_macs(pkt, _info[0], >>> + _info[1]); >>> tx_ev[alloc] = odp_packet_to_event(pkt); >>> } >>> >>> @@ -775,6 +779,9 @@ static void pktio_test_start_stop(void) >>> if (pkt == ODP_PACKET_INVALID) >>> break; >>> pktio_init_packet(pkt); >>> + if (num_ifaces > 1) >>> + pktio_pkt_set_macs(pkt, _info[0], >>> + _info[1]); >>> tx_ev[alloc] = odp_packet_to_event(pkt); >>> } >>> >>> @@ -849,13 +856,14 @@ static void pktio_test_send_failure(void) >>> odp_pool_param_t pool_params; >>> odp_pool_t pkt_pool; >>> int long_pkt_idx = TX_BATCH_LEN / 2; >>> - pktio_info_t info_rx; >>> + pktio_info_t info_tx, info_rx; >>> >>> pktio_tx = create_pktio(0, ODP_PKTIN_MODE_RECV); >>> if (pktio_tx == ODP_PKTIO_INVALID) { >>> CU_FAIL("failed to open pktio"); >>> return; >>> } >>> + info_tx.id = pktio_tx; >>> >>> /* read the MTU from the transmit interface */ >>> mtu = odp_pktio_mtu(pktio_tx); >>> @@ -880,6 +888,10 @@ static void pktio_test_send_failure(void) >>> } else { >>> pktio_rx = pktio_tx; >>> } >>> + info_rx.id = pktio_rx; >>> + info_rx.outq = ODP_QUEUE_INVALID; >>> + info_rx.inq = ODP_QUEUE_INVALID; >>> + info_rx.in_mode = ODP_PKTIN_MODE_RECV; >>> >>> /* generate a batch of packets with a single overly long packet >>> * in the middle */ >>> @@ -898,6 +910,8 @@ static void pktio_test_send_failure(void) >>> pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); >>> if (pkt_seq[i] == TEST_SEQ_INVALID) >>> break; >>> + >>> + pktio_pkt_set_macs(pkt_tbl[i], _tx, _rx); >>> } >>> alloc_pkts = i; >>> >>> @@ -907,12 +921,6 @@ static void pktio_test_send_failure(void) >>> odp_errno_zero(); >>> ret = odp_pktio_send(pktio_tx, pkt_tbl, TX_BATCH_LEN); >>> CU_ASSERT(ret == long_pkt_idx); >>> - CU_ASSERT(odp_errno() == 0); >>> - >>> - info_rx.id = pktio_rx; >>> - info_rx.outq = ODP_QUEUE_INVALID; >>> - info_rx.inq = ODP_QUEUE_INVALID; >>> - info_rx.in_mode = ODP_PKTIN_MODE_RECV; >>> >>> for (i = 0; i < ret; ++i) { >>> pkt_tbl[i] = wait_for_packet(_rx, pkt_seq[i], >>> @@ -929,9 +937,8 @@ static void pktio_test_send_failure(void) >>>
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
Matias, please check out my patch ( https://lists.linaro.org/pipermail/lng-odp/2015-December/018057.html ) from my 'TAP pktio' patch set. Best regards, Ilya Maximets. On 08.12.2015 13:36, Elo, Matias (Nokia - FI/Espoo) wrote: > In both tests set MAC addresses to test packets to enable > testing with real pktio interfaces. > > In pktio_test_send_failure() don't check for errno values > as they are not defined in the API. > > Signed-off-by: Matias Elo> --- > test/validation/pktio/pktio.c | 26 +- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index 50a6d8a..4ded924 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -695,6 +695,7 @@ void pktio_test_inq(void) > static void pktio_test_start_stop(void) > { > odp_pktio_t pktio[MAX_NUM_IFACES]; > + pktio_info_t pktio_info[MAX_NUM_IFACES]; > odp_packet_t pkt; > odp_event_t tx_ev[1000]; > odp_event_t ev; > @@ -705,6 +706,7 @@ static void pktio_test_start_stop(void) > for (i = 0; i < num_ifaces; i++) { > pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); > CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); > + pktio_info[i].id = pktio[i]; > create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); > } > > @@ -724,6 +726,8 @@ static void pktio_test_start_stop(void) > if (pkt == ODP_PACKET_INVALID) > break; > pktio_init_packet(pkt); > + pktio_pkt_set_macs(pkt, _info[0], > +_info[1]); > tx_ev[alloc] = odp_packet_to_event(pkt); > } > > @@ -775,6 +779,9 @@ static void pktio_test_start_stop(void) > if (pkt == ODP_PACKET_INVALID) > break; > pktio_init_packet(pkt); > + if (num_ifaces > 1) > + pktio_pkt_set_macs(pkt, _info[0], > +_info[1]); > tx_ev[alloc] = odp_packet_to_event(pkt); > } > > @@ -849,13 +856,14 @@ static void pktio_test_send_failure(void) > odp_pool_param_t pool_params; > odp_pool_t pkt_pool; > int long_pkt_idx = TX_BATCH_LEN / 2; > - pktio_info_t info_rx; > + pktio_info_t info_tx, info_rx; > > pktio_tx = create_pktio(0, ODP_PKTIN_MODE_RECV); > if (pktio_tx == ODP_PKTIO_INVALID) { > CU_FAIL("failed to open pktio"); > return; > } > + info_tx.id = pktio_tx; > > /* read the MTU from the transmit interface */ > mtu = odp_pktio_mtu(pktio_tx); > @@ -880,6 +888,10 @@ static void pktio_test_send_failure(void) > } else { > pktio_rx = pktio_tx; > } > + info_rx.id = pktio_rx; > + info_rx.outq = ODP_QUEUE_INVALID; > + info_rx.inq = ODP_QUEUE_INVALID; > + info_rx.in_mode = ODP_PKTIN_MODE_RECV; > > /* generate a batch of packets with a single overly long packet >* in the middle */ > @@ -898,6 +910,8 @@ static void pktio_test_send_failure(void) > pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); > if (pkt_seq[i] == TEST_SEQ_INVALID) > break; > + > + pktio_pkt_set_macs(pkt_tbl[i], _tx, _rx); > } > alloc_pkts = i; > > @@ -907,12 +921,6 @@ static void pktio_test_send_failure(void) > odp_errno_zero(); > ret = odp_pktio_send(pktio_tx, pkt_tbl, TX_BATCH_LEN); > CU_ASSERT(ret == long_pkt_idx); > - CU_ASSERT(odp_errno() == 0); > - > - info_rx.id = pktio_rx; > - info_rx.outq = ODP_QUEUE_INVALID; > - info_rx.inq = ODP_QUEUE_INVALID; > - info_rx.in_mode = ODP_PKTIN_MODE_RECV; > > for (i = 0; i < ret; ++i) { > pkt_tbl[i] = wait_for_packet(_rx, pkt_seq[i], > @@ -929,9 +937,8 @@ static void pktio_test_send_failure(void) >_tbl[long_pkt_idx], >TX_BATCH_LEN - long_pkt_idx); > CU_ASSERT(ret == -1); > - CU_ASSERT(odp_errno() != 0); > } else { > - CU_FAIL("failed to receive transmitted packets\n"); > + CU_FAIL("failed to receive transmitted packets"); > } > > /* now reduce the size of the long packet and attempt to send > @@ -942,6 +949,7 @@ static void pktio_test_send_failure(void) >PKT_LEN_NORMAL); > pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); > CU_ASSERT_FATAL(pkt_seq[i] != TEST_SEQ_INVALID); > + pktio_pkt_set_macs(pkt_tbl[i], _tx, _rx); > ret = odp_pktio_send(pktio_tx,
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
On 8 December 2015 at 10:17, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolai...@nokia.com> wrote: > > > > -Original Message- > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT > > Ilya Maximets > > Sent: Tuesday, December 08, 2015 4:48 PM > > To: Stuart Haslam; Elo, Matias (Nokia - FI/Espoo) > > Cc: lng-odp@lists.linaro.org > > Subject: Re: [lng-odp] validation: pktio: fix start_stop and send_failure > > tests > > > > > > > > On 08.12.2015 17:38, Stuart Haslam wrote: > > > On Tue, Dec 08, 2015 at 12:21:06PM +, Elo, Matias (Nokia - > FI/Espoo) > > wrote: > > >> Hi Ilya, > > >> > > >> I had completely missed your previous patch. You could also remove the > > two errno value checks in pktio_test_send_failure(). They are not defined > > in the API, so they should not be tested. > > >> > > >> -Matias > > >> > > > > > > It realise it's back-to-front, but IMO it would be better to document > > > that the errno must be set. > > > > > > > I agree with that. There is so little set of functions, according to > > documentation, sets up errno. > > My opinion is that almost all functions should set it on errors, > otherwise > > it useless. > > > > Best regards, Ilya Maximets. > > > The problem with errno is that we'd need to define also a set of errno > values and usage of those in a way that is valid for all implementations. > Specifying errno's is easy on a single implementation but same errors may > not be easily checked over multiple implementations. > > So in this phase, When would we tackle the next phase if we don't start creating the supported list now ? The rationale that not all platforms support it will still be true then. > I'd specify that generally ODP functions that return "failure" may set > odp_errno with implementation specific, non-zero value. Errno could be > logged and used for debugging, but not tested against in an application. > App would always test e.g. ret < 0. > > -Petri > > > > > > ___ > 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 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
On 08.12.2015 17:38, Stuart Haslam wrote: > On Tue, Dec 08, 2015 at 12:21:06PM +, Elo, Matias (Nokia - FI/Espoo) > wrote: >> Hi Ilya, >> >> I had completely missed your previous patch. You could also remove the two >> errno value checks in pktio_test_send_failure(). They are not defined in the >> API, so they should not be tested. >> >> -Matias >> > > It realise it's back-to-front, but IMO it would be better to document > that the errno must be set. > I agree with that. There is so little set of functions, according to documentation, sets up errno. My opinion is that almost all functions should set it on errors, otherwise it useless. Best regards, Ilya Maximets. ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
> -Original Message- > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT > Ilya Maximets > Sent: Tuesday, December 08, 2015 4:48 PM > To: Stuart Haslam; Elo, Matias (Nokia - FI/Espoo) > Cc: lng-odp@lists.linaro.org > Subject: Re: [lng-odp] validation: pktio: fix start_stop and send_failure > tests > > > > On 08.12.2015 17:38, Stuart Haslam wrote: > > On Tue, Dec 08, 2015 at 12:21:06PM +, Elo, Matias (Nokia - FI/Espoo) > wrote: > >> Hi Ilya, > >> > >> I had completely missed your previous patch. You could also remove the > two errno value checks in pktio_test_send_failure(). They are not defined > in the API, so they should not be tested. > >> > >> -Matias > >> > > > > It realise it's back-to-front, but IMO it would be better to document > > that the errno must be set. > > > > I agree with that. There is so little set of functions, according to > documentation, sets up errno. > My opinion is that almost all functions should set it on errors, otherwise > it useless. > > Best regards, Ilya Maximets. The problem with errno is that we'd need to define also a set of errno values and usage of those in a way that is valid for all implementations. Specifying errno's is easy on a single implementation but same errors may not be easily checked over multiple implementations. So in this phase, I'd specify that generally ODP functions that return "failure" may set odp_errno with implementation specific, non-zero value. Errno could be logged and used for debugging, but not tested against in an application. App would always test e.g. ret < 0. -Petri ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
On Tue, Dec 08, 2015 at 12:21:06PM +, Elo, Matias (Nokia - FI/Espoo) wrote: > Hi Ilya, > > I had completely missed your previous patch. You could also remove the two > errno value checks in pktio_test_send_failure(). They are not defined in the > API, so they should not be tested. > > -Matias > It realise it's back-to-front, but IMO it would be better to document that the errno must be set. -- Stuart. ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
On Tue, Dec 08, 2015 at 03:17:46PM +, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > -Original Message- > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT > > Ilya Maximets > > Sent: Tuesday, December 08, 2015 4:48 PM > > To: Stuart Haslam; Elo, Matias (Nokia - FI/Espoo) > > Cc: lng-odp@lists.linaro.org > > Subject: Re: [lng-odp] validation: pktio: fix start_stop and send_failure > > tests > > > > > > > > On 08.12.2015 17:38, Stuart Haslam wrote: > > > On Tue, Dec 08, 2015 at 12:21:06PM +, Elo, Matias (Nokia - FI/Espoo) > > wrote: > > >> Hi Ilya, > > >> > > >> I had completely missed your previous patch. You could also remove the > > two errno value checks in pktio_test_send_failure(). They are not defined > > in the API, so they should not be tested. > > >> > > >> -Matias > > >> > > > > > > It realise it's back-to-front, but IMO it would be better to document > > > that the errno must be set. > > > > > > > I agree with that. There is so little set of functions, according to > > documentation, sets up errno. > > My opinion is that almost all functions should set it on errors, otherwise > > it useless. > > > > Best regards, Ilya Maximets. > > > The problem with errno is that we'd need to define also a set of errno values > and usage of those in a way that is valid for all implementations. Specifying > errno's is easy on a single implementation but same errors may not be easily > checked over multiple implementations. > > So in this phase, I'd specify that generally ODP functions that return > "failure" may set odp_errno with implementation specific, non-zero value. > Errno could be logged and used for debugging, but not tested against in an > application. App would always test e.g. ret < 0. > > -Petri I agree that for now specifying particular errno values is probably a step too far, but it doesn't seem unreasonable to specify that if an API function returns an error indication (in this case retval < 0) then it must also set an errno. The current test is checking only that odp_errno() != 0 on failure, and is == 0 on success. -- Stuart. ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp