| Message ID | 1418824616-19221-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 12/17/2014 05:45 PM, Stuart Haslam wrote: > On Wed, Dec 17, 2014 at 01:56:56PM +0000, Maxim Uvarov wrote: >> odp_pktio_inq_remdef() calls odp_pktio_inq_setdef() with >> ODP_QUEUE_INVALID. odp_pktio_inq_setdef should set up >> ODP_QUEUE_INVALID to pktio instead of returning with error. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> platform/linux-generic/odp_packet_io.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c >> index 3ca8100..4e54658 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -367,9 +367,16 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue) >> pktio_entry_t *pktio_entry = get_pktio_entry(id); >> queue_entry_t *qentry; >> >> - if (pktio_entry == NULL || queue == ODP_QUEUE_INVALID) >> + if (pktio_entry == NULL) >> return -1; >> >> + if (queue == ODP_QUEUE_INVALID) { >> + lock_entry(pktio_entry); >> + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; >> + unlock_entry(pktio_entry); > You could've avoided this duplicated chunk of code by just shuffling the > existing code around and adding an early return for an INVALID queue. > >> + return 0; >> + } >> + >> qentry = queue_to_qentry(queue); >> >> if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN) >> -- >> 1.8.5.1.163.gd7aced9 >> > My main concern though is that setdef and remdef are not symmetric. > setdef not only sets the queue as the default for the pktio (which is > only used when the application calls getdef) but also sets the pktin for > the queue and schedules it. With this change, after calling remdef the > previous default inq is still being scheduled, is that what the caller > expected and how do they now remove it from the scheduler if required? > In that case current odp_pktio_inq_remdef() is wrong. It has to: 1. take default queue: queue = pktio_entry->s.inq_default; 2. get entry: qentry = queue_to_qentry(queue); 3. Remove it from scheduler: qentry->s.status = QUEUE_STATUS_FREE or QUEUE_STATUS_DESTROYED (need to check code, unclear). 4. Return. Right? Maxim.
diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index 3ca8100..4e54658 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -367,9 +367,16 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue) pktio_entry_t *pktio_entry = get_pktio_entry(id); queue_entry_t *qentry; - if (pktio_entry == NULL || queue == ODP_QUEUE_INVALID) + if (pktio_entry == NULL) return -1; + if (queue == ODP_QUEUE_INVALID) { + lock_entry(pktio_entry); + pktio_entry->s.inq_default = ODP_QUEUE_INVALID; + unlock_entry(pktio_entry); + return 0; + } + qentry = queue_to_qentry(queue); if (qentry->s.type != ODP_QUEUE_TYPE_PKTIN)
odp_pktio_inq_remdef() calls odp_pktio_inq_setdef() with ODP_QUEUE_INVALID. odp_pktio_inq_setdef should set up ODP_QUEUE_INVALID to pktio instead of returning with error. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/odp_packet_io.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)