| Message ID | 1421413097-17325-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Accepted |
| Commit | bd5224e28a8dfc1853402a4d7225aec869befe5a |
| Headers | show |
On 01/16/2015 03:58 PM, Maxim Uvarov wrote: > Correctly remove queue from packet i/o and remove it from scheduler. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > v8: fixed Stuart comments and added test. Implementation of odp_pktio_inq_remdef > also fixes segfault in existance pktio test. Looks like I thought about something else. I should say: because of odp_pktio_inq_remdef was not implemented before, ODP can have special conditions where odp_schedule() for closed pktio can pull from old incoming queue. In linux-generic implementation packet recv() will be called and implementation will try to place packet to destroyed queue. So there is segfault. Because current odp examples do not destroy/create packet i/o and then call scheduler we did not see that segfault. So in this patch I implemented odp_pktio_inq_rmdef and added tests case for odp_schedule on closed packet i.o. Maxim. > .../linux-generic/include/odp_queue_internal.h | 10 ++++++++ > platform/linux-generic/odp_packet_io.c | 29 +++++++++++++++++++++- > platform/linux-generic/odp_schedule.c | 5 ++++ > test/validation/odp_pktio.c | 17 +++++++++++++ > 4 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h > index d5c8e4e..dbc42c0 100644 > --- a/platform/linux-generic/include/odp_queue_internal.h > +++ b/platform/linux-generic/include/odp_queue_internal.h > @@ -129,6 +129,16 @@ static inline int queue_is_destroyed(odp_queue_t handle) > > return queue->s.status == QUEUE_STATUS_DESTROYED; > } > + > +static inline int queue_is_sched(odp_queue_t handle) > +{ > + queue_entry_t *queue; > + > + queue = queue_to_qentry(handle); > + > + return ((queue->s.status == QUEUE_STATUS_SCHED) && > + (queue->s.pktin != ODP_PKTIO_INVALID)); > +} > #ifdef __cplusplus > } > #endif > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index cd109d2..04de756 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -429,7 +429,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue) > > int odp_pktio_inq_remdef(odp_pktio_t id) > { > - return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID); > + pktio_entry_t *pktio_entry = get_pktio_entry(id); > + odp_queue_t queue; > + queue_entry_t *qentry; > + > + if (pktio_entry == NULL) > + return -1; > + > + lock_entry(pktio_entry); > + queue = pktio_entry->s.inq_default; > + qentry = queue_to_qentry(queue); > + > + queue_lock(qentry); > + if (qentry->s.status == QUEUE_STATUS_FREE) { > + queue_unlock(qentry); > + unlock_entry(pktio_entry); > + return -1; > + } > + > + qentry->s.enqueue = queue_enq_dummy; > + qentry->s.enqueue_multi = queue_enq_multi_dummy; > + qentry->s.status = QUEUE_STATUS_NOTSCHED; > + qentry->s.pktin = ODP_PKTIO_INVALID; > + queue_unlock(qentry); > + > + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; > + unlock_entry(pktio_entry); > + > + return 0; > } > > odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id) > diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c > index a14de4f..775b788 100644 > --- a/platform/linux-generic/odp_schedule.c > +++ b/platform/linux-generic/odp_schedule.c > @@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, odp_buffer_t out_buf[], > desc = odp_buffer_addr(desc_buf); > queue = desc->queue; > > + if (odp_queue_type(queue) == > + ODP_QUEUE_TYPE_PKTIN && > + !queue_is_sched(queue)) > + continue; > + > num = odp_queue_deq_multi(queue, > sched_local.buf, > max_deq); > diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c > index d1eb0d5..03e954a 100644 > --- a/test/validation/odp_pktio.c > +++ b/test/validation/odp_pktio.c > @@ -379,6 +379,7 @@ static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) > pktio_txrx_multi(&pktios[0], &pktios[if_b], num_pkts); > > for (i = 0; i < num_ifaces; ++i) { > + odp_pktio_inq_remdef(pktios[i].id); > ret = odp_pktio_close(pktios[i].id); > CU_ASSERT(ret == 0); > } > @@ -472,6 +473,21 @@ static void test_odp_pktio_mac(void) > return; > } > > +static void test_odp_pktio_inq_remdef(void) > +{ > + odp_pktio_t pktio = create_pktio(iface_name[0]); > + int i; > + > + CU_ASSERT(pktio != ODP_PKTIO_INVALID); > + CU_ASSERT(create_inq(pktio) == 0); > + CU_ASSERT(odp_pktio_inq_remdef(pktio) == 0); > + > + for (i = 0; i < 100; i++) > + odp_schedule(NULL, ODP_TIME_MSEC); > + > + CU_ASSERT(odp_pktio_close(pktio) == 0); > +} > + > static void test_odp_pktio_open(void) > { > odp_pktio_t pktio; > @@ -582,6 +598,7 @@ CU_TestInfo pktio_tests[] = { > {"pktio mtu", test_odp_pktio_mtu}, > {"pktio promisc mode", test_odp_pktio_promisc}, > {"pktio mac", test_odp_pktio_mac}, > + {"pktio inq_remdef", test_odp_pktio_inq_remdef}, > CU_TEST_INFO_NULL > }; >
Stuart, can you set review-by for v8? Maxim. On 01/19/2015 01:01 PM, Maxim Uvarov wrote: > On 01/16/2015 03:58 PM, Maxim Uvarov wrote: >> Correctly remove queue from packet i/o and remove it from scheduler. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> v8: fixed Stuart comments and added test. Implementation of >> odp_pktio_inq_remdef >> also fixes segfault in existance pktio test. > Looks like I thought about something else. I should say: > > because of odp_pktio_inq_remdef was not implemented before, ODP can > have special conditions where odp_schedule() for closed > pktio can pull from old incoming queue. In linux-generic > implementation packet recv() will be called and implementation will > try to > place packet to destroyed queue. So there is segfault. Because current > odp examples do not destroy/create packet i/o and then call > scheduler we did not see that segfault. So in this patch I implemented > odp_pktio_inq_rmdef and added tests case for odp_schedule > on closed packet i.o. > > Maxim. >> .../linux-generic/include/odp_queue_internal.h | 10 ++++++++ >> platform/linux-generic/odp_packet_io.c | 29 >> +++++++++++++++++++++- >> platform/linux-generic/odp_schedule.c | 5 ++++ >> test/validation/odp_pktio.c | 17 +++++++++++++ >> 4 files changed, 60 insertions(+), 1 deletion(-) >> >> diff --git a/platform/linux-generic/include/odp_queue_internal.h >> b/platform/linux-generic/include/odp_queue_internal.h >> index d5c8e4e..dbc42c0 100644 >> --- a/platform/linux-generic/include/odp_queue_internal.h >> +++ b/platform/linux-generic/include/odp_queue_internal.h >> @@ -129,6 +129,16 @@ static inline int queue_is_destroyed(odp_queue_t >> handle) >> return queue->s.status == QUEUE_STATUS_DESTROYED; >> } >> + >> +static inline int queue_is_sched(odp_queue_t handle) >> +{ >> + queue_entry_t *queue; >> + >> + queue = queue_to_qentry(handle); >> + >> + return ((queue->s.status == QUEUE_STATUS_SCHED) && >> + (queue->s.pktin != ODP_PKTIO_INVALID)); >> +} >> #ifdef __cplusplus >> } >> #endif >> diff --git a/platform/linux-generic/odp_packet_io.c >> b/platform/linux-generic/odp_packet_io.c >> index cd109d2..04de756 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -429,7 +429,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, >> odp_queue_t queue) >> int odp_pktio_inq_remdef(odp_pktio_t id) >> { >> - return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID); >> + pktio_entry_t *pktio_entry = get_pktio_entry(id); >> + odp_queue_t queue; >> + queue_entry_t *qentry; >> + >> + if (pktio_entry == NULL) >> + return -1; >> + >> + lock_entry(pktio_entry); >> + queue = pktio_entry->s.inq_default; >> + qentry = queue_to_qentry(queue); >> + >> + queue_lock(qentry); >> + if (qentry->s.status == QUEUE_STATUS_FREE) { >> + queue_unlock(qentry); >> + unlock_entry(pktio_entry); >> + return -1; >> + } >> + >> + qentry->s.enqueue = queue_enq_dummy; >> + qentry->s.enqueue_multi = queue_enq_multi_dummy; >> + qentry->s.status = QUEUE_STATUS_NOTSCHED; >> + qentry->s.pktin = ODP_PKTIO_INVALID; >> + queue_unlock(qentry); >> + >> + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; >> + unlock_entry(pktio_entry); >> + >> + return 0; >> } >> odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id) >> diff --git a/platform/linux-generic/odp_schedule.c >> b/platform/linux-generic/odp_schedule.c >> index a14de4f..775b788 100644 >> --- a/platform/linux-generic/odp_schedule.c >> +++ b/platform/linux-generic/odp_schedule.c >> @@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, >> odp_buffer_t out_buf[], >> desc = odp_buffer_addr(desc_buf); >> queue = desc->queue; >> + if (odp_queue_type(queue) == >> + ODP_QUEUE_TYPE_PKTIN && >> + !queue_is_sched(queue)) >> + continue; >> + >> num = odp_queue_deq_multi(queue, >> sched_local.buf, >> max_deq); >> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c >> index d1eb0d5..03e954a 100644 >> --- a/test/validation/odp_pktio.c >> +++ b/test/validation/odp_pktio.c >> @@ -379,6 +379,7 @@ static void pktio_test_txrx(odp_queue_type_t >> q_type, int num_pkts) >> pktio_txrx_multi(&pktios[0], &pktios[if_b], num_pkts); >> for (i = 0; i < num_ifaces; ++i) { >> + odp_pktio_inq_remdef(pktios[i].id); >> ret = odp_pktio_close(pktios[i].id); >> CU_ASSERT(ret == 0); >> } >> @@ -472,6 +473,21 @@ static void test_odp_pktio_mac(void) >> return; >> } >> +static void test_odp_pktio_inq_remdef(void) >> +{ >> + odp_pktio_t pktio = create_pktio(iface_name[0]); >> + int i; >> + >> + CU_ASSERT(pktio != ODP_PKTIO_INVALID); >> + CU_ASSERT(create_inq(pktio) == 0); >> + CU_ASSERT(odp_pktio_inq_remdef(pktio) == 0); >> + >> + for (i = 0; i < 100; i++) >> + odp_schedule(NULL, ODP_TIME_MSEC); >> + >> + CU_ASSERT(odp_pktio_close(pktio) == 0); >> +} >> + >> static void test_odp_pktio_open(void) >> { >> odp_pktio_t pktio; >> @@ -582,6 +598,7 @@ CU_TestInfo pktio_tests[] = { >> {"pktio mtu", test_odp_pktio_mtu}, >> {"pktio promisc mode", test_odp_pktio_promisc}, >> {"pktio mac", test_odp_pktio_mac}, >> + {"pktio inq_remdef", test_odp_pktio_inq_remdef}, >> CU_TEST_INFO_NULL >> }; >
Ping! Maxim. On 01/20/2015 08:12 PM, Maxim Uvarov wrote: > Stuart, can you set review-by for v8? > > Maxim. > > On 01/19/2015 01:01 PM, Maxim Uvarov wrote: >> On 01/16/2015 03:58 PM, Maxim Uvarov wrote: >>> Correctly remove queue from packet i/o and remove it from scheduler. >>> >>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>> --- >>> v8: fixed Stuart comments and added test. Implementation of >>> odp_pktio_inq_remdef >>> also fixes segfault in existance pktio test. >> Looks like I thought about something else. I should say: >> >> because of odp_pktio_inq_remdef was not implemented before, ODP can >> have special conditions where odp_schedule() for closed >> pktio can pull from old incoming queue. In linux-generic >> implementation packet recv() will be called and implementation will >> try to >> place packet to destroyed queue. So there is segfault. Because >> current odp examples do not destroy/create packet i/o and then call >> scheduler we did not see that segfault. So in this patch I >> implemented odp_pktio_inq_rmdef and added tests case for odp_schedule >> on closed packet i.o. >> >> Maxim. >>> .../linux-generic/include/odp_queue_internal.h | 10 ++++++++ >>> platform/linux-generic/odp_packet_io.c | 29 >>> +++++++++++++++++++++- >>> platform/linux-generic/odp_schedule.c | 5 ++++ >>> test/validation/odp_pktio.c | 17 +++++++++++++ >>> 4 files changed, 60 insertions(+), 1 deletion(-) >>> >>> diff --git a/platform/linux-generic/include/odp_queue_internal.h >>> b/platform/linux-generic/include/odp_queue_internal.h >>> index d5c8e4e..dbc42c0 100644 >>> --- a/platform/linux-generic/include/odp_queue_internal.h >>> +++ b/platform/linux-generic/include/odp_queue_internal.h >>> @@ -129,6 +129,16 @@ static inline int >>> queue_is_destroyed(odp_queue_t handle) >>> return queue->s.status == QUEUE_STATUS_DESTROYED; >>> } >>> + >>> +static inline int queue_is_sched(odp_queue_t handle) >>> +{ >>> + queue_entry_t *queue; >>> + >>> + queue = queue_to_qentry(handle); >>> + >>> + return ((queue->s.status == QUEUE_STATUS_SCHED) && >>> + (queue->s.pktin != ODP_PKTIO_INVALID)); >>> +} >>> #ifdef __cplusplus >>> } >>> #endif >>> diff --git a/platform/linux-generic/odp_packet_io.c >>> b/platform/linux-generic/odp_packet_io.c >>> index cd109d2..04de756 100644 >>> --- a/platform/linux-generic/odp_packet_io.c >>> +++ b/platform/linux-generic/odp_packet_io.c >>> @@ -429,7 +429,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, >>> odp_queue_t queue) >>> int odp_pktio_inq_remdef(odp_pktio_t id) >>> { >>> - return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID); >>> + pktio_entry_t *pktio_entry = get_pktio_entry(id); >>> + odp_queue_t queue; >>> + queue_entry_t *qentry; >>> + >>> + if (pktio_entry == NULL) >>> + return -1; >>> + >>> + lock_entry(pktio_entry); >>> + queue = pktio_entry->s.inq_default; >>> + qentry = queue_to_qentry(queue); >>> + >>> + queue_lock(qentry); >>> + if (qentry->s.status == QUEUE_STATUS_FREE) { >>> + queue_unlock(qentry); >>> + unlock_entry(pktio_entry); >>> + return -1; >>> + } >>> + >>> + qentry->s.enqueue = queue_enq_dummy; >>> + qentry->s.enqueue_multi = queue_enq_multi_dummy; >>> + qentry->s.status = QUEUE_STATUS_NOTSCHED; >>> + qentry->s.pktin = ODP_PKTIO_INVALID; >>> + queue_unlock(qentry); >>> + >>> + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; >>> + unlock_entry(pktio_entry); >>> + >>> + return 0; >>> } >>> odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id) >>> diff --git a/platform/linux-generic/odp_schedule.c >>> b/platform/linux-generic/odp_schedule.c >>> index a14de4f..775b788 100644 >>> --- a/platform/linux-generic/odp_schedule.c >>> +++ b/platform/linux-generic/odp_schedule.c >>> @@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, >>> odp_buffer_t out_buf[], >>> desc = odp_buffer_addr(desc_buf); >>> queue = desc->queue; >>> + if (odp_queue_type(queue) == >>> + ODP_QUEUE_TYPE_PKTIN && >>> + !queue_is_sched(queue)) >>> + continue; >>> + >>> num = odp_queue_deq_multi(queue, >>> sched_local.buf, >>> max_deq); >>> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c >>> index d1eb0d5..03e954a 100644 >>> --- a/test/validation/odp_pktio.c >>> +++ b/test/validation/odp_pktio.c >>> @@ -379,6 +379,7 @@ static void pktio_test_txrx(odp_queue_type_t >>> q_type, int num_pkts) >>> pktio_txrx_multi(&pktios[0], &pktios[if_b], num_pkts); >>> for (i = 0; i < num_ifaces; ++i) { >>> + odp_pktio_inq_remdef(pktios[i].id); >>> ret = odp_pktio_close(pktios[i].id); >>> CU_ASSERT(ret == 0); >>> } >>> @@ -472,6 +473,21 @@ static void test_odp_pktio_mac(void) >>> return; >>> } >>> +static void test_odp_pktio_inq_remdef(void) >>> +{ >>> + odp_pktio_t pktio = create_pktio(iface_name[0]); >>> + int i; >>> + >>> + CU_ASSERT(pktio != ODP_PKTIO_INVALID); >>> + CU_ASSERT(create_inq(pktio) == 0); >>> + CU_ASSERT(odp_pktio_inq_remdef(pktio) == 0); >>> + >>> + for (i = 0; i < 100; i++) >>> + odp_schedule(NULL, ODP_TIME_MSEC); >>> + >>> + CU_ASSERT(odp_pktio_close(pktio) == 0); >>> +} >>> + >>> static void test_odp_pktio_open(void) >>> { >>> odp_pktio_t pktio; >>> @@ -582,6 +598,7 @@ CU_TestInfo pktio_tests[] = { >>> {"pktio mtu", test_odp_pktio_mtu}, >>> {"pktio promisc mode", test_odp_pktio_promisc}, >>> {"pktio mac", test_odp_pktio_mac}, >>> + {"pktio inq_remdef", test_odp_pktio_inq_remdef}, >>> CU_TEST_INFO_NULL >>> }; >> >
On 16 January 2015 at 13:58, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Correctly remove queue from packet i/o and remove it from scheduler. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> Tested-by: Anders Roxell <anders.roxell@linaro.org> > --- > v8: fixed Stuart comments and added test. Implementation of odp_pktio_inq_remdef > also fixes segfault in existance pktio test. > > .../linux-generic/include/odp_queue_internal.h | 10 ++++++++ > platform/linux-generic/odp_packet_io.c | 29 +++++++++++++++++++++- > platform/linux-generic/odp_schedule.c | 5 ++++ > test/validation/odp_pktio.c | 17 +++++++++++++ > 4 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h > index d5c8e4e..dbc42c0 100644 > --- a/platform/linux-generic/include/odp_queue_internal.h > +++ b/platform/linux-generic/include/odp_queue_internal.h > @@ -129,6 +129,16 @@ static inline int queue_is_destroyed(odp_queue_t handle) > > return queue->s.status == QUEUE_STATUS_DESTROYED; > } > + > +static inline int queue_is_sched(odp_queue_t handle) > +{ > + queue_entry_t *queue; > + > + queue = queue_to_qentry(handle); > + > + return ((queue->s.status == QUEUE_STATUS_SCHED) && > + (queue->s.pktin != ODP_PKTIO_INVALID)); > +} > #ifdef __cplusplus > } > #endif > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index cd109d2..04de756 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -429,7 +429,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue) > > int odp_pktio_inq_remdef(odp_pktio_t id) > { > - return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID); > + pktio_entry_t *pktio_entry = get_pktio_entry(id); > + odp_queue_t queue; > + queue_entry_t *qentry; > + > + if (pktio_entry == NULL) > + return -1; > + > + lock_entry(pktio_entry); > + queue = pktio_entry->s.inq_default; > + qentry = queue_to_qentry(queue); > + > + queue_lock(qentry); > + if (qentry->s.status == QUEUE_STATUS_FREE) { > + queue_unlock(qentry); > + unlock_entry(pktio_entry); > + return -1; > + } > + > + qentry->s.enqueue = queue_enq_dummy; > + qentry->s.enqueue_multi = queue_enq_multi_dummy; > + qentry->s.status = QUEUE_STATUS_NOTSCHED; > + qentry->s.pktin = ODP_PKTIO_INVALID; > + queue_unlock(qentry); > + > + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; > + unlock_entry(pktio_entry); > + > + return 0; > } > > odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id) > diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c > index a14de4f..775b788 100644 > --- a/platform/linux-generic/odp_schedule.c > +++ b/platform/linux-generic/odp_schedule.c > @@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, odp_buffer_t out_buf[], > desc = odp_buffer_addr(desc_buf); > queue = desc->queue; > > + if (odp_queue_type(queue) == > + ODP_QUEUE_TYPE_PKTIN && > + !queue_is_sched(queue)) > + continue; > + > num = odp_queue_deq_multi(queue, > sched_local.buf, > max_deq); > diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c > index d1eb0d5..03e954a 100644 > --- a/test/validation/odp_pktio.c > +++ b/test/validation/odp_pktio.c > @@ -379,6 +379,7 @@ static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) > pktio_txrx_multi(&pktios[0], &pktios[if_b], num_pkts); > > for (i = 0; i < num_ifaces; ++i) { > + odp_pktio_inq_remdef(pktios[i].id); > ret = odp_pktio_close(pktios[i].id); > CU_ASSERT(ret == 0); > } > @@ -472,6 +473,21 @@ static void test_odp_pktio_mac(void) > return; > } > > +static void test_odp_pktio_inq_remdef(void) > +{ > + odp_pktio_t pktio = create_pktio(iface_name[0]); > + int i; > + > + CU_ASSERT(pktio != ODP_PKTIO_INVALID); > + CU_ASSERT(create_inq(pktio) == 0); > + CU_ASSERT(odp_pktio_inq_remdef(pktio) == 0); > + > + for (i = 0; i < 100; i++) > + odp_schedule(NULL, ODP_TIME_MSEC); > + > + CU_ASSERT(odp_pktio_close(pktio) == 0); > +} > + > static void test_odp_pktio_open(void) > { > odp_pktio_t pktio; > @@ -582,6 +598,7 @@ CU_TestInfo pktio_tests[] = { > {"pktio mtu", test_odp_pktio_mtu}, > {"pktio promisc mode", test_odp_pktio_promisc}, > {"pktio mac", test_odp_pktio_mac}, > + {"pktio inq_remdef", test_odp_pktio_inq_remdef}, > CU_TEST_INFO_NULL > }; > > -- > 1.8.5.1.163.gd7aced9 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
This has been on the list for seven days, I think Stewart is out and so can't confirm he has the fix he wanted, but the API has a review-by and it has tested-by I think we should merge this. On 23 January 2015 at 12:39, Anders Roxell <anders.roxell@linaro.org> wrote: > On 16 January 2015 at 13:58, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > Correctly remove queue from packet i/o and remove it from scheduler. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > Tested-by: Anders Roxell <anders.roxell@linaro.org> > > > --- > > v8: fixed Stuart comments and added test. Implementation of > odp_pktio_inq_remdef > > also fixes segfault in existance pktio test. > > > > .../linux-generic/include/odp_queue_internal.h | 10 ++++++++ > > platform/linux-generic/odp_packet_io.c | 29 > +++++++++++++++++++++- > > platform/linux-generic/odp_schedule.c | 5 ++++ > > test/validation/odp_pktio.c | 17 +++++++++++++ > > 4 files changed, 60 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/odp_queue_internal.h > b/platform/linux-generic/include/odp_queue_internal.h > > index d5c8e4e..dbc42c0 100644 > > --- a/platform/linux-generic/include/odp_queue_internal.h > > +++ b/platform/linux-generic/include/odp_queue_internal.h > > @@ -129,6 +129,16 @@ static inline int queue_is_destroyed(odp_queue_t > handle) > > > > return queue->s.status == QUEUE_STATUS_DESTROYED; > > } > > + > > +static inline int queue_is_sched(odp_queue_t handle) > > +{ > > + queue_entry_t *queue; > > + > > + queue = queue_to_qentry(handle); > > + > > + return ((queue->s.status == QUEUE_STATUS_SCHED) && > > + (queue->s.pktin != ODP_PKTIO_INVALID)); > > +} > > #ifdef __cplusplus > > } > > #endif > > diff --git a/platform/linux-generic/odp_packet_io.c > b/platform/linux-generic/odp_packet_io.c > > index cd109d2..04de756 100644 > > --- a/platform/linux-generic/odp_packet_io.c > > +++ b/platform/linux-generic/odp_packet_io.c > > @@ -429,7 +429,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, > odp_queue_t queue) > > > > int odp_pktio_inq_remdef(odp_pktio_t id) > > { > > - return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID); > > + pktio_entry_t *pktio_entry = get_pktio_entry(id); > > + odp_queue_t queue; > > + queue_entry_t *qentry; > > + > > + if (pktio_entry == NULL) > > + return -1; > > + > > + lock_entry(pktio_entry); > > + queue = pktio_entry->s.inq_default; > > + qentry = queue_to_qentry(queue); > > + > > + queue_lock(qentry); > > + if (qentry->s.status == QUEUE_STATUS_FREE) { > > + queue_unlock(qentry); > > + unlock_entry(pktio_entry); > > + return -1; > > + } > > + > > + qentry->s.enqueue = queue_enq_dummy; > > + qentry->s.enqueue_multi = queue_enq_multi_dummy; > > + qentry->s.status = QUEUE_STATUS_NOTSCHED; > > + qentry->s.pktin = ODP_PKTIO_INVALID; > > + queue_unlock(qentry); > > + > > + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; > > + unlock_entry(pktio_entry); > > + > > + return 0; > > } > > > > odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id) > > diff --git a/platform/linux-generic/odp_schedule.c > b/platform/linux-generic/odp_schedule.c > > index a14de4f..775b788 100644 > > --- a/platform/linux-generic/odp_schedule.c > > +++ b/platform/linux-generic/odp_schedule.c > > @@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, > odp_buffer_t out_buf[], > > desc = odp_buffer_addr(desc_buf); > > queue = desc->queue; > > > > + if (odp_queue_type(queue) == > > + ODP_QUEUE_TYPE_PKTIN && > > + !queue_is_sched(queue)) > > + continue; > > + > > num = odp_queue_deq_multi(queue, > > > sched_local.buf, > > max_deq); > > diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c > > index d1eb0d5..03e954a 100644 > > --- a/test/validation/odp_pktio.c > > +++ b/test/validation/odp_pktio.c > > @@ -379,6 +379,7 @@ static void pktio_test_txrx(odp_queue_type_t q_type, > int num_pkts) > > pktio_txrx_multi(&pktios[0], &pktios[if_b], num_pkts); > > > > for (i = 0; i < num_ifaces; ++i) { > > + odp_pktio_inq_remdef(pktios[i].id); > > ret = odp_pktio_close(pktios[i].id); > > CU_ASSERT(ret == 0); > > } > > @@ -472,6 +473,21 @@ static void test_odp_pktio_mac(void) > > return; > > } > > > > +static void test_odp_pktio_inq_remdef(void) > > +{ > > + odp_pktio_t pktio = create_pktio(iface_name[0]); > > + int i; > > + > > + CU_ASSERT(pktio != ODP_PKTIO_INVALID); > > + CU_ASSERT(create_inq(pktio) == 0); > > + CU_ASSERT(odp_pktio_inq_remdef(pktio) == 0); > > + > > + for (i = 0; i < 100; i++) > > + odp_schedule(NULL, ODP_TIME_MSEC); > > + > > + CU_ASSERT(odp_pktio_close(pktio) == 0); > > +} > > + > > static void test_odp_pktio_open(void) > > { > > odp_pktio_t pktio; > > @@ -582,6 +598,7 @@ CU_TestInfo pktio_tests[] = { > > {"pktio mtu", test_odp_pktio_mtu}, > > {"pktio promisc mode", test_odp_pktio_promisc}, > > {"pktio mac", test_odp_pktio_mac}, > > + {"pktio inq_remdef", test_odp_pktio_inq_remdef}, > > CU_TEST_INFO_NULL > > }; > > > > -- > > 1.8.5.1.163.gd7aced9 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h index d5c8e4e..dbc42c0 100644 --- a/platform/linux-generic/include/odp_queue_internal.h +++ b/platform/linux-generic/include/odp_queue_internal.h @@ -129,6 +129,16 @@ static inline int queue_is_destroyed(odp_queue_t handle) return queue->s.status == QUEUE_STATUS_DESTROYED; } + +static inline int queue_is_sched(odp_queue_t handle) +{ + queue_entry_t *queue; + + queue = queue_to_qentry(handle); + + return ((queue->s.status == QUEUE_STATUS_SCHED) && + (queue->s.pktin != ODP_PKTIO_INVALID)); +} #ifdef __cplusplus } #endif diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index cd109d2..04de756 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -429,7 +429,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue) int odp_pktio_inq_remdef(odp_pktio_t id) { - return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID); + pktio_entry_t *pktio_entry = get_pktio_entry(id); + odp_queue_t queue; + queue_entry_t *qentry; + + if (pktio_entry == NULL) + return -1; + + lock_entry(pktio_entry); + queue = pktio_entry->s.inq_default; + qentry = queue_to_qentry(queue); + + queue_lock(qentry); + if (qentry->s.status == QUEUE_STATUS_FREE) { + queue_unlock(qentry); + unlock_entry(pktio_entry); + return -1; + } + + qentry->s.enqueue = queue_enq_dummy; + qentry->s.enqueue_multi = queue_enq_multi_dummy; + qentry->s.status = QUEUE_STATUS_NOTSCHED; + qentry->s.pktin = ODP_PKTIO_INVALID; + queue_unlock(qentry); + + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; + unlock_entry(pktio_entry); + + return 0; } odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id) diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c index a14de4f..775b788 100644 --- a/platform/linux-generic/odp_schedule.c +++ b/platform/linux-generic/odp_schedule.c @@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, odp_buffer_t out_buf[], desc = odp_buffer_addr(desc_buf); queue = desc->queue; + if (odp_queue_type(queue) == + ODP_QUEUE_TYPE_PKTIN && + !queue_is_sched(queue)) + continue; + num = odp_queue_deq_multi(queue, sched_local.buf, max_deq); diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c index d1eb0d5..03e954a 100644 --- a/test/validation/odp_pktio.c +++ b/test/validation/odp_pktio.c @@ -379,6 +379,7 @@ static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) pktio_txrx_multi(&pktios[0], &pktios[if_b], num_pkts); for (i = 0; i < num_ifaces; ++i) { + odp_pktio_inq_remdef(pktios[i].id); ret = odp_pktio_close(pktios[i].id); CU_ASSERT(ret == 0); } @@ -472,6 +473,21 @@ static void test_odp_pktio_mac(void) return; } +static void test_odp_pktio_inq_remdef(void) +{ + odp_pktio_t pktio = create_pktio(iface_name[0]); + int i; + + CU_ASSERT(pktio != ODP_PKTIO_INVALID); + CU_ASSERT(create_inq(pktio) == 0); + CU_ASSERT(odp_pktio_inq_remdef(pktio) == 0); + + for (i = 0; i < 100; i++) + odp_schedule(NULL, ODP_TIME_MSEC); + + CU_ASSERT(odp_pktio_close(pktio) == 0); +} + static void test_odp_pktio_open(void) { odp_pktio_t pktio; @@ -582,6 +598,7 @@ CU_TestInfo pktio_tests[] = { {"pktio mtu", test_odp_pktio_mtu}, {"pktio promisc mode", test_odp_pktio_promisc}, {"pktio mac", test_odp_pktio_mac}, + {"pktio inq_remdef", test_odp_pktio_inq_remdef}, CU_TEST_INFO_NULL };
Correctly remove queue from packet i/o and remove it from scheduler. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- v8: fixed Stuart comments and added test. Implementation of odp_pktio_inq_remdef also fixes segfault in existance pktio test. .../linux-generic/include/odp_queue_internal.h | 10 ++++++++ platform/linux-generic/odp_packet_io.c | 29 +++++++++++++++++++++- platform/linux-generic/odp_schedule.c | 5 ++++ test/validation/odp_pktio.c | 17 +++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-)