[ewg] Re: [PATCH] call skb_orphan() after sending an SKB
Or Gerlitz
ogerlitz at voltaire.com
Wed Feb 6 00:17:35 PST 2008
> commit f17ebf3e2099257da244587f1ee33f51745f7cdb
> Author: Eli Cohen <eli at mellanox.co.il>
> Date: Tue Feb 5 11:15:46 2008 +0200
>
> Call skb_orphan() after sending an SKB
>
> This will call the destructor of the SKB (but not free the
> memory). It appears that some applications (ttcpv for example)
> are sensitive to delaying the time the SKB is freed. This commit
> fixes this problem.
Can you explain what is the difference from the socket send buffer accounting
point of view, between freeing the SKB to freeing the memory? what was the
problem with ttcpv, did it hanged? have you tested the unsig_udqp.patch with
different socket buffer sizes to make sure there's no live-lock etc? what
was the app you were using?
Also, I see that you have added a call to netif_stop_queue(), is this to
solve another problem?
Or.
>
> Signed-off-by: Eli Cohen <eli at mellanox.co.il>
>
> diff --git a/kernel_patches/fixes/ipoib_0190_unsig_udqp.patch b/kernel_patches/fixes/ipoib_0190_unsig_udqp.patch
> index b76cdab..3fbeda3 100644
> --- a/kernel_patches/fixes/ipoib_0190_unsig_udqp.patch
> +++ b/kernel_patches/fixes/ipoib_0190_unsig_udqp.patch
> @@ -10,10 +10,10 @@ UDP messages, went up from 380 mbps to 508 mbps.
>
> Signed-off-by: Eli Cohen <eli at mellanox.co.il>
> ---
> -Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib.h
> +Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib.h
> ===================================================================
> ---- ofed_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib.h
> -+++ ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib.h
> +--- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib.h 2008-02-05 11:04:35.000000000 +0200
> ++++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib.h 2008-02-05 11:05:07.000000000 +0200
> @@ -373,6 +373,7 @@ struct ipoib_dev_priv {
>
> struct ib_wc ibwc[IPOIB_NUM_WC];
> @@ -39,10 +39,10 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib.h
>
> struct ipoib_ah *ipoib_create_ah(struct net_device *dev,
> struct ib_pd *pd, struct ib_ah_attr *attr);
> -Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> ===================================================================
> ---- ofed_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> -+++ ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +--- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2008-02-05 11:04:35.000000000 +0200
> ++++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2008-02-05 11:05:44.000000000 +0200
> @@ -254,12 +254,10 @@ repost:
> "for buf %d\n", wr_id);
> }
> @@ -128,7 +128,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> }
>
> int ipoib_poll(struct napi_struct *napi, int budget)
> -@@ -361,11 +372,65 @@ void ipoib_ib_rx_completion(struct ib_cq
> +@@ -361,11 +372,68 @@ void ipoib_ib_rx_completion(struct ib_cq
> netif_rx_schedule(dev, &priv->napi);
> }
>
> @@ -168,8 +168,11 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> + ipoib_warn(priv, "failed to post zlen send\n");
> + else {
> + ++priv->tx_head;
> -+ ++priv->tx_outstanding;
> + ipoib_dbg(priv, "%s-%d: head = %d\n", __func__, __LINE__, priv->tx_head);
> ++ if (++priv->tx_outstanding == ipoib_sendq_size) {
> ++ ipoib_dbg(priv, "TX ring full, stopping kernel net queue\n");
> ++ netif_stop_queue(dev);
> ++ }
> + }
> + }
> + poll_tx(priv);
> @@ -197,7 +200,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> }
>
> static inline int post_send(struct ipoib_dev_priv *priv,
> -@@ -405,6 +470,11 @@ static inline int post_send(struct ipoib
> +@@ -405,6 +473,11 @@ static inline int post_send(struct ipoib
> } else
> priv->tx_wr.opcode = IB_WR_SEND;
>
> @@ -209,16 +212,18 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> return ib_post_send(priv->qp, &priv->tx_wr, &bad_wr);
> }
>
> -@@ -489,7 +559,7 @@ void ipoib_send(struct net_device *dev,
> +@@ -489,7 +562,9 @@ void ipoib_send(struct net_device *dev,
> }
>
> if (unlikely(priv->tx_outstanding > MAX_SEND_CQE + 1))
> - poll_tx(priv, 0);
> + poll_tx(priv);
> ++
> ++ skb_orphan(skb);
>
> return;
>
> -@@ -530,6 +600,32 @@ void ipoib_reap_ah(struct work_struct *w
> +@@ -530,6 +605,32 @@ void ipoib_reap_ah(struct work_struct *w
> round_jiffies_relative(HZ));
> }
>
> @@ -251,7 +256,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> int ipoib_ib_dev_open(struct net_device *dev)
> {
> struct ipoib_dev_priv *priv = netdev_priv(dev);
> -@@ -542,9 +638,17 @@ int ipoib_ib_dev_open(struct net_device
> +@@ -542,9 +643,17 @@ int ipoib_ib_dev_open(struct net_device
> }
> set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
>
> @@ -269,7 +274,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> return -1;
> }
>
> -@@ -566,6 +670,11 @@ int ipoib_ib_dev_open(struct net_device
> +@@ -566,6 +675,11 @@ int ipoib_ib_dev_open(struct net_device
> queue_delayed_work(ipoib_workqueue, &priv->ah_reap_task,
> round_jiffies_relative(HZ));
>
> @@ -281,7 +286,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> set_bit(IPOIB_FLAG_INITIALIZED, &priv->flags);
>
> return 0;
> -@@ -662,7 +771,7 @@ void ipoib_drain_cq(struct net_device *d
> +@@ -662,7 +776,7 @@ void ipoib_drain_cq(struct net_device *d
> if (priv->ibwc[i].wr_id & IPOIB_OP_CM)
> ipoib_cm_handle_tx_wc(dev, priv->ibwc + i);
> else
> @@ -290,7 +295,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> }
> }
> } while (n == IPOIB_NUM_WC);
> -@@ -673,12 +782,14 @@ int ipoib_ib_dev_stop(struct net_device
> +@@ -673,12 +787,14 @@ int ipoib_ib_dev_stop(struct net_device
> struct ipoib_dev_priv *priv = netdev_priv(dev);
> struct ib_qp_attr qp_attr;
> unsigned long begin;
> @@ -306,7 +311,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
>
> /*
> * Move our QP to the error state and then reinitialize in
> -@@ -700,32 +811,30 @@ int ipoib_ib_dev_stop(struct net_device
> +@@ -700,32 +816,30 @@ int ipoib_ib_dev_stop(struct net_device
> * assume the HW is wedged and just free up
> * all our pending work requests.
> */
> @@ -354,7 +359,7 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> ipoib_drain_cq(dev);
>
> msleep(1);
> -@@ -734,6 +843,7 @@ int ipoib_ib_dev_stop(struct net_device
> +@@ -734,6 +848,7 @@ int ipoib_ib_dev_stop(struct net_device
> ipoib_dbg(priv, "All sends and receives done.\n");
>
> timeout:
> @@ -362,10 +367,10 @@ Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> qp_attr.qp_state = IB_QPS_RESET;
> if (ib_modify_qp(priv->qp, &qp_attr, IB_QP_STATE))
> ipoib_warn(priv, "Failed to modify QP to RESET state\n");
> -Index: ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> +Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> ===================================================================
> ---- ofed_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> -+++ ofed_kernel/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> +--- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib_verbs.c 2008-02-05 11:04:35.000000000 +0200
> ++++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_verbs.c 2008-02-05 11:05:07.000000000 +0200
> @@ -153,7 +153,7 @@ int ipoib_transport_dev_init(struct net_
> .max_send_sge = dev->features & NETIF_F_SG ? MAX_SKB_FRAGS + 1 : 1,
> .max_recv_sge = 1
>
More information about the ewg
mailing list