> Roland, The following code in neigh_add_path looks strange to me:
>
> neigh->ah = NULL;
> if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE)
> {
> __skb_queue_tail(&neigh->queue, skb);
> } else {
> ++priv->stats.tx_dropped;
> dev_kfree_skb_any(skb);
> }
>
> if (!path->query && path_rec_start(dev, path))
> goto err_list;
>
> It seems that if the queue is full, and path_rec_start fails, we will
> double-free the skb. Would it make sense to simply goto err_list if the queue
> is full?
Yes, that looks like an obvious mistake. I checked this in:
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c
b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 1633aad..00342d5 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -519,12 +519,10 @@ static void neigh_add_path(struct sk_buf
be32_to_cpup((__be32 *) skb->dst->neighbour->ha));
} else {
neigh->ah = NULL;
- if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
+ if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE)
__skb_queue_tail(&neigh->queue, skb);
- } else {
- ++priv->stats.tx_dropped;
- dev_kfree_skb_any(skb);
- }
+ else
+ goto err_list;
if (!path->query && path_rec_start(dev, path))
goto err;
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
