On 11/05/2014 05:18 PM, Stuart Haslam wrote:
Running the odp_ipsec application built with IPSEC_POLLED_QUEUES
enabled triggers a crash (on linux-generic).

pktin_dequeue can end up calling queue_enq_multi with the num
parameter set to 0, num-1 is then used as an array index causing a
SEGV. There's also a minor issue with pktin_deq_multi returning 0
buffers immediately after it had received some from the pktio, so
fix both and refactor a bit at the same time.

Signed-off-by: Stuart Haslam <stuart.has...@arm.com>
---
  platform/linux-generic/odp_packet_io.c | 66 +++++++++++++++-------------------
  1 file changed, 28 insertions(+), 38 deletions(-)

diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index f35193f..560740f 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -413,34 +413,38 @@ int pktin_enqueue(queue_entry_t *qentry, odp_buffer_hdr_t 
*buf_hdr)
        return queue_enq(qentry, buf_hdr);
  }
+static inline int pktin_recv_multi(queue_entry_t *qentry,
+                                  odp_buffer_hdr_t *buf_hdr[], int 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, j, nbr = 0;
+
+       pkts = odp_pktio_recv(qentry->s.pktin, pkt_tbl, QUEUE_MULTI_MAX);
+       if (pkts > 0) {
+               for (i = 0, j = 0; i < pkts; ++i) {
+                       buf = odp_packet_to_buffer(pkt_tbl[i]);
+                       if (i < num)
+                               buf_hdr[nbr++] = odp_buf_to_hdr(buf);
+                       else
+                               tmp_hdr_tbl[j++] = odp_buf_to_hdr(buf);
+               }
+               if (j > 0)
+                       queue_enq_multi(qentry, tmp_hdr_tbl, j);

I think it's bad idea to do that. Looks you ask for MAX packets if you receive more then requested (num)
then you push packets back to the queue.

So 2 problems:
1. Reordering;
2. At that time other thread can do read / queue, but you already got MAX packets in one thread.

so I think you need to calculate requested packets as min(num, QUEUE_MULTI_MAX);

BR,
Maxim.
+       }
+
+       return nbr;
+}
+
  odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *qentry)
  {
        odp_buffer_hdr_t *buf_hdr;
buf_hdr = queue_deq(qentry); - 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);
-
-                       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);
-               }
-       }
+       if (buf_hdr == NULL)
+               pktin_recv_multi(qentry, &buf_hdr, 1);
return buf_hdr;
  }
@@ -457,22 +461,8 @@ int pktin_deq_multi(queue_entry_t *qentry, 
odp_buffer_hdr_t *buf_hdr[], int num)
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)
+               nbr += pktin_recv_multi(qentry, &buf_hdr[nbr], num-nbr);
return nbr;
  }


_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to