| Message ID | 1415197137-17342-1-git-send-email-stuart.haslam@arm.com |
|---|---|
| State | New |
| Headers | show |
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.haslam@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; > }
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); + } + + 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; }
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.haslam@arm.com> --- platform/linux-generic/odp_packet_io.c | 66 +++++++++++++++------------------- 1 file changed, 28 insertions(+), 38 deletions(-)