| Message ID | 1446739097-29843-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Accepted |
| Commit | d33dcd8d49a0f804748887d2ea5c71e0c76c99ca |
| Headers | show |
On 11/06/2015 11:45, Nicolas Morey-Chaisemartin wrote: > On a side note, what is the expected behavior when stopping a stopped interface or starting a started one? Current api does not say anything about it except first sentence. I think we should prevent double stop and double start and refine doc for it. /** * Stop packet receive and transmit * * Stop packet receive and transmit on a previously started interface. New * packets are not received from or transmitted to the network. Packets already * received from the network may be still available from interface and * application can receive those normally. New packets may not be accepted for * transmit. Packets already stored for transmit are not freed. A following * odp_packet_start() call restarts packet receive and transmit. * * @param pktio Packet IO handle * * @retval 0 on success * @retval <0 on failure * * @see odp_pktio_start(), odp_pktio_close() */ int odp_pktio_stop(odp_pktio_t pktio); Maxim. > On 11/05/2015 04:58 PM, Maxim Uvarov wrote: >> According to API odp_pktio_close() can be called only for stopped >> pktio. So in odp_pktio_term_global try to stop it first then >> call close. Also check all returns codes. >> https://bugs.linaro.org/show_bug.cgi?id=1854 >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> v2: lock pktio for stop and close in odp_pktio_term_global() (Nicolases note) >> >> platform/linux-generic/odp_packet_io.c | 119 ++++++++++++++++++++++----------- >> 1 file changed, 79 insertions(+), 40 deletions(-) >> >> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c >> index 1246bff..c3bc6a0 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -84,33 +84,6 @@ int odp_pktio_init_global(void) >> return 0; >> } >> >> -int odp_pktio_term_global(void) >> -{ >> - pktio_entry_t *pktio_entry; >> - int ret = 0; >> - int id; >> - int pktio_if; >> - >> - for (id = 1; id <= ODP_CONFIG_PKTIO_ENTRIES; ++id) { >> - pktio_entry = &pktio_tbl->entries[id - 1]; >> - odp_pktio_close(pktio_entry->s.handle); >> - odp_queue_destroy(pktio_entry->s.outq_default); >> - } >> - >> - for (pktio_if = 0; pktio_if_ops[pktio_if]; ++pktio_if) { >> - if (pktio_if_ops[pktio_if]->term) >> - if (pktio_if_ops[pktio_if]->term()) >> - ODP_ERR("failed to terminate pktio type %d", >> - pktio_if); >> - } >> - >> - ret = odp_shm_free(odp_shm_lookup("odp_pktio_entries")); >> - if (ret < 0) >> - ODP_ERR("shm free failed for odp_pktio_entries"); >> - >> - return ret; >> -} >> - >> int odp_pktio_init_local(void) >> { >> return 0; >> @@ -284,10 +257,22 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_pool_t pool, >> return id; >> } >> >> +static int _pktio_close(pktio_entry_t *entry) >> +{ >> + int ret; >> + >> + ret = entry->s.ops->close(entry); >> + if (ret) >> + return -1; >> + >> + set_free(entry); >> + return 0; >> +} >> + >> int odp_pktio_close(odp_pktio_t id) >> { >> pktio_entry_t *entry; >> - int res = -1; >> + int res; >> >> entry = get_pktio_entry(id); >> if (entry == NULL) >> @@ -295,14 +280,12 @@ int odp_pktio_close(odp_pktio_t id) >> >> lock_entry(entry); >> if (!is_free(entry)) { >> - res = entry->s.ops->close(entry); >> - res |= free_pktio_entry(id); >> + res = _pktio_close(entry); >> + if (res) >> + ODP_ABORT("unable to close pktio\n"); >> } >> unlock_entry(entry); >> >> - if (res != 0) >> - return -1; >> - >> return 0; >> } >> >> @@ -325,20 +308,29 @@ int odp_pktio_start(odp_pktio_t id) >> return res; >> } >> >> -int odp_pktio_stop(odp_pktio_t id) >> +static int _pktio_stop(pktio_entry_t *entry) >> { >> - pktio_entry_t *entry; >> int res = 0; >> >> - entry = get_pktio_entry(id); >> - if (!entry) >> - return -1; >> - >> - lock_entry(entry); >> if (entry->s.ops->stop) >> res = entry->s.ops->stop(entry); >> if (!res) >> entry->s.state = STATE_STOP; >> + >> + return res; >> +} >> + >> +int odp_pktio_stop(odp_pktio_t id) >> +{ >> + pktio_entry_t *entry; >> + int res; >> + >> + entry = get_pktio_entry(id); >> + if (!entry) >> + return -1; >> + >> + lock_entry(entry); >> + res = _pktio_stop(entry); >> unlock_entry(entry); >> >> return res; >> @@ -822,3 +814,50 @@ void odp_pktio_param_init(odp_pktio_param_t *params) >> { >> memset(params, 0, sizeof(odp_pktio_param_t)); >> } >> + >> +int odp_pktio_term_global(void) >> +{ >> + int ret; >> + int id; >> + int pktio_if; >> + >> + for (id = 0; id < ODP_CONFIG_PKTIO_ENTRIES; ++id) { >> + pktio_entry_t *pktio_entry; >> + >> + pktio_entry = &pktio_tbl->entries[id]; >> + >> + ret = odp_queue_destroy(pktio_entry->s.outq_default); >> + if (ret) >> + ODP_ABORT("unable to destroy outq %s\n", >> + pktio_entry->s.name); >> + >> + if (is_free(pktio_entry)) >> + continue; >> + >> + lock_entry(pktio_entry); >> + if (pktio_entry->s.state != STATE_STOP) { >> + ret = _pktio_stop(pktio_entry); >> + if (ret) >> + ODP_ABORT("unable to stop pktio %s\n", >> + pktio_entry->s.name); >> + } >> + ret = _pktio_close(pktio_entry); >> + if (ret) >> + ODP_ABORT("unable to close pktio %s\n", >> + pktio_entry->s.name); >> + unlock_entry(pktio_entry); >> + } >> + >> + for (pktio_if = 0; pktio_if_ops[pktio_if]; ++pktio_if) { >> + if (pktio_if_ops[pktio_if]->term) >> + if (pktio_if_ops[pktio_if]->term()) >> + ODP_ABORT("failed to terminate pktio type %d", >> + pktio_if); >> + } >> + >> + ret = odp_shm_free(odp_shm_lookup("odp_pktio_entries")); >> + if (ret != 0) >> + ODP_ERR("shm free failed for odp_pktio_entries"); >> + >> + return ret; >> +}
Merged, Maxim. On 11/06/2015 11:44, Nicolas Morey-Chaisemartin wrote: > Reviewed-by: Nicolas Morey-Chaisemartin <nmorey@kalray.eu> > > On 11/05/2015 04:58 PM, Maxim Uvarov wrote: >> According to API odp_pktio_close() can be called only for stopped >> pktio. So in odp_pktio_term_global try to stop it first then >> call close. Also check all returns codes. >> https://bugs.linaro.org/show_bug.cgi?id=1854 >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> v2: lock pktio for stop and close in odp_pktio_term_global() (Nicolases note) >> >> platform/linux-generic/odp_packet_io.c | 119 ++++++++++++++++++++++----------- >> 1 file changed, 79 insertions(+), 40 deletions(-) >> >> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c >> index 1246bff..c3bc6a0 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -84,33 +84,6 @@ int odp_pktio_init_global(void) >> return 0; >> } >> >> -int odp_pktio_term_global(void) >> -{ >> - pktio_entry_t *pktio_entry; >> - int ret = 0; >> - int id; >> - int pktio_if; >> - >> - for (id = 1; id <= ODP_CONFIG_PKTIO_ENTRIES; ++id) { >> - pktio_entry = &pktio_tbl->entries[id - 1]; >> - odp_pktio_close(pktio_entry->s.handle); >> - odp_queue_destroy(pktio_entry->s.outq_default); >> - } >> - >> - for (pktio_if = 0; pktio_if_ops[pktio_if]; ++pktio_if) { >> - if (pktio_if_ops[pktio_if]->term) >> - if (pktio_if_ops[pktio_if]->term()) >> - ODP_ERR("failed to terminate pktio type %d", >> - pktio_if); >> - } >> - >> - ret = odp_shm_free(odp_shm_lookup("odp_pktio_entries")); >> - if (ret < 0) >> - ODP_ERR("shm free failed for odp_pktio_entries"); >> - >> - return ret; >> -} >> - >> int odp_pktio_init_local(void) >> { >> return 0; >> @@ -284,10 +257,22 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_pool_t pool, >> return id; >> } >> >> +static int _pktio_close(pktio_entry_t *entry) >> +{ >> + int ret; >> + >> + ret = entry->s.ops->close(entry); >> + if (ret) >> + return -1; >> + >> + set_free(entry); >> + return 0; >> +} >> + >> int odp_pktio_close(odp_pktio_t id) >> { >> pktio_entry_t *entry; >> - int res = -1; >> + int res; >> >> entry = get_pktio_entry(id); >> if (entry == NULL) >> @@ -295,14 +280,12 @@ int odp_pktio_close(odp_pktio_t id) >> >> lock_entry(entry); >> if (!is_free(entry)) { >> - res = entry->s.ops->close(entry); >> - res |= free_pktio_entry(id); >> + res = _pktio_close(entry); >> + if (res) >> + ODP_ABORT("unable to close pktio\n"); >> } >> unlock_entry(entry); >> >> - if (res != 0) >> - return -1; >> - >> return 0; >> } >> >> @@ -325,20 +308,29 @@ int odp_pktio_start(odp_pktio_t id) >> return res; >> } >> >> -int odp_pktio_stop(odp_pktio_t id) >> +static int _pktio_stop(pktio_entry_t *entry) >> { >> - pktio_entry_t *entry; >> int res = 0; >> >> - entry = get_pktio_entry(id); >> - if (!entry) >> - return -1; >> - >> - lock_entry(entry); >> if (entry->s.ops->stop) >> res = entry->s.ops->stop(entry); >> if (!res) >> entry->s.state = STATE_STOP; >> + >> + return res; >> +} >> + >> +int odp_pktio_stop(odp_pktio_t id) >> +{ >> + pktio_entry_t *entry; >> + int res; >> + >> + entry = get_pktio_entry(id); >> + if (!entry) >> + return -1; >> + >> + lock_entry(entry); >> + res = _pktio_stop(entry); >> unlock_entry(entry); >> >> return res; >> @@ -822,3 +814,50 @@ void odp_pktio_param_init(odp_pktio_param_t *params) >> { >> memset(params, 0, sizeof(odp_pktio_param_t)); >> } >> + >> +int odp_pktio_term_global(void) >> +{ >> + int ret; >> + int id; >> + int pktio_if; >> + >> + for (id = 0; id < ODP_CONFIG_PKTIO_ENTRIES; ++id) { >> + pktio_entry_t *pktio_entry; >> + >> + pktio_entry = &pktio_tbl->entries[id]; >> + >> + ret = odp_queue_destroy(pktio_entry->s.outq_default); >> + if (ret) >> + ODP_ABORT("unable to destroy outq %s\n", >> + pktio_entry->s.name); >> + >> + if (is_free(pktio_entry)) >> + continue; >> + >> + lock_entry(pktio_entry); >> + if (pktio_entry->s.state != STATE_STOP) { >> + ret = _pktio_stop(pktio_entry); >> + if (ret) >> + ODP_ABORT("unable to stop pktio %s\n", >> + pktio_entry->s.name); >> + } >> + ret = _pktio_close(pktio_entry); >> + if (ret) >> + ODP_ABORT("unable to close pktio %s\n", >> + pktio_entry->s.name); >> + unlock_entry(pktio_entry); >> + } >> + >> + for (pktio_if = 0; pktio_if_ops[pktio_if]; ++pktio_if) { >> + if (pktio_if_ops[pktio_if]->term) >> + if (pktio_if_ops[pktio_if]->term()) >> + ODP_ABORT("failed to terminate pktio type %d", >> + pktio_if); >> + } >> + >> + ret = odp_shm_free(odp_shm_lookup("odp_pktio_entries")); >> + if (ret != 0) >> + ODP_ERR("shm free failed for odp_pktio_entries"); >> + >> + return ret; >> +}
diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index 1246bff..c3bc6a0 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -84,33 +84,6 @@ int odp_pktio_init_global(void) return 0; } -int odp_pktio_term_global(void) -{ - pktio_entry_t *pktio_entry; - int ret = 0; - int id; - int pktio_if; - - for (id = 1; id <= ODP_CONFIG_PKTIO_ENTRIES; ++id) { - pktio_entry = &pktio_tbl->entries[id - 1]; - odp_pktio_close(pktio_entry->s.handle); - odp_queue_destroy(pktio_entry->s.outq_default); - } - - for (pktio_if = 0; pktio_if_ops[pktio_if]; ++pktio_if) { - if (pktio_if_ops[pktio_if]->term) - if (pktio_if_ops[pktio_if]->term()) - ODP_ERR("failed to terminate pktio type %d", - pktio_if); - } - - ret = odp_shm_free(odp_shm_lookup("odp_pktio_entries")); - if (ret < 0) - ODP_ERR("shm free failed for odp_pktio_entries"); - - return ret; -} - int odp_pktio_init_local(void) { return 0; @@ -284,10 +257,22 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_pool_t pool, return id; } +static int _pktio_close(pktio_entry_t *entry) +{ + int ret; + + ret = entry->s.ops->close(entry); + if (ret) + return -1; + + set_free(entry); + return 0; +} + int odp_pktio_close(odp_pktio_t id) { pktio_entry_t *entry; - int res = -1; + int res; entry = get_pktio_entry(id); if (entry == NULL) @@ -295,14 +280,12 @@ int odp_pktio_close(odp_pktio_t id) lock_entry(entry); if (!is_free(entry)) { - res = entry->s.ops->close(entry); - res |= free_pktio_entry(id); + res = _pktio_close(entry); + if (res) + ODP_ABORT("unable to close pktio\n"); } unlock_entry(entry); - if (res != 0) - return -1; - return 0; } @@ -325,20 +308,29 @@ int odp_pktio_start(odp_pktio_t id) return res; } -int odp_pktio_stop(odp_pktio_t id) +static int _pktio_stop(pktio_entry_t *entry) { - pktio_entry_t *entry; int res = 0; - entry = get_pktio_entry(id); - if (!entry) - return -1; - - lock_entry(entry); if (entry->s.ops->stop) res = entry->s.ops->stop(entry); if (!res) entry->s.state = STATE_STOP; + + return res; +} + +int odp_pktio_stop(odp_pktio_t id) +{ + pktio_entry_t *entry; + int res; + + entry = get_pktio_entry(id); + if (!entry) + return -1; + + lock_entry(entry); + res = _pktio_stop(entry); unlock_entry(entry); return res; @@ -822,3 +814,50 @@ void odp_pktio_param_init(odp_pktio_param_t *params) { memset(params, 0, sizeof(odp_pktio_param_t)); } + +int odp_pktio_term_global(void) +{ + int ret; + int id; + int pktio_if; + + for (id = 0; id < ODP_CONFIG_PKTIO_ENTRIES; ++id) { + pktio_entry_t *pktio_entry; + + pktio_entry = &pktio_tbl->entries[id]; + + ret = odp_queue_destroy(pktio_entry->s.outq_default); + if (ret) + ODP_ABORT("unable to destroy outq %s\n", + pktio_entry->s.name); + + if (is_free(pktio_entry)) + continue; + + lock_entry(pktio_entry); + if (pktio_entry->s.state != STATE_STOP) { + ret = _pktio_stop(pktio_entry); + if (ret) + ODP_ABORT("unable to stop pktio %s\n", + pktio_entry->s.name); + } + ret = _pktio_close(pktio_entry); + if (ret) + ODP_ABORT("unable to close pktio %s\n", + pktio_entry->s.name); + unlock_entry(pktio_entry); + } + + for (pktio_if = 0; pktio_if_ops[pktio_if]; ++pktio_if) { + if (pktio_if_ops[pktio_if]->term) + if (pktio_if_ops[pktio_if]->term()) + ODP_ABORT("failed to terminate pktio type %d", + pktio_if); + } + + ret = odp_shm_free(odp_shm_lookup("odp_pktio_entries")); + if (ret != 0) + ODP_ERR("shm free failed for odp_pktio_entries"); + + return ret; +}
According to API odp_pktio_close() can be called only for stopped pktio. So in odp_pktio_term_global try to stop it first then call close. Also check all returns codes. https://bugs.linaro.org/show_bug.cgi?id=1854 Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- v2: lock pktio for stop and close in odp_pktio_term_global() (Nicolases note) platform/linux-generic/odp_packet_io.c | 119 ++++++++++++++++++++++----------- 1 file changed, 79 insertions(+), 40 deletions(-)