linux-generic: packet_io: fix state handling

Message ID	1459529044-25396-1-git-send-email-zoltan.kiss@linaro.org
State	Accepted
Commit	8b87bcf374aafe2d22bbb8f2ce7ffd9ed2ae10e3
Headers	show Delivered-To: patch@linaro.org Received-SPF: pass (google.com: domain of lng-odp-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; From: Zoltan Kiss <zoltan.kiss@linaro.org> To: lng-odp@lists.linaro.org Date: Fri, 1 Apr 2016 17:44:04 +0100 Message-Id: <1459529044-25396-1-git-send-email-zoltan.kiss@linaro.org> Cc: petri.savolainen@nokia.com Subject: [lng-odp] [PATCH] linux-generic: packet_io: fix state handling Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: lng-odp-bounces@lists.linaro.org Sender: "lng-odp" <lng-odp-bounces@lists.linaro.org>

Message ID

1459529044-25396-1-git-send-email-zoltan.kiss@linaro.org

State

Accepted

Commit

8b87bcf374aafe2d22bbb8f2ce7ffd9ed2ae10e3

Headers

Received-SPF: pass (google.com: domain of lng-odp-bounces@lists.linaro.org
	designates 54.225.227.206 as permitted sender)
	client-ip=54.225.227.206; 
From: Zoltan Kiss <zoltan.kiss@linaro.org>
To: lng-odp@lists.linaro.org
Date: Fri,  1 Apr 2016 17:44:04 +0100
Message-Id: <1459529044-25396-1-git-send-email-zoltan.kiss@linaro.org>
Cc: petri.savolainen@nokia.com
Subject: [lng-odp] [PATCH] linux-generic: packet_io: fix state handling
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: lng-odp-bounces@lists.linaro.org
Sender: "lng-odp" <lng-odp-bounces@lists.linaro.org>

Commit Message

Zoltan Kiss April 1, 2016, 4:44 p.m. UTC

The current two state doesn't tell odp_pktio_close() whether the device has ever
been started, which is important to decide whether it should flush anything or
not.
The new OPENED state only happen after odp_pktio_open(), after a start -> stop
transition it goes to a STOPPED state. Both valid for configuration, so instead
of "== STATE_STOPPED", check "!= STATE_STARTED".

Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---

Comments

Bill Fischofer April 3, 2016, 7:41 p.m. UTC | #1

On Fri, Apr 1, 2016 at 11:44 AM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote:

> The current two state doesn't tell odp_pktio_close() whether the device

> has ever

> been started, which is important to decide whether it should flush

> anything or

> not.

> The new OPENED state only happen after odp_pktio_open(), after a start ->

> stop

> transition it goes to a STOPPED state. Both valid for configuration, so

> instead

> of "== STATE_STOPPED", check "!= STATE_STARTED".

>

> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>

>


Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>


> ---

> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h

> b/platform/linux-generic/include/odp_packet_io_internal.h

> index 7636768..cca5c39 100644

> --- a/platform/linux-generic/include/odp_packet_io_internal.h

> +++ b/platform/linux-generic/include/odp_packet_io_internal.h

> @@ -89,8 +89,10 @@ struct pktio_entry {

>                 pkt_tap_t pkt_tap;              /**< using TAP for IO */

>         };

>         enum {

> -               STATE_START = 0,

> -               STATE_STOP

> +               STATE_OPENED = 0,       /**< After open() */

> +               STATE_STARTED,          /**< After start() */

> +               STATE_STOPPED           /**< Same as OPENED, but only

> happens

> +                                       after STARTED */

>         } state;

>         classifier_t cls;               /**< classifier linked with this

> pktio*/

>         odp_pktio_stats_t stats;        /**< statistic counters for pktio

> */

> diff --git a/platform/linux-generic/odp_packet_io.c

> b/platform/linux-generic/odp_packet_io.c

> index 9192be2..f927c15 100644

> --- a/platform/linux-generic/odp_packet_io.c

> +++ b/platform/linux-generic/odp_packet_io.c

> @@ -228,7 +228,7 @@ static odp_pktio_t setup_pktio_entry(const char *name,

> odp_pool_t pool,

>         } else {

>                 snprintf(pktio_entry->s.name,

>                          sizeof(pktio_entry->s.name), "%s", name);

> -               pktio_entry->s.state = STATE_STOP;

> +               pktio_entry->s.state = STATE_OPENED;

>         }

>

>         unlock_entry(pktio_entry);

> @@ -343,7 +343,8 @@ int odp_pktio_close(odp_pktio_t id)

>         if (entry == NULL)

>                 return -1;

>

> -       flush_in_queues(entry);

> +       if (entry->s.state == STATE_STOPPED)

> +               flush_in_queues(entry);

>

>         lock_entry(entry);

>

> @@ -374,14 +375,14 @@ int odp_pktio_start(odp_pktio_t id)

>                 return -1;

>

>         lock_entry(entry);

> -       if (entry->s.state == STATE_START) {

> +       if (entry->s.state == STATE_STARTED) {

>                 unlock_entry(entry);

>                 return -1;

>         }

>         if (entry->s.ops->start)

>                 res = entry->s.ops->start(entry);

>         if (!res)

> -               entry->s.state = STATE_START;

> +               entry->s.state = STATE_STARTED;

>

>         unlock_entry(entry);

>

> @@ -409,13 +410,13 @@ static int _pktio_stop(pktio_entry_t *entry)

>  {

>         int res = 0;

>

> -       if (entry->s.state == STATE_STOP)

> +       if (entry->s.state != STATE_STARTED)

>                 return -1;

>

>         if (entry->s.ops->stop)

>                 res = entry->s.ops->stop(entry);

>         if (!res)

> -               entry->s.state = STATE_STOP;

> +               entry->s.state = STATE_STOPPED;

>

>         return res;

>  }

> @@ -502,7 +503,7 @@ static int _odp_pktio_send(odp_pktio_t id,

> odp_packet_t pkt_table[], int len)

>                 return -1;

>

>         odp_ticketlock_lock(&pktio_entry->s.txl);

> -       if (pktio_entry->s.state == STATE_STOP ||

> +       if (pktio_entry->s.state != STATE_STARTED ||

>             pktio_entry->s.param.out_mode == ODP_PKTOUT_MODE_DISABLED) {

>                         odp_ticketlock_unlock(&pktio_entry->s.txl);

>                 __odp_errno = EPERM;

> @@ -650,7 +651,7 @@ int pktin_poll(pktio_entry_t *entry, int num_queue,

> int index[])

>         if (odp_unlikely(entry->s.num_in_queue == 0))

>                 return -1;

>

> -       if (entry->s.state == STATE_STOP)

> +       if (entry->s.state != STATE_STARTED)

>                 return 0;

>

>         for (idx = 0; idx < num_queue; idx++) {

> @@ -725,7 +726,7 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id,

> odp_bool_t enable)

>                 ODP_DBG("already freed pktio\n");

>                 return -1;

>         }

> -       if (entry->s.state != STATE_STOP) {

> +       if (entry->s.state == STATE_STARTED) {

>                 unlock_entry(entry);

>                 return -1;

>         }

> @@ -891,8 +892,10 @@ void odp_pktio_print(odp_pktio_t id)

>                         "  type         %s\n", entry->s.ops->name);

>         len += snprintf(&str[len], n - len,

>                         "  state        %s\n",

> -                       entry->s.state ==  STATE_START ? "start" :

> -                      (entry->s.state ==  STATE_STOP ? "stop" :

> "unknown"));

> +                       entry->s.state ==  STATE_STARTED ? "start" :

> +                      (entry->s.state ==  STATE_STOPPED ? "stop" :

> +                      (entry->s.state ==  STATE_OPENED ? "opened" :

> +                                                         "unknown")));

>         memset(addr, 0, sizeof(addr));

>         odp_pktio_mac_addr(id, addr, ETH_ALEN);

>         len += snprintf(&str[len], n - len,

> @@ -923,7 +926,7 @@ int odp_pktio_term_global(void)

>                         continue;

>

>                 lock_entry(pktio_entry);

> -               if (pktio_entry->s.state != STATE_STOP) {

> +               if (pktio_entry->s.state == STATE_STARTED) {

>                         ret = _pktio_stop(pktio_entry);

>                         if (ret)

>                                 ODP_ABORT("unable to stop pktio %s\n",

> @@ -1041,7 +1044,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio,

>                 return -1;

>         }

>

> -       if (entry->s.state != STATE_STOP) {

> +       if (entry->s.state == STATE_STARTED) {

>                 ODP_DBG("pktio %s: not stopped\n", entry->s.name);

>                 return -1;

>         }

> @@ -1148,7 +1151,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio,

>                 return -1;

>         }

>

> -       if (entry->s.state != STATE_STOP) {

> +       if (entry->s.state == STATE_STARTED) {

>                 ODP_DBG("pktio %s: not stopped\n", entry->s.name);

>                 return -1;

>         }

> diff --git a/platform/linux-generic/pktio/pcap.c

> b/platform/linux-generic/pktio/pcap.c

> index 8be8462..d1458bf 100644

> --- a/platform/linux-generic/pktio/pcap.c

> +++ b/platform/linux-generic/pktio/pcap.c

> @@ -212,7 +212,7 @@ static int pcapif_recv_pkt(pktio_entry_t *pktio_entry,

> odp_packet_t pkts[],

>         uint32_t pkt_len;

>         pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;

>

> -       if (pktio_entry->s.state == STATE_STOP)

> +       if (pktio_entry->s.state != STATE_STARTED)

>                 return 0;

>

>         if (!pcap->rx)

> @@ -295,7 +295,7 @@ static int pcapif_send_pkt(pktio_entry_t *pktio_entry,

> odp_packet_t pkts[],

>         pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;

>         unsigned i;

>

> -       ODP_ASSERT(pktio_entry->s.state == STATE_START);

> +       ODP_ASSERT(pktio_entry->s.state == STATE_STARTED);

>

>         for (i = 0; i < len; ++i) {

>                 int pkt_len = odp_packet_len(pkts[i]);

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>

Zoltan Kiss April 7, 2016, 1:57 p.m. UTC | #2

Ping, I think it only waits for merge

On 03/04/16 20:41, Bill Fischofer wrote:
>
>
> On Fri, Apr 1, 2016 at 11:44 AM, Zoltan Kiss <zoltan.kiss@linaro.org
> <mailto:zoltan.kiss@linaro.org>> wrote:
>
>     The current two state doesn't tell odp_pktio_close() whether the
>     device has ever
>     been started, which is important to decide whether it should flush
>     anything or
>     not.
>     The new OPENED state only happen after odp_pktio_open(), after a
>     start -> stop
>     transition it goes to a STOPPED state. Both valid for configuration,
>     so instead
>     of "== STATE_STOPPED", check "!= STATE_STARTED".
>
>     Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org
>     <mailto:zoltan.kiss@linaro.org>>
>
>
> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org
> <mailto:bill.fischofer@linaro.org>>
>
>     ---
>     diff --git a/platform/linux-generic/include/odp_packet_io_internal.h
>     b/platform/linux-generic/include/odp_packet_io_internal.h
>     index 7636768..cca5c39 100644
>     --- a/platform/linux-generic/include/odp_packet_io_internal.h
>     +++ b/platform/linux-generic/include/odp_packet_io_internal.h
>     @@ -89,8 +89,10 @@ struct pktio_entry {
>                      pkt_tap_t pkt_tap;              /**< using TAP for
>     IO */
>              };
>              enum {
>     -               STATE_START = 0,
>     -               STATE_STOP
>     +               STATE_OPENED = 0,       /**< After open() */
>     +               STATE_STARTED,          /**< After start() */
>     +               STATE_STOPPED           /**< Same as OPENED, but
>     only happens
>     +                                       after STARTED */
>              } state;
>              classifier_t cls;               /**< classifier linked with
>     this pktio*/
>              odp_pktio_stats_t stats;        /**< statistic counters for
>     pktio */
>     diff --git a/platform/linux-generic/odp_packet_io.c
>     b/platform/linux-generic/odp_packet_io.c
>     index 9192be2..f927c15 100644
>     --- a/platform/linux-generic/odp_packet_io.c
>     +++ b/platform/linux-generic/odp_packet_io.c
>     @@ -228,7 +228,7 @@ static odp_pktio_t setup_pktio_entry(const char
>     *name, odp_pool_t pool,
>              } else {
>                      snprintf(pktio_entry->s.name <http://s.name>,
>                               sizeof(pktio_entry->s.name
>     <http://s.name>), "%s", name);
>     -               pktio_entry->s.state = STATE_STOP;
>     +               pktio_entry->s.state = STATE_OPENED;
>              }
>
>              unlock_entry(pktio_entry);
>     @@ -343,7 +343,8 @@ int odp_pktio_close(odp_pktio_t id)
>              if (entry == NULL)
>                      return -1;
>
>     -       flush_in_queues(entry);
>     +       if (entry->s.state == STATE_STOPPED)
>     +               flush_in_queues(entry);
>
>              lock_entry(entry);
>
>     @@ -374,14 +375,14 @@ int odp_pktio_start(odp_pktio_t id)
>                      return -1;
>
>              lock_entry(entry);
>     -       if (entry->s.state == STATE_START) {
>     +       if (entry->s.state == STATE_STARTED) {
>                      unlock_entry(entry);
>                      return -1;
>              }
>              if (entry->s.ops->start)
>                      res = entry->s.ops->start(entry);
>              if (!res)
>     -               entry->s.state = STATE_START;
>     +               entry->s.state = STATE_STARTED;
>
>              unlock_entry(entry);
>
>     @@ -409,13 +410,13 @@ static int _pktio_stop(pktio_entry_t *entry)
>       {
>              int res = 0;
>
>     -       if (entry->s.state == STATE_STOP)
>     +       if (entry->s.state != STATE_STARTED)
>                      return -1;
>
>              if (entry->s.ops->stop)
>                      res = entry->s.ops->stop(entry);
>              if (!res)
>     -               entry->s.state = STATE_STOP;
>     +               entry->s.state = STATE_STOPPED;
>
>              return res;
>       }
>     @@ -502,7 +503,7 @@ static int _odp_pktio_send(odp_pktio_t id,
>     odp_packet_t pkt_table[], int len)
>                      return -1;
>
>              odp_ticketlock_lock(&pktio_entry->s.txl);
>     -       if (pktio_entry->s.state == STATE_STOP ||
>     +       if (pktio_entry->s.state != STATE_STARTED ||
>                  pktio_entry->s.param.out_mode ==
>     ODP_PKTOUT_MODE_DISABLED) {
>                              odp_ticketlock_unlock(&pktio_entry->s.txl);
>                      __odp_errno = EPERM;
>     @@ -650,7 +651,7 @@ int pktin_poll(pktio_entry_t *entry, int
>     num_queue, int index[])
>              if (odp_unlikely(entry->s.num_in_queue == 0))
>                      return -1;
>
>     -       if (entry->s.state == STATE_STOP)
>     +       if (entry->s.state != STATE_STARTED)
>                      return 0;
>
>              for (idx = 0; idx < num_queue; idx++) {
>     @@ -725,7 +726,7 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id,
>     odp_bool_t enable)
>                      ODP_DBG("already freed pktio\n");
>                      return -1;
>              }
>     -       if (entry->s.state != STATE_STOP) {
>     +       if (entry->s.state == STATE_STARTED) {
>                      unlock_entry(entry);
>                      return -1;
>              }
>     @@ -891,8 +892,10 @@ void odp_pktio_print(odp_pktio_t id)
>                              "  type         %s\n", entry->s.ops->name);
>              len += snprintf(&str[len], n - len,
>                              "  state        %s\n",
>     -                       entry->s.state ==  STATE_START ? "start" :
>     -                      (entry->s.state ==  STATE_STOP ? "stop" :
>     "unknown"));
>     +                       entry->s.state ==  STATE_STARTED ? "start" :
>     +                      (entry->s.state ==  STATE_STOPPED ? "stop" :
>     +                      (entry->s.state ==  STATE_OPENED ? "opened" :
>     +                                                         "unknown")));
>              memset(addr, 0, sizeof(addr));
>              odp_pktio_mac_addr(id, addr, ETH_ALEN);
>              len += snprintf(&str[len], n - len,
>     @@ -923,7 +926,7 @@ int odp_pktio_term_global(void)
>                              continue;
>
>                      lock_entry(pktio_entry);
>     -               if (pktio_entry->s.state != STATE_STOP) {
>     +               if (pktio_entry->s.state == STATE_STARTED) {
>                              ret = _pktio_stop(pktio_entry);
>                              if (ret)
>                                      ODP_ABORT("unable to stop pktio %s\n",
>     @@ -1041,7 +1044,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio,
>                      return -1;
>              }
>
>     -       if (entry->s.state != STATE_STOP) {
>     +       if (entry->s.state == STATE_STARTED) {
>                      ODP_DBG("pktio %s: not stopped\n", entry->s.name
>     <http://s.name>);
>                      return -1;
>              }
>     @@ -1148,7 +1151,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio,
>                      return -1;
>              }
>
>     -       if (entry->s.state != STATE_STOP) {
>     +       if (entry->s.state == STATE_STARTED) {
>                      ODP_DBG("pktio %s: not stopped\n", entry->s.name
>     <http://s.name>);
>                      return -1;
>              }
>     diff --git a/platform/linux-generic/pktio/pcap.c
>     b/platform/linux-generic/pktio/pcap.c
>     index 8be8462..d1458bf 100644
>     --- a/platform/linux-generic/pktio/pcap.c
>     +++ b/platform/linux-generic/pktio/pcap.c
>     @@ -212,7 +212,7 @@ static int pcapif_recv_pkt(pktio_entry_t
>     *pktio_entry, odp_packet_t pkts[],
>              uint32_t pkt_len;
>              pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;
>
>     -       if (pktio_entry->s.state == STATE_STOP)
>     +       if (pktio_entry->s.state != STATE_STARTED)
>                      return 0;
>
>              if (!pcap->rx)
>     @@ -295,7 +295,7 @@ static int pcapif_send_pkt(pktio_entry_t
>     *pktio_entry, odp_packet_t pkts[],
>              pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;
>              unsigned i;
>
>     -       ODP_ASSERT(pktio_entry->s.state == STATE_START);
>     +       ODP_ASSERT(pktio_entry->s.state == STATE_STARTED);
>
>              for (i = 0; i < len; ++i) {
>                      int pkt_len = odp_packet_len(pkts[i]);
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>

Maxim Uvarov April 7, 2016, 2:43 p.m. UTC | #3

Merged,
Maxim.


On 04/07/16 16:57, Zoltan Kiss wrote:
> Ping, I think it only waits for merge
>
> On 03/04/16 20:41, Bill Fischofer wrote:
>>
>>
>> On Fri, Apr 1, 2016 at 11:44 AM, Zoltan Kiss <zoltan.kiss@linaro.org
>> <mailto:zoltan.kiss@linaro.org>> wrote:
>>
>>     The current two state doesn't tell odp_pktio_close() whether the
>>     device has ever
>>     been started, which is important to decide whether it should flush
>>     anything or
>>     not.
>>     The new OPENED state only happen after odp_pktio_open(), after a
>>     start -> stop
>>     transition it goes to a STOPPED state. Both valid for configuration,
>>     so instead
>>     of "== STATE_STOPPED", check "!= STATE_STARTED".
>>
>>     Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org
>>     <mailto:zoltan.kiss@linaro.org>>
>>
>>
>> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org
>> <mailto:bill.fischofer@linaro.org>>
>>
>>     ---
>>     diff --git a/platform/linux-generic/include/odp_packet_io_internal.h
>>     b/platform/linux-generic/include/odp_packet_io_internal.h
>>     index 7636768..cca5c39 100644
>>     --- a/platform/linux-generic/include/odp_packet_io_internal.h
>>     +++ b/platform/linux-generic/include/odp_packet_io_internal.h
>>     @@ -89,8 +89,10 @@ struct pktio_entry {
>>                      pkt_tap_t pkt_tap;              /**< using TAP for
>>     IO */
>>              };
>>              enum {
>>     -               STATE_START = 0,
>>     -               STATE_STOP
>>     +               STATE_OPENED = 0,       /**< After open() */
>>     +               STATE_STARTED,          /**< After start() */
>>     +               STATE_STOPPED           /**< Same as OPENED, but
>>     only happens
>>     +                                       after STARTED */
>>              } state;
>>              classifier_t cls;               /**< classifier linked with
>>     this pktio*/
>>              odp_pktio_stats_t stats;        /**< statistic counters for
>>     pktio */
>>     diff --git a/platform/linux-generic/odp_packet_io.c
>>     b/platform/linux-generic/odp_packet_io.c
>>     index 9192be2..f927c15 100644
>>     --- a/platform/linux-generic/odp_packet_io.c
>>     +++ b/platform/linux-generic/odp_packet_io.c
>>     @@ -228,7 +228,7 @@ static odp_pktio_t setup_pktio_entry(const char
>>     *name, odp_pool_t pool,
>>              } else {
>>                      snprintf(pktio_entry->s.name <http://s.name>,
>>                               sizeof(pktio_entry->s.name
>>     <http://s.name>), "%s", name);
>>     -               pktio_entry->s.state = STATE_STOP;
>>     +               pktio_entry->s.state = STATE_OPENED;
>>              }
>>
>>              unlock_entry(pktio_entry);
>>     @@ -343,7 +343,8 @@ int odp_pktio_close(odp_pktio_t id)
>>              if (entry == NULL)
>>                      return -1;
>>
>>     -       flush_in_queues(entry);
>>     +       if (entry->s.state == STATE_STOPPED)
>>     +               flush_in_queues(entry);
>>
>>              lock_entry(entry);
>>
>>     @@ -374,14 +375,14 @@ int odp_pktio_start(odp_pktio_t id)
>>                      return -1;
>>
>>              lock_entry(entry);
>>     -       if (entry->s.state == STATE_START) {
>>     +       if (entry->s.state == STATE_STARTED) {
>>                      unlock_entry(entry);
>>                      return -1;
>>              }
>>              if (entry->s.ops->start)
>>                      res = entry->s.ops->start(entry);
>>              if (!res)
>>     -               entry->s.state = STATE_START;
>>     +               entry->s.state = STATE_STARTED;
>>
>>              unlock_entry(entry);
>>
>>     @@ -409,13 +410,13 @@ static int _pktio_stop(pktio_entry_t *entry)
>>       {
>>              int res = 0;
>>
>>     -       if (entry->s.state == STATE_STOP)
>>     +       if (entry->s.state != STATE_STARTED)
>>                      return -1;
>>
>>              if (entry->s.ops->stop)
>>                      res = entry->s.ops->stop(entry);
>>              if (!res)
>>     -               entry->s.state = STATE_STOP;
>>     +               entry->s.state = STATE_STOPPED;
>>
>>              return res;
>>       }
>>     @@ -502,7 +503,7 @@ static int _odp_pktio_send(odp_pktio_t id,
>>     odp_packet_t pkt_table[], int len)
>>                      return -1;
>>
>>              odp_ticketlock_lock(&pktio_entry->s.txl);
>>     -       if (pktio_entry->s.state == STATE_STOP ||
>>     +       if (pktio_entry->s.state != STATE_STARTED ||
>>                  pktio_entry->s.param.out_mode ==
>>     ODP_PKTOUT_MODE_DISABLED) {
>> odp_ticketlock_unlock(&pktio_entry->s.txl);
>>                      __odp_errno = EPERM;
>>     @@ -650,7 +651,7 @@ int pktin_poll(pktio_entry_t *entry, int
>>     num_queue, int index[])
>>              if (odp_unlikely(entry->s.num_in_queue == 0))
>>                      return -1;
>>
>>     -       if (entry->s.state == STATE_STOP)
>>     +       if (entry->s.state != STATE_STARTED)
>>                      return 0;
>>
>>              for (idx = 0; idx < num_queue; idx++) {
>>     @@ -725,7 +726,7 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id,
>>     odp_bool_t enable)
>>                      ODP_DBG("already freed pktio\n");
>>                      return -1;
>>              }
>>     -       if (entry->s.state != STATE_STOP) {
>>     +       if (entry->s.state == STATE_STARTED) {
>>                      unlock_entry(entry);
>>                      return -1;
>>              }
>>     @@ -891,8 +892,10 @@ void odp_pktio_print(odp_pktio_t id)
>>                              "  type         %s\n", entry->s.ops->name);
>>              len += snprintf(&str[len], n - len,
>>                              "  state        %s\n",
>>     -                       entry->s.state ==  STATE_START ? "start" :
>>     -                      (entry->s.state ==  STATE_STOP ? "stop" :
>>     "unknown"));
>>     +                       entry->s.state ==  STATE_STARTED ? "start" :
>>     +                      (entry->s.state ==  STATE_STOPPED ? "stop" :
>>     +                      (entry->s.state ==  STATE_OPENED ? "opened" :
>>     + "unknown")));
>>              memset(addr, 0, sizeof(addr));
>>              odp_pktio_mac_addr(id, addr, ETH_ALEN);
>>              len += snprintf(&str[len], n - len,
>>     @@ -923,7 +926,7 @@ int odp_pktio_term_global(void)
>>                              continue;
>>
>>                      lock_entry(pktio_entry);
>>     -               if (pktio_entry->s.state != STATE_STOP) {
>>     +               if (pktio_entry->s.state == STATE_STARTED) {
>>                              ret = _pktio_stop(pktio_entry);
>>                              if (ret)
>>                                      ODP_ABORT("unable to stop pktio 
>> %s\n",
>>     @@ -1041,7 +1044,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio,
>>                      return -1;
>>              }
>>
>>     -       if (entry->s.state != STATE_STOP) {
>>     +       if (entry->s.state == STATE_STARTED) {
>>                      ODP_DBG("pktio %s: not stopped\n", entry->s.name
>>     <http://s.name>);
>>                      return -1;
>>              }
>>     @@ -1148,7 +1151,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio,
>>                      return -1;
>>              }
>>
>>     -       if (entry->s.state != STATE_STOP) {
>>     +       if (entry->s.state == STATE_STARTED) {
>>                      ODP_DBG("pktio %s: not stopped\n", entry->s.name
>>     <http://s.name>);
>>                      return -1;
>>              }
>>     diff --git a/platform/linux-generic/pktio/pcap.c
>>     b/platform/linux-generic/pktio/pcap.c
>>     index 8be8462..d1458bf 100644
>>     --- a/platform/linux-generic/pktio/pcap.c
>>     +++ b/platform/linux-generic/pktio/pcap.c
>>     @@ -212,7 +212,7 @@ static int pcapif_recv_pkt(pktio_entry_t
>>     *pktio_entry, odp_packet_t pkts[],
>>              uint32_t pkt_len;
>>              pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;
>>
>>     -       if (pktio_entry->s.state == STATE_STOP)
>>     +       if (pktio_entry->s.state != STATE_STARTED)
>>                      return 0;
>>
>>              if (!pcap->rx)
>>     @@ -295,7 +295,7 @@ static int pcapif_send_pkt(pktio_entry_t
>>     *pktio_entry, odp_packet_t pkts[],
>>              pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;
>>              unsigned i;
>>
>>     -       ODP_ASSERT(pktio_entry->s.state == STATE_START);
>>     +       ODP_ASSERT(pktio_entry->s.state == STATE_STARTED);
>>
>>              for (i = 0; i < len; ++i) {
>>                      int pkt_len = odp_packet_len(pkts[i]);
>>     _______________________________________________
>>     lng-odp mailing list
>>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>>     https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp

diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h
index 7636768..cca5c39 100644
--- a/platform/linux-generic/include/odp_packet_io_internal.h
+++ b/platform/linux-generic/include/odp_packet_io_internal.h
@@ -89,8 +89,10 @@  struct pktio_entry {
 		pkt_tap_t pkt_tap;		/**< using TAP for IO */
 	};
 	enum {
-		STATE_START = 0,
-		STATE_STOP
+		STATE_OPENED = 0,	/**< After open() */
+		STATE_STARTED,		/**< After start() */
+		STATE_STOPPED		/**< Same as OPENED, but only happens
+					after STARTED */
 	} state;
 	classifier_t cls;		/**< classifier linked with this pktio*/
 	odp_pktio_stats_t stats;	/**< statistic counters for pktio */
diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
index 9192be2..f927c15 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -228,7 +228,7 @@  static odp_pktio_t setup_pktio_entry(const char *name, odp_pool_t pool,
 	} else {
 		snprintf(pktio_entry->s.name,
 			 sizeof(pktio_entry->s.name), "%s", name);
-		pktio_entry->s.state = STATE_STOP;
+		pktio_entry->s.state = STATE_OPENED;
 	}
 
 	unlock_entry(pktio_entry);
@@ -343,7 +343,8 @@  int odp_pktio_close(odp_pktio_t id)
 	if (entry == NULL)
 		return -1;
 
-	flush_in_queues(entry);
+	if (entry->s.state == STATE_STOPPED)
+		flush_in_queues(entry);
 
 	lock_entry(entry);
 
@@ -374,14 +375,14 @@  int odp_pktio_start(odp_pktio_t id)
 		return -1;
 
 	lock_entry(entry);
-	if (entry->s.state == STATE_START) {
+	if (entry->s.state == STATE_STARTED) {
 		unlock_entry(entry);
 		return -1;
 	}
 	if (entry->s.ops->start)
 		res = entry->s.ops->start(entry);
 	if (!res)
-		entry->s.state = STATE_START;
+		entry->s.state = STATE_STARTED;
 
 	unlock_entry(entry);
 
@@ -409,13 +410,13 @@  static int _pktio_stop(pktio_entry_t *entry)
 {
 	int res = 0;
 
-	if (entry->s.state == STATE_STOP)
+	if (entry->s.state != STATE_STARTED)
 		return -1;
 
 	if (entry->s.ops->stop)
 		res = entry->s.ops->stop(entry);
 	if (!res)
-		entry->s.state = STATE_STOP;
+		entry->s.state = STATE_STOPPED;
 
 	return res;
 }
@@ -502,7 +503,7 @@  static int _odp_pktio_send(odp_pktio_t id, odp_packet_t pkt_table[], int len)
 		return -1;
 
 	odp_ticketlock_lock(&pktio_entry->s.txl);
-	if (pktio_entry->s.state == STATE_STOP ||
+	if (pktio_entry->s.state != STATE_STARTED ||
 	    pktio_entry->s.param.out_mode == ODP_PKTOUT_MODE_DISABLED) {
 			odp_ticketlock_unlock(&pktio_entry->s.txl);
 		__odp_errno = EPERM;
@@ -650,7 +651,7 @@  int pktin_poll(pktio_entry_t *entry, int num_queue, int index[])
 	if (odp_unlikely(entry->s.num_in_queue == 0))
 		return -1;
 
-	if (entry->s.state == STATE_STOP)
+	if (entry->s.state != STATE_STARTED)
 		return 0;
 
 	for (idx = 0; idx < num_queue; idx++) {
@@ -725,7 +726,7 @@  int odp_pktio_promisc_mode_set(odp_pktio_t id, odp_bool_t enable)
 		ODP_DBG("already freed pktio\n");
 		return -1;
 	}
-	if (entry->s.state != STATE_STOP) {
+	if (entry->s.state == STATE_STARTED) {
 		unlock_entry(entry);
 		return -1;
 	}
@@ -891,8 +892,10 @@  void odp_pktio_print(odp_pktio_t id)
 			"  type         %s\n", entry->s.ops->name);
 	len += snprintf(&str[len], n - len,
 			"  state        %s\n",
-			entry->s.state ==  STATE_START ? "start" :
-		       (entry->s.state ==  STATE_STOP ? "stop" : "unknown"));
+			entry->s.state ==  STATE_STARTED ? "start" :
+		       (entry->s.state ==  STATE_STOPPED ? "stop" :
+		       (entry->s.state ==  STATE_OPENED ? "opened" :
+							  "unknown")));
 	memset(addr, 0, sizeof(addr));
 	odp_pktio_mac_addr(id, addr, ETH_ALEN);
 	len += snprintf(&str[len], n - len,
@@ -923,7 +926,7 @@  int odp_pktio_term_global(void)
 			continue;
 
 		lock_entry(pktio_entry);
-		if (pktio_entry->s.state != STATE_STOP) {
+		if (pktio_entry->s.state == STATE_STARTED) {
 			ret = _pktio_stop(pktio_entry);
 			if (ret)
 				ODP_ABORT("unable to stop pktio %s\n",
@@ -1041,7 +1044,7 @@  int odp_pktin_queue_config(odp_pktio_t pktio,
 		return -1;
 	}
 
-	if (entry->s.state != STATE_STOP) {
+	if (entry->s.state == STATE_STARTED) {
 		ODP_DBG("pktio %s: not stopped\n", entry->s.name);
 		return -1;
 	}
@@ -1148,7 +1151,7 @@  int odp_pktout_queue_config(odp_pktio_t pktio,
 		return -1;
 	}
 
-	if (entry->s.state != STATE_STOP) {
+	if (entry->s.state == STATE_STARTED) {
 		ODP_DBG("pktio %s: not stopped\n", entry->s.name);
 		return -1;
 	}
diff --git a/platform/linux-generic/pktio/pcap.c b/platform/linux-generic/pktio/pcap.c
index 8be8462..d1458bf 100644
--- a/platform/linux-generic/pktio/pcap.c
+++ b/platform/linux-generic/pktio/pcap.c
@@ -212,7 +212,7 @@  static int pcapif_recv_pkt(pktio_entry_t *pktio_entry, odp_packet_t pkts[],
 	uint32_t pkt_len;
 	pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;
 
-	if (pktio_entry->s.state == STATE_STOP)
+	if (pktio_entry->s.state != STATE_STARTED)
 		return 0;
 
 	if (!pcap->rx)
@@ -295,7 +295,7 @@  static int pcapif_send_pkt(pktio_entry_t *pktio_entry, odp_packet_t pkts[],
 	pkt_pcap_t *pcap = &pktio_entry->s.pkt_pcap;
 	unsigned i;
 
-	ODP_ASSERT(pktio_entry->s.state == STATE_START);
+	ODP_ASSERT(pktio_entry->s.state == STATE_STARTED);
 
 	for (i = 0; i < len; ++i) {
 		int pkt_len = odp_packet_len(pkts[i]);

linux-generic: packet_io: fix state handling

Commit Message

Comments

Patch