On Fri, Jul 29, 2016 at 8:41 AM, Maxim Uvarov <[email protected]>
wrote:

> Add clean termination path for ipc pktio example to destroy
> all created resouces including openned shared memory.
>
> Signed-off-by: Maxim Uvarov <[email protected]>
>

Reviewed-by: Bill Fischofer <[email protected]>


> ---
>  v2: do not touch shm_free. (Bill's comment)
>
>  platform/linux-generic/pktio/ipc.c                 | 31 +++++++--------
>  test/platform/linux-generic/pktio_ipc/ipc_common.c | 15 ++++++--
>  test/platform/linux-generic/pktio_ipc/ipc_common.h |  5 ++-
>  test/platform/linux-generic/pktio_ipc/pktio_ipc1.c | 45
> ++++++++++++++--------
>  test/platform/linux-generic/pktio_ipc/pktio_ipc2.c | 33 +++++++++++-----
>  .../linux-generic/pktio_ipc/pktio_ipc_run.sh       |  1 +
>  6 files changed, 81 insertions(+), 49 deletions(-)
>
> diff --git a/platform/linux-generic/pktio/ipc.c
> b/platform/linux-generic/pktio/ipc.c
> index f9e7a00..60779df 100644
> --- a/platform/linux-generic/pktio/ipc.c
> +++ b/platform/linux-generic/pktio/ipc.c
> @@ -721,6 +721,8 @@ static int ipc_start(pktio_entry_t *pktio_entry)
>
>  static int ipc_stop(pktio_entry_t *pktio_entry)
>  {
> +       unsigned tx_send, tx_free;
> +
>         odp_atomic_store_u32(&pktio_entry->s.ipc.ready, 0);
>
>         _ipc_free_ring_packets(pktio_entry->s.ipc.tx.send);
> @@ -729,21 +731,28 @@ static int ipc_stop(pktio_entry_t *pktio_entry)
>         sleep(1);
>         _ipc_free_ring_packets(pktio_entry->s.ipc.tx.free);
>
> +       tx_send = _ring_count(pktio_entry->s.ipc.tx.send);
> +       tx_free = _ring_count(pktio_entry->s.ipc.tx.free);
> +       if (tx_send | tx_free) {
> +               ODP_DBG("IPC rings: tx send %d tx free %d\n",
> +                       _ring_free_count(pktio_entry->s.ipc.tx.send),
> +                       _ring_free_count(pktio_entry->s.ipc.tx.free));
> +       }
> +
>         return 0;
>  }
>
>  static int ipc_close(pktio_entry_t *pktio_entry)
>  {
> -       odp_shm_t shm;
>         char ipc_shm_name[ODP_POOL_NAME_LEN + sizeof("_m_prod")];
>         char *dev = pktio_entry->s.name;
>
>         ipc_stop(pktio_entry);
>
> -       /* unlink this pktio info for both master and slave */
> -       odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
> -
>         if (pktio_entry->s.ipc.type == PKTIO_TYPE_IPC_MASTER) {
> +               /* unlink this pktio info for both master and slave */
> +               odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
> +
>                 /* destroy rings */
>                 snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons",
> dev);
>                 _ring_destroy(ipc_shm_name);
> @@ -753,20 +762,6 @@ static int ipc_close(pktio_entry_t *pktio_entry)
>                 _ring_destroy(ipc_shm_name);
>                 snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod",
> dev);
>                 _ring_destroy(ipc_shm_name);
> -       } else {
> -               /* unlink rings */
> -               snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons",
> dev);
> -               shm = odp_shm_lookup(ipc_shm_name);
> -               odp_shm_free(shm);
> -               snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_prod",
> dev);
> -               shm = odp_shm_lookup(ipc_shm_name);
> -               odp_shm_free(shm);
> -               snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_cons",
> dev);
> -               shm = odp_shm_lookup(ipc_shm_name);
> -               odp_shm_free(shm);
> -               snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod",
> dev);
> -               shm = odp_shm_lookup(ipc_shm_name);
> -               odp_shm_free(shm);
>         }
>
>         return 0;
> diff --git a/test/platform/linux-generic/pktio_ipc/ipc_common.c
> b/test/platform/linux-generic/pktio_ipc/ipc_common.c
> index 2ee326e..387c921 100644
> --- a/test/platform/linux-generic/pktio_ipc/ipc_common.c
> +++ b/test/platform/linux-generic/pktio_ipc/ipc_common.c
> @@ -10,8 +10,8 @@
>  int run_time_sec;
>  int ipc_name_space;
>
> -int ipc_odp_packet_sendall(odp_pktio_t pktio,
> -                          odp_packet_t pkt_tbl[], int num)
> +int ipc_odp_packet_send_or_free(odp_pktio_t pktio,
> +                               odp_packet_t pkt_tbl[], int num)
>  {
>         int ret;
>         int sent = 0;
> @@ -19,6 +19,7 @@ int ipc_odp_packet_sendall(odp_pktio_t pktio,
>         odp_time_t end_time;
>         odp_time_t wait;
>         odp_pktout_queue_t pktout;
> +       int i;
>
>         start_time = odp_time_local();
>         wait = odp_time_local_from_ns(ODP_TIME_SEC_IN_NS);
> @@ -31,13 +32,19 @@ int ipc_odp_packet_sendall(odp_pktio_t pktio,
>
>         while (sent != num) {
>                 ret = odp_pktout_send(pktout, &pkt_tbl[sent], num - sent);
> -               if (ret < 0)
> +               if (ret < 0) {
> +                       for (i = sent; i < num; i++)
> +                               odp_packet_free(pkt_tbl[i]);
>                         return -1;
> +               }
>
>                 sent += ret;
>
> -               if (odp_time_cmp(end_time, odp_time_local()) < 0)
> +               if (odp_time_cmp(end_time, odp_time_local()) < 0) {
> +                       for (i = sent; i < num; i++)
> +                               odp_packet_free(pkt_tbl[i]);
>                         return -1;
> +               }
>         }
>
>         return 0;
> diff --git a/test/platform/linux-generic/pktio_ipc/ipc_common.h
> b/test/platform/linux-generic/pktio_ipc/ipc_common.h
> index a6b7c58..99276b5 100644
> --- a/test/platform/linux-generic/pktio_ipc/ipc_common.h
> +++ b/test/platform/linux-generic/pktio_ipc/ipc_common.h
> @@ -87,5 +87,6 @@ odp_pktio_t create_pktio(odp_pool_t pool);
>   * @param pkt_tbl      packets table
>   * @param num          number of packets
>   */
> -int ipc_odp_packet_sendall(odp_pktio_t pktio,
> -                          odp_packet_t pkt_tbl[], int num);
> +int ipc_odp_packet_send_or_free(odp_pktio_t pktio,
> +                               odp_packet_t pkt_tbl[],
> +                               int num);
> diff --git a/test/platform/linux-generic/pktio_ipc/pktio_ipc1.c
> b/test/platform/linux-generic/pktio_ipc/pktio_ipc1.c
> index a4eed88..5c1da23 100644
> --- a/test/platform/linux-generic/pktio_ipc/pktio_ipc1.c
> +++ b/test/platform/linux-generic/pktio_ipc/pktio_ipc1.c
> @@ -34,6 +34,7 @@ static int pktio_run_loop(odp_pool_t pool)
>         uint64_t stat_pkts_alloc = 0;
>         uint64_t stat_pkts_prev = 0;
>         uint64_t stat_errors = 0;
> +       uint64_t stat_free = 0;
>         odp_time_t start_cycle;
>         odp_time_t current_cycle;
>         odp_time_t cycle;
> @@ -118,19 +119,21 @@ static int pktio_run_loop(odp_pool_t pool)
>                                                              &head);
>                                 if (ret) {
>                                         stat_errors++;
> +                                       stat_free++;
>                                         odp_packet_free(pkt);
>                                         EXAMPLE_DBG("error\n");
>                                         continue;
>                                 }
>
>                                 if (head.magic == TEST_ALLOC_MAGIC) {
> -                                       stat_pkts_alloc++;
> +                                       stat_free++;
>                                         odp_packet_free(pkt);
>                                         continue;
>                                 }
>
>                                 if (head.magic != TEST_SEQ_MAGIC_2) {
>                                         stat_errors++;
> +                                       stat_free++;
>                                         odp_packet_free(pkt);
>                                         EXAMPLE_DBG("error\n");
>                                         continue;
> @@ -142,12 +145,14 @@ static int pktio_run_loop(odp_pool_t pool)
>                                                              &tail);
>                                 if (ret) {
>                                         stat_errors++;
> +                                       stat_free++;
>                                         odp_packet_free(pkt);
>                                         continue;
>                                 }
>
>                                 if (tail.magic != TEST_SEQ_MAGIC) {
>                                         stat_errors++;
> +                                       stat_free++;
>                                         odp_packet_free(pkt);
>                                         continue;
>                                 }
> @@ -163,6 +168,8 @@ static int pktio_run_loop(odp_pool_t pool)
>                                                     head.seq, cnt_recv,
>                                                     head.seq - cnt_recv);
>                                         cnt_recv = head.seq;
> +                                       stat_errors++;
> +                                       stat_free++;
>                                         continue;
>                                 }
>
> @@ -182,6 +189,7 @@ static int pktio_run_loop(odp_pool_t pool)
>                         if (pkt == ODP_PACKET_INVALID)
>                                 break;
>
> +                       stat_pkts_alloc++;
>                         odp_packet_l4_offset_set(pkt, 30);
>                         pkt_tbl[i] = pkt;
>                 }
> @@ -224,7 +232,7 @@ static int pktio_run_loop(odp_pool_t pool)
>                 }
>
>                 /* 5. Send packets to ipc_pktio */
> -               ret = ipc_odp_packet_sendall(ipc_pktio, pkt_tbl, pkts);
> +               ret = ipc_odp_packet_send_or_free(ipc_pktio, pkt_tbl,
> pkts);
>                 if (ret < 0) {
>                         EXAMPLE_DBG("unable to sending to ipc pktio\n");
>                         break;
> @@ -236,9 +244,11 @@ static int pktio_run_loop(odp_pool_t pool)
>                                  diff) < 0) {
>                         current_cycle = cycle;
>                         printf("\rpkts:  %" PRIu64 ", alloc  %" PRIu64 ","
> -                              " errors %" PRIu64 ", pps  %" PRIu64 ".",
> +                              " errors %" PRIu64 ", pps  %" PRIu64 ","
> +                              " free %" PRIu64 ".",
>                                stat_pkts, stat_pkts_alloc, stat_errors,
> -                              (stat_pkts + stat_pkts_alloc -
> stat_pkts_prev));
> +                              (stat_pkts + stat_pkts_alloc -
> stat_pkts_prev),
> +                              stat_free);
>                         fflush(stdout);
>                         stat_pkts_prev = stat_pkts + stat_pkts_alloc;
>                 }
> @@ -258,18 +268,6 @@ exit:
>                 return -1;
>         }
>
> -       ret = odp_pool_destroy(pool);
> -       if (ret) {
> -               EXAMPLE_DBG("pool_destroy error %d\n", ret);
> -               /* Remote process can end with reference to our local pool.
> -                * Usully it unmaps it clenealy but some time there are
> some
> -                * pending packets in the pool in case of remote process
> was
> -                * trapped or did not call odp_pktio_close() correctly and
> -                * release buffers and free buffer from shared rings.
> -                * return -1;
> -                */
> -       }
> -
>         return (stat_errors > 10 || stat_pkts < 1000) ? -1 : 0;
>  }
>
> @@ -324,6 +322,21 @@ int main(int argc, char *argv[])
>
>         ret = pktio_run_loop(pool);
>
> +       if (odp_pool_destroy(pool)) {
> +               EXAMPLE_ERR("Error: odp_pool_destroy() failed.\n");
> +               exit(EXIT_FAILURE);
> +       }
> +
> +       if (odp_term_local()) {
> +               EXAMPLE_ERR("Error: odp_term_local() failed.\n");
> +               exit(EXIT_FAILURE);
> +       }
> +
> +       if (odp_term_global(instance)) {
> +               EXAMPLE_ERR("Error: odp_term_global() failed.\n");
> +               exit(EXIT_FAILURE);
> +       }
> +
>         EXAMPLE_DBG("return %d\n", ret);
>         return ret;
>  }
> diff --git a/test/platform/linux-generic/pktio_ipc/pktio_ipc2.c
> b/test/platform/linux-generic/pktio_ipc/pktio_ipc2.c
> index c0c6ff5..5c1f142 100644
> --- a/test/platform/linux-generic/pktio_ipc/pktio_ipc2.c
> +++ b/test/platform/linux-generic/pktio_ipc/pktio_ipc2.c
> @@ -65,7 +65,7 @@ static int ipc_second_process(void)
>                         if (odp_time_cmp(wait, diff) < 0) {
>                                 printf("timeout exit, run_time_sec %d\n",
>                                        run_time_sec);
> -                               goto exit;
> +                               goto not_started;
>                         }
>                 }
>
> @@ -118,10 +118,11 @@ static int ipc_second_process(void)
>                 }
>
>                 /* send all packets back */
> -               ret = ipc_odp_packet_sendall(ipc_pktio, pkt_tbl, pkts);
> +               ret = ipc_odp_packet_send_or_free(ipc_pktio, pkt_tbl,
> pkts);
>                 if (ret < 0)
>                         EXAMPLE_ABORT("can not send packets\n");
> -               stat_pkts += pkts;
> +
> +               stat_pkts += ret;
>
>                 /* alloc packet from local pool, set magic to ALLOC_MAGIC,
>                  * and send it.*/
> @@ -143,7 +144,8 @@ static int ipc_second_process(void)
>                                 EXAMPLE_ABORT("unable to copy in head
> data");
>
>                         pkt_tbl[0] = alloc_pkt;
> -                       ret = ipc_odp_packet_sendall(ipc_pktio, pkt_tbl,
> 1);
> +                       ret = ipc_odp_packet_send_or_free(ipc_pktio,
> +                                                         pkt_tbl, 1);
>                         if (ret < 0)
>                                 EXAMPLE_ABORT("can not send packets\n");
>                         stat_pkts += 1;
> @@ -153,20 +155,20 @@ static int ipc_second_process(void)
>         /* cleanup and exit */
>         ret = odp_pktio_stop(ipc_pktio);
>         if (ret) {
> -               EXAMPLE_DBG("odp_pktio_stop error %d\n", ret);
> +               EXAMPLE_DBG("ipc2: odp_pktio_stop error %d\n", ret);
>                 return -1;
>         }
>
> -exit:
> +not_started:
>         ret = odp_pktio_close(ipc_pktio);
>         if (ret) {
> -               EXAMPLE_DBG("odp_pktio_close error %d\n", ret);
> +               EXAMPLE_DBG("ipc2: odp_pktio_close error %d\n", ret);
>                 return -1;
>         }
>
>         ret = odp_pool_destroy(pool);
>         if (ret)
> -               EXAMPLE_DBG("pool_destroy error %d\n", ret);
> +               EXAMPLE_DBG("ipc2: pool_destroy error %d\n", ret);
>
>         return stat_pkts > 1000 ? 0 : -1;
>  }
> @@ -175,6 +177,7 @@ int main(int argc, char *argv[])
>  {
>         odp_instance_t instance;
>         odp_platform_init_t plat_idata;
> +       int ret;
>
>         /* Parse and store the application arguments */
>         parse_args(argc, argv);
> @@ -193,5 +196,17 @@ int main(int argc, char *argv[])
>                 exit(EXIT_FAILURE);
>         }
>
> -       return ipc_second_process();
> +       ret = ipc_second_process();
> +
> +       if (odp_term_local()) {
> +               EXAMPLE_ERR("Error: odp_term_local() failed.\n");
> +               exit(EXIT_FAILURE);
> +       }
> +
> +       if (odp_term_global(instance)) {
> +               EXAMPLE_ERR("Error: odp_term_global() failed.\n");
> +               exit(EXIT_FAILURE);
> +       }
> +
> +       return ret;
>  }
> diff --git a/test/platform/linux-generic/pktio_ipc/pktio_ipc_run.sh
> b/test/platform/linux-generic/pktio_ipc/pktio_ipc_run.sh
> index 1128002..2f99f32 100755
> --- a/test/platform/linux-generic/pktio_ipc/pktio_ipc_run.sh
> +++ b/test/platform/linux-generic/pktio_ipc/pktio_ipc_run.sh
> @@ -49,6 +49,7 @@ run()
>         fi
>
>         echo "==== run pktio_ipc2 then pktio_ipc1 ===="
> +       IPC_NS=`expr $IPC_NS - 1`
>         pktio_ipc2${EXEEXT} -n ${IPC_NS} -t 10 &
>         IPC_PID=$!
>
> --
> 1.9.1
>
>

Reply via email to