diff mbox

[PATCHv3] simplify pktin_dequeue/pktin_deq_multi functions

Message ID 1416322057-6444-1-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Nov. 18, 2014, 2:47 p.m. UTC 
Mostly keep original logic but make function more clear.
In pktin_deq_multi() do not do actual rx if there are enough
packets in the queue.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 v3: - use pkts instead if i in queue_enq_multi();
     - do not do actual rx if there are enough
packets in the queue.
 v2: - fixes for checks that Ciprian found.

 Tested this patch in lxc with ping -f for both burst and queue modes.

 Maxim.

 platform/linux-generic/odp_packet_io.c | 74 +++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 37 deletions(-)

Comments

Ciprian Barbu Nov. 19, 2014, 9:55 a.m. UTC | #1 
On Tue, Nov 18, 2014 at 4:47 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> Mostly keep original logic but make function more clear.
> In pktin_deq_multi() do not do actual rx if there are enough
> packets in the queue.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  v3: - use pkts instead if i in queue_enq_multi();
>      - do not do actual rx if there are enough
> packets in the queue.
>  v2: - fixes for checks that Ciprian found.
>
>  Tested this patch in lxc with ping -f for both burst and queue modes.
>
>  Maxim.
>
>  platform/linux-generic/odp_packet_io.c | 74 +++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
> index f35193f..039003d 100644
> --- a/platform/linux-generic/odp_packet_io.c
> +++ b/platform/linux-generic/odp_packet_io.c
> @@ -416,32 +416,26 @@ int pktin_enqueue(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr)
>  odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *qentry)
>  {
>         odp_buffer_hdr_t *buf_hdr;
> +       odp_buffer_t buf;
> +       odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
> +       odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
> +       int pkts, i;
>
>         buf_hdr = queue_deq(qentry);
> +       if (buf_hdr != NULL)
> +               return buf_hdr;
>
> -       if (buf_hdr == NULL) {
> -               odp_packet_t pkt;
> -               odp_buffer_t buf;
> -               odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
> -               odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
> -               int pkts, i, j;
> -
> -               pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl,
> -                                     QUEUE_MULTI_MAX);
> -
> -               if (pkts > 0) {
> -                       pkt = pkt_tbl[0];
> -                       buf = odp_packet_to_buffer(pkt);
> -                       buf_hdr = odp_buf_to_hdr(buf);
> +       pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl, QUEUE_MULTI_MAX);
> +       if (pkts <= 0)
> +               return NULL;
>
> -                       for (i = 1, j = 0; i < pkts; ++i) {
> -                               buf = odp_packet_to_buffer(pkt_tbl[i]);
> -                               tmp_hdr_tbl[j++] = odp_buf_to_hdr(buf);
> -                       }
> -                       queue_enq_multi(qentry, tmp_hdr_tbl, j);
> -               }
> +       for (i = 0; i < pkts; ++i) {
> +               buf = odp_packet_to_buffer(pkt_tbl[i]);
> +               tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
>         }
>
> +       queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
> +       buf_hdr = tmp_hdr_tbl[0];
>         return buf_hdr;
>  }
>
> @@ -454,25 +448,31 @@ int pktin_enq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num)
>  int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num)
>  {
>         int nbr;
> +       odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
> +       odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
> +       odp_buffer_t buf;
> +       int pkts, i;
>
>         nbr = queue_deq_multi(qentry, buf_hdr, num);
> -
> -       if (nbr < num) {
> -               odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
> -               odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
> -               odp_buffer_t buf;
> -               int pkts, i;
> -
> -               pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl,
> -                                     QUEUE_MULTI_MAX);
> -               if (pkts > 0) {
> -                       for (i = 0; i < pkts; ++i) {
> -                               buf = odp_packet_to_buffer(pkt_tbl[i]);
> -                               tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
> -                       }
> -                       queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
> -               }
> +       if (nbr > num)

Not necessarily needed, but using odp_unlikely may be a good idea.

> +               ODP_ABORT("queue_deq_multi req: %d, returned %d\n",
> +                       num, nbr);
> +
> +       /** queue already has number of requsted buffers,
> +        *  do not do recieve in that case.
> +        */
> +       if (nbr == num)
> +               return nbr;
> +
> +       pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl, QUEUE_MULTI_MAX);
> +       if (pkts <= 0)
> +               return nbr;
> +
> +       for (i = 0; i < pkts; ++i) {
> +               buf = odp_packet_to_buffer(pkt_tbl[i]);
> +               tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
>         }
> -
> +       queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
> +       buf_hdr = &tmp_hdr_tbl[0];

Please remove this, I think you mistakenly copiedit from pktin_dequeue.

>         return nbr;
>  }
> --
> 1.8.5.1.163.gd7aced9
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Maxim Uvarov Nov. 19, 2014, 12:33 p.m. UTC | #2 
On 11/19/2014 12:55 PM, Ciprian Barbu wrote:
> On Tue, Nov 18, 2014 at 4:47 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>> Mostly keep original logic but make function more clear.
>> In pktin_deq_multi() do not do actual rx if there are enough
>> packets in the queue.
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   v3: - use pkts instead if i in queue_enq_multi();
>>       - do not do actual rx if there are enough
>> packets in the queue.
>>   v2: - fixes for checks that Ciprian found.
>>
>>   Tested this patch in lxc with ping -f for both burst and queue modes.
>>
>>   Maxim.
>>
>>   platform/linux-generic/odp_packet_io.c | 74 +++++++++++++++++-----------------
>>   1 file changed, 37 insertions(+), 37 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
>> index f35193f..039003d 100644
>> --- a/platform/linux-generic/odp_packet_io.c
>> +++ b/platform/linux-generic/odp_packet_io.c
>> @@ -416,32 +416,26 @@ int pktin_enqueue(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr)
>>   odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *qentry)
>>   {
>>          odp_buffer_hdr_t *buf_hdr;
>> +       odp_buffer_t buf;
>> +       odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
>> +       odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
>> +       int pkts, i;
>>
>>          buf_hdr = queue_deq(qentry);
>> +       if (buf_hdr != NULL)
>> +               return buf_hdr;
>>
>> -       if (buf_hdr == NULL) {
>> -               odp_packet_t pkt;
>> -               odp_buffer_t buf;
>> -               odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
>> -               odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
>> -               int pkts, i, j;
>> -
>> -               pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl,
>> -                                     QUEUE_MULTI_MAX);
>> -
>> -               if (pkts > 0) {
>> -                       pkt = pkt_tbl[0];
>> -                       buf = odp_packet_to_buffer(pkt);
>> -                       buf_hdr = odp_buf_to_hdr(buf);
>> +       pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl, QUEUE_MULTI_MAX);
>> +       if (pkts <= 0)
>> +               return NULL;
>>
>> -                       for (i = 1, j = 0; i < pkts; ++i) {
>> -                               buf = odp_packet_to_buffer(pkt_tbl[i]);
>> -                               tmp_hdr_tbl[j++] = odp_buf_to_hdr(buf);
>> -                       }
>> -                       queue_enq_multi(qentry, tmp_hdr_tbl, j);
>> -               }
>> +       for (i = 0; i < pkts; ++i) {
>> +               buf = odp_packet_to_buffer(pkt_tbl[i]);
>> +               tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
>>          }
>>
>> +       queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
>> +       buf_hdr = tmp_hdr_tbl[0];
>>          return buf_hdr;
>>   }
>>
>> @@ -454,25 +448,31 @@ int pktin_enq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num)
>>   int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num)
>>   {
>>          int nbr;
>> +       odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
>> +       odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
>> +       odp_buffer_t buf;
>> +       int pkts, i;
>>
>>          nbr = queue_deq_multi(qentry, buf_hdr, num);
>> -
>> -       if (nbr < num) {
>> -               odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
>> -               odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
>> -               odp_buffer_t buf;
>> -               int pkts, i;
>> -
>> -               pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl,
>> -                                     QUEUE_MULTI_MAX);
>> -               if (pkts > 0) {
>> -                       for (i = 0; i < pkts; ++i) {
>> -                               buf = odp_packet_to_buffer(pkt_tbl[i]);
>> -                               tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
>> -                       }
>> -                       queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
>> -               }
>> +       if (nbr > num)
> Not necessarily needed, but using odp_unlikely may be a good idea.
>
>> +               ODP_ABORT("queue_deq_multi req: %d, returned %d\n",
>> +                       num, nbr);
>> +
>> +       /** queue already has number of requsted buffers,
>> +        *  do not do recieve in that case.
>> +        */
>> +       if (nbr == num)
>> +               return nbr;
>> +
>> +       pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl, QUEUE_MULTI_MAX);
>> +       if (pkts <= 0)
>> +               return nbr;
>> +
>> +       for (i = 0; i < pkts; ++i) {
>> +               buf = odp_packet_to_buffer(pkt_tbl[i]);
>> +               tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
>>          }
>> -
>> +       queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
>> +       buf_hdr = &tmp_hdr_tbl[0];
> Please remove this, I think you mistakenly copiedit from pktin_dequeue.

Ooops, thanks :)

>>          return nbr;
>>   }
>> --
>> 1.8.5.1.163.gd7aced9
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
index f35193f..039003d 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -416,32 +416,26 @@  int pktin_enqueue(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr)
 odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *qentry)
 {
 	odp_buffer_hdr_t *buf_hdr;
+	odp_buffer_t buf;
+	odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
+	odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
+	int pkts, i;
 
 	buf_hdr = queue_deq(qentry);
+	if (buf_hdr != NULL)
+		return buf_hdr;
 
-	if (buf_hdr == NULL) {
-		odp_packet_t pkt;
-		odp_buffer_t buf;
-		odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
-		odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
-		int pkts, i, j;
-
-		pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl,
-				      QUEUE_MULTI_MAX);
-
-		if (pkts > 0) {
-			pkt = pkt_tbl[0];
-			buf = odp_packet_to_buffer(pkt);
-			buf_hdr = odp_buf_to_hdr(buf);
+	pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl, QUEUE_MULTI_MAX);
+	if (pkts <= 0)
+		return NULL;
 
-			for (i = 1, j = 0; i < pkts; ++i) {
-				buf = odp_packet_to_buffer(pkt_tbl[i]);
-				tmp_hdr_tbl[j++] = odp_buf_to_hdr(buf);
-			}
-			queue_enq_multi(qentry, tmp_hdr_tbl, j);
-		}
+	for (i = 0; i < pkts; ++i) {
+		buf = odp_packet_to_buffer(pkt_tbl[i]);
+		tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
 	}
 
+	queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
+	buf_hdr = tmp_hdr_tbl[0];
 	return buf_hdr;
 }
 
@@ -454,25 +448,31 @@  int pktin_enq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num)
 int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num)
 {
 	int nbr;
+	odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
+	odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
+	odp_buffer_t buf;
+	int pkts, i;
 
 	nbr = queue_deq_multi(qentry, buf_hdr, num);
-
-	if (nbr < num) {
-		odp_packet_t pkt_tbl[QUEUE_MULTI_MAX];
-		odp_buffer_hdr_t *tmp_hdr_tbl[QUEUE_MULTI_MAX];
-		odp_buffer_t buf;
-		int pkts, i;
-
-		pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl,
-				      QUEUE_MULTI_MAX);
-		if (pkts > 0) {
-			for (i = 0; i < pkts; ++i) {
-				buf = odp_packet_to_buffer(pkt_tbl[i]);
-				tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
-			}
-			queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
-		}
+	if (nbr > num)
+		ODP_ABORT("queue_deq_multi req: %d, returned %d\n",
+			num, nbr);
+
+	/** queue already has number of requsted buffers,
+	 *  do not do recieve in that case.
+	 */
+	if (nbr == num)
+		return nbr;
+
+	pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl, QUEUE_MULTI_MAX);
+	if (pkts <= 0)
+		return nbr;
+
+	for (i = 0; i < pkts; ++i) {
+		buf = odp_packet_to_buffer(pkt_tbl[i]);
+		tmp_hdr_tbl[i] = odp_buf_to_hdr(buf);
 	}
-
+	queue_enq_multi(qentry, tmp_hdr_tbl, pkts);
+	buf_hdr = &tmp_hdr_tbl[0];
 	return nbr;
 }