IPoIB: Free ipoib neigh on path record failure so path rec queries are retried
From: Roland Dreier <[email protected]>
If IPoIB fails to look up a path record (eg if it tries during an SM failover when one SM is dead but the new one hasn't taken over yet), the driver ends up with a neighbour structure but no address handle (AH). There's no mechanism to recover from this: any further packets sent to this destination will be silently dumped in ipoib_start_xmit().
Fix this by freeing the neighbour structures when a path rec query fails, so that the next packet queued to be sent will trigger a new path record query.
---
We actually hit this while testing SM failover. Let me know if there are any objections to merging this for 3.9.
drivers/infiniband/ulp/ipoib/ipoib_main.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/infiniband/ulp/ipoib/ipoib_main.c +3 -0
@@ -510,6 +510,9 @@ static void path_rec_completion(int status,spin_unlock_irqrestore (&priv->lock, flags); +if (IS_ERR_OR_NULL (ah)) +ipoib_del_neighs_by_gid (dev, path->pathrec.dgid.raw); +if (old_ah)ipoib_put_ah (old_ah);
Signature · 6 lines
1.8.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 1361814409-6704-1-git-send-email-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Quoted 5 lines · Roland Dreier
If IPoIB fails to look up a path record (eg if it tries during an SM failover when one SM is dead but the new one hasn't taken over yet), the driver ends up with a neighbour structure but no address handle (AH). There's no mechanism to recover from this: any further packets sent to this destination will be silently dumped in
ipoib_start_xmit().
Looking on the flow of sending ARP probes, I see that in unicast_arp_send, if there's no AH for the path, a path query is initiated,
if (path->ah) {
ipoib_dbg(priv, "Send unicast ARP to %04x\n",be16_to_cpu(path->pathrec.dlid));
spin_unlock_irqrestore(&priv->lock, flags);
ipoib_send(dev, skb, path->ah, IPOIB_QPN(cb->hwaddr));
return;
} else if ((path->query || !path_rec_start(dev, path)) &&
skb_queue_len(&path->queue) <
IPOIB_MAX_PATH_REC_QUEUE) {
__skb_queue_tail(&path->queue, skb);
} else {
++dev->stats.tx_dropped;
dev_kfree_skb_any(skb);
}so eventually the traffic should resume once the ND state machine sends a probe, agree? did you only wanted to make that faster?
Or.
Quoted 1 line
Fix this by freeing the neighbour structures when a path rec query fails, so that the next packet queued to be sent will trigger a new path record query.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 512C8F4F.9000606-VPRAkNaXOzVWk0Htik3J/[email protected]
Quoted 19 lines · Or Gerlitz
Looking on the flow of sending ARP probes, I see that in
unicast_arp_send, if there's no AH for the path, a path query is initiated,if (
path->ah) {
ipoib_dbg(priv, "Send unicast ARP to %04x\n",be16_to_cpu(path->pathrec.dlid));spin_unlock_irqrestore(&priv->lock, flags); ipoib_send(dev, skb, path->ah, IPOIB_QPN(cb->hwaddr)); return; } else if ((path->query || !path_rec_start(dev, path)) && skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) { __skb_queue_tail(&path->queue, skb); } else { ++dev->stats.tx_dropped; dev_kfree_skb_any(skb); }
Quoted 2 lines
so eventually the traffic should resume once the ND state machine sends a probe, agree? did you only wanted to make that faster?
Well, look at ipoib_start_xmit():
/* unicast, arrange "switch" according to probability */
switch (header->proto) {
case htons(ETH_P_IP):
case htons(ETH_P_IPV6):
neigh = ipoib_neigh_get(dev, cb->hwaddr);
if (unlikely(!neigh)) {
neigh_add_path(skb, cb->hwaddr, dev);
return NETDEV_TX_OK;
}
break;
case htons(ETH_P_ARP):
case htons(ETH_P_RARP):
/* for unicast ARP and RARP should always perform path find */
unicast_arp_send(skb, dev, cb);
return NETDEV_TX_OK;etc. So although you mention ND, we only call unicast_arp_send() for IPv4 ARP. And in our case we were actually using IPv6, which follows the main flow with neighbours even for neighbour discovery packets. So there's no way to ever recover from having the failed path record.
In fact I bet this is why the bug has been there as long as it has
been: almost no one is using IPv6 on IPoIB seriously, and IPv4 should work OK as you point out.
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAG4TOxMyTEMKLUF1+YLej7Txj4P5MLZ49cVqAhQP4CW8qi0AUA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
On 26.02.2013 17:55, Roland Dreier wrote:
[...]
Quoted 3 lines
In fact I bet this is why the bug has been there as long as it has
been: almost no one is using IPv6 on IPoIB seriously, and IPv4 should work OK as you point out.
Thanks a lot,.... Unfortunately, we are using IPoIB with IPv6 in production for the inter-VM and the internet gateway traffic. We had an issue last Friday were one of our gateway machines wasn't reachable anymore and pings didn't come through. We had to reload the ib_ipoib modules of nearly every server connected to that gateway. I'm so glad that we use SRP - all SRP connections survived.
I don't have the time to care for IPoIB as well at ProfitBricks so I will encourage our networking teams to get their hands on IPoIB and linux-rdma communication.
How to verify if we've hit this bug? How to reproduce this bug?
Cheers,
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html