Skip to content

Commit

Permalink
rxe: fix completion queue consumer index overrun
Browse files Browse the repository at this point in the history
Fix a bug in the CQ polling sequence where the consumer index can
incorrectly advance beyond available completions.

Consider this polling sequence:

```
    ibv_start_poll();
    ibv_next_poll();
    ibv_end_poll();
    ibv_start_poll();
```

With one completion in the queue, the indices would be:

1. After `ibv_start_poll()` (reading first CQE):
      P
 ┌──┬─┴┬──┬──┬──┐
 └─┬┴──┴──┴──┴──┘
   C

2. After `ibv_next_poll()` (returns `ENOENT`):
      P
 ┌──┬─┴┬──┬──┬──┐
 └──┴─┬┴──┴──┴──┘
      C

3. After `ibv_end_poll()`:
      P
 ┌──┬─┴┬──┬──┬──┐
 └──┴──┴─┬┴──┴──┘
         C

The issue occurs because `ibv_end_poll()` advances the consumer index
even after `ibv_next_poll()` returns `ENOENT`. This causes the consumer
index to move beyond the producer index, leading to:

- False indication of available completions
- Reading of uninitialized completion entries

Fix this by checking for available completions before advancing the
consumer index in `ibv_next_poll()`.

Note: According to the man page, `ibv_end_poll()` must be called even
when `ibv_next_poll()` returns `ENOENT`, but consumer index should only
be advanced once in this case.

Signed-off-by: Luke Yue <lukedyue@gmail.com>
  • Loading branch information
dragonJACson committed Nov 7, 2024
1 parent 9f40685 commit b78b414
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions providers/rxe/rxe.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,17 @@ static int cq_next_poll(struct ibv_cq_ex *current)
{
struct rxe_cq *cq = container_of(current, struct rxe_cq, vcq.cq_ex);

advance_cq_cur_index(cq);
struct rxe_queue_buf *q = cq->queue;
uint32_t next_index = (cq->cur_index + 1) & q->index_mask;

if (check_cq_queue_empty(cq)) {
if (next_index == load_producer_index(q)) {
store_consumer_index(cq->queue, cq->cur_index);
pthread_spin_unlock(&cq->lock);
errno = ENOENT;
return errno;
}

cq->cur_index = next_index;
cq->wc = addr_from_index(cq->queue, cq->cur_index);
cq->vcq.cq_ex.status = cq->wc->status;
cq->vcq.cq_ex.wr_id = cq->wc->wr_id;
Expand Down

0 comments on commit b78b414

Please # to comment.