diff options
author | Aaron Knister <aaron.s.knister@nasa.gov> | 2018-08-24 08:42:46 -0400 |
---|---|---|
committer | Jason Gunthorpe <jgg@mellanox.com> | 2018-09-05 17:32:06 -0400 |
commit | 816e846c2eb9129a3e0afa5f920c8bbc71efecaa (patch) | |
tree | 0db74bb6e3c843640b8c161d6716a7adcf8f0493 | |
parent | 308aa2b8f7b7db3332a7d41099fd37851fb793b2 (diff) |
IB/ipoib: Avoid a race condition between start_xmit and cm_rep_handler
Inside of start_xmit() the call to check if the connection is up and the
queueing of the packets for later transmission is not atomic which leaves
a window where cm_rep_handler can run, set the connection up, dequeue
pending packets and leave the subsequently queued packets by start_xmit()
sitting on neigh->queue until they're dropped when the connection is torn
down. This only applies to connected mode. These dropped packets can
really upset TCP, for example, and cause multi-minute delays in
transmission for open connections.
Here's the code in start_xmit where we check to see if the connection is
up:
if (ipoib_cm_get(neigh)) {
if (ipoib_cm_up(neigh)) {
ipoib_cm_send(dev, skb, ipoib_cm_get(neigh));
goto unref;
}
}
The race occurs if cm_rep_handler execution occurs after the above
connection check (specifically if it gets to the point where it acquires
priv->lock to dequeue pending skb's) but before the below code snippet in
start_xmit where packets are queued.
if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
push_pseudo_header(skb, phdr->hwaddr);
spin_lock_irqsave(&priv->lock, flags);
__skb_queue_tail(&neigh->queue, skb);
spin_unlock_irqrestore(&priv->lock, flags);
} else {
++dev->stats.tx_dropped;
dev_kfree_skb_any(skb);
}
The patch acquires the netif tx lock in cm_rep_handler for the section
where it sets the connection up and dequeues and retransmits deferred
skb's.
Fixes: 839fcaba355a ("IPoIB: Connected mode experimental support")
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Knister <aaron.s.knister@nasa.gov>
Tested-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
-rw-r--r-- | drivers/infiniband/ulp/ipoib/ipoib_cm.c | 2 |
1 files changed, 2 insertions, 0 deletions
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index ea01b8dd2be6..3d5424f335cb 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c | |||
@@ -1027,12 +1027,14 @@ static int ipoib_cm_rep_handler(struct ib_cm_id *cm_id, | |||
1027 | 1027 | ||
1028 | skb_queue_head_init(&skqueue); | 1028 | skb_queue_head_init(&skqueue); |
1029 | 1029 | ||
1030 | netif_tx_lock_bh(p->dev); | ||
1030 | spin_lock_irq(&priv->lock); | 1031 | spin_lock_irq(&priv->lock); |
1031 | set_bit(IPOIB_FLAG_OPER_UP, &p->flags); | 1032 | set_bit(IPOIB_FLAG_OPER_UP, &p->flags); |
1032 | if (p->neigh) | 1033 | if (p->neigh) |
1033 | while ((skb = __skb_dequeue(&p->neigh->queue))) | 1034 | while ((skb = __skb_dequeue(&p->neigh->queue))) |
1034 | __skb_queue_tail(&skqueue, skb); | 1035 | __skb_queue_tail(&skqueue, skb); |
1035 | spin_unlock_irq(&priv->lock); | 1036 | spin_unlock_irq(&priv->lock); |
1037 | netif_tx_unlock_bh(p->dev); | ||
1036 | 1038 | ||
1037 | while ((skb = __skb_dequeue(&skqueue))) { | 1039 | while ((skb = __skb_dequeue(&skqueue))) { |
1038 | skb->dev = p->dev; | 1040 | skb->dev = p->dev; |