netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] Low latency Socket support
@ 2013-06-17 10:58 Amir Vadai
  2013-06-17 10:58 ` [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support Amir Vadai
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Amir Vadai @ 2013-06-17 10:58 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, eliezer.tamir, Or Gerlitz, Amir Vadai

Hi Dave,

Please pull those 2 patches, which add support in Low Latency Socket (LLS) to
mlx4_en.

Thanks,
Amir

Amir Vadai (2):
  net/mlx4_en: Add Low Latency Socket (LLS) support
  net/mlx4_en: Low Latency recv statistics

 drivers/net/ethernet/mellanox/mlx4/en_cq.c      |   2 +
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |  20 +++-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c  |  49 ++++++++-
 drivers/net/ethernet/mellanox/mlx4/en_rx.c      |   8 ++
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    | 133 ++++++++++++++++++++++++
 5 files changed, 209 insertions(+), 3 deletions(-)

-- 
1.8.3.251.g1462b67

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support
  2013-06-17 10:58 [PATCH net-next 0/2] Low latency Socket support Amir Vadai
@ 2013-06-17 10:58 ` Amir Vadai
  2013-06-17 11:37   ` Eric Dumazet
  2013-06-17 10:58 ` [PATCH net-next 2/2] net/mlx4_en: Low Latency recv statistics Amir Vadai
  2013-06-17 11:23 ` [PATCH net-next 0/2] Low latency Socket support Eliezer Tamir
  2 siblings, 1 reply; 8+ messages in thread
From: Amir Vadai @ 2013-06-17 10:58 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, eliezer.tamir, Or Gerlitz, Amir Vadai

Add basic support for LLS.

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_cq.c     |   2 +
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |  49 +++++++++-
 drivers/net/ethernet/mellanox/mlx4/en_rx.c     |   8 ++
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   | 124 +++++++++++++++++++++++++
 4 files changed, 181 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index 1e6c594..5e47efd 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -139,6 +139,7 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
 
 	if (!cq->is_tx) {
 		netif_napi_add(cq->dev, &cq->napi, mlx4_en_poll_rx_cq, 64);
+		napi_hash_add(&cq->napi);
 		napi_enable(&cq->napi);
 	}
 
@@ -162,6 +163,7 @@ void mlx4_en_deactivate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq)
 {
 	if (!cq->is_tx) {
 		napi_disable(&cq->napi);
+		napi_hash_del(&cq->napi);
 		netif_napi_del(&cq->napi);
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 89c47ea..fd50511 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -38,6 +38,7 @@
 #include <linux/slab.h>
 #include <linux/hash.h>
 #include <net/ip.h>
+#include <net/ll_poll.h>
 
 #include <linux/mlx4/driver.h>
 #include <linux/mlx4/device.h>
@@ -67,6 +68,36 @@ int mlx4_en_setup_tc(struct net_device *dev, u8 up)
 	return 0;
 }
 
+#ifdef CONFIG_NET_LL_RX_POLL
+/* must be called with local_bh_disable()d */
+static int mlx4_en_low_latency_recv(struct napi_struct *napi)
+{
+	struct mlx4_en_cq *cq = container_of(napi, struct mlx4_en_cq, napi);
+	struct net_device *dev = cq->dev;
+	struct mlx4_en_priv *priv = netdev_priv(dev);
+	struct mlx4_en_rx_ring *rx_ring = &priv->rx_ring[cq->ring];
+	int done;
+
+	if (!priv->port_up)
+		return LL_FLUSH_FAILED;
+
+	if (!mlx4_en_cq_lock_poll(cq))
+		return LL_FLUSH_BUSY;
+
+	done = mlx4_en_process_rx_cq(dev, cq, 4);
+#ifdef LL_EXTENDED_STATS
+	if (done)
+		rx_ring->cleaned += done;
+	else
+		rx_ring->misses++;
+#endif
+
+	mlx4_en_cq_unlock_poll(cq);
+
+	return done;
+}
+#endif	/* CONFIG_NET_LL_RX_POLL */
+
 #ifdef CONFIG_RFS_ACCEL
 
 struct mlx4_en_filter {
@@ -1445,6 +1476,8 @@ int mlx4_en_start_port(struct net_device *dev)
 	for (i = 0; i < priv->rx_ring_num; i++) {
 		cq = &priv->rx_cq[i];
 
+		mlx4_en_cq_init_lock(cq);
+
 		err = mlx4_en_activate_cq(priv, cq, i);
 		if (err) {
 			en_err(priv, "Failed activating Rx CQ\n");
@@ -1694,10 +1727,19 @@ void mlx4_en_stop_port(struct net_device *dev, int detach)
 
 	/* Free RX Rings */
 	for (i = 0; i < priv->rx_ring_num; i++) {
+		struct mlx4_en_cq *cq = &priv->rx_cq[i];
+
+		local_bh_disable();
+		while (!mlx4_en_cq_lock_napi(cq)) {
+			pr_info("CQ %d locked\n", i);
+			mdelay(1);
+		}
+		local_bh_enable();
+
 		mlx4_en_deactivate_rx_ring(priv, &priv->rx_ring[i]);
-		while (test_bit(NAPI_STATE_SCHED, &priv->rx_cq[i].napi.state))
+		while (test_bit(NAPI_STATE_SCHED, &cq->napi.state))
 			msleep(1);
-		mlx4_en_deactivate_cq(priv, &priv->rx_cq[i]);
+		mlx4_en_deactivate_cq(priv, cq);
 	}
 
 	/* close port*/
@@ -2083,6 +2125,9 @@ static const struct net_device_ops mlx4_netdev_ops = {
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	= mlx4_en_filter_rfs,
 #endif
+#ifdef CONFIG_NET_LL_RX_POLL
+	.ndo_ll_poll		= mlx4_en_low_latency_recv,
+#endif
 };
 
 static const struct net_device_ops mlx4_netdev_ops_master = {
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 02aee1e..15b5980 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -31,6 +31,7 @@
  *
  */
 
+#include <net/ll_poll.h>
 #include <linux/mlx4/cq.h>
 #include <linux/slab.h>
 #include <linux/mlx4/qp.h>
@@ -737,6 +738,8 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 					       timestamp);
 		}
 
+		skb_mark_ll(skb, &cq->napi);
+
 		/* Push it up the stack */
 		netif_receive_skb(skb);
 
@@ -781,8 +784,13 @@ int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget)
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	int done;
 
+	if (!mlx4_en_cq_lock_napi(cq))
+		return budget;
+
 	done = mlx4_en_process_rx_cq(dev, cq, budget);
 
+	mlx4_en_cq_unlock_napi(cq);
+
 	/* If we used up all the quota - we're probably not done yet... */
 	if (done == budget)
 		INC_PERF_COUNTER(priv->pstats.napi_quota);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index b1f51c1..ab3dad5 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -310,6 +310,19 @@ struct mlx4_en_cq {
 	u16 moder_cnt;
 	struct mlx4_cqe *buf;
 #define MLX4_EN_OPCODE_ERROR	0x1e
+
+#ifdef CONFIG_NET_LL_RX_POLL
+	unsigned int state;
+#define MLX4_EN_CQ_STATEIDLE        0
+#define MLX4_EN_CQ_STATENAPI     1    /* NAPI owns this CQ */
+#define MLX4_EN_CQ_STATEPOLL     2    /* poll owns this CQ */
+#define MLX4_CQ_LOCKED (MLX4_EN_CQ_STATENAPI | MLX4_EN_CQ_STATEPOLL)
+#define MLX4_EN_CQ_STATENAPI_YIELD  4    /* NAPI yielded this CQ */
+#define MLX4_EN_CQ_STATEPOLL_YIELD  8    /* poll yielded this CQ */
+#define CQ_YIELD (MLX4_EN_CQ_STATENAPI_YIELD | MLX4_EN_CQ_STATEPOLL_YIELD)
+#define CQ_USER_PEND (MLX4_EN_CQ_STATEPOLL | MLX4_EN_CQ_STATEPOLL_YIELD)
+	spinlock_t poll_lock; /* protects from LLS/napi conflicts */
+#endif  /* CONFIG_NET_LL_RX_POLL */
 };
 
 struct mlx4_en_port_profile {
@@ -562,6 +575,117 @@ struct mlx4_mac_entry {
 	struct rcu_head rcu;
 };
 
+#ifdef CONFIG_NET_LL_RX_POLL
+static inline void mlx4_en_cq_init_lock(struct mlx4_en_cq *cq)
+{
+	spin_lock_init(&cq->poll_lock);
+	cq->state = MLX4_EN_CQ_STATEIDLE;
+}
+
+/* called from the device poll rutine to get ownership of a cq */
+static inline int mlx4_en_cq_lock_napi(struct mlx4_en_cq *cq)
+{
+	int rc = true;
+	spin_lock(&cq->poll_lock);
+	if (cq->state & MLX4_CQ_LOCKED) {
+		WARN_ON(cq->state & MLX4_EN_CQ_STATENAPI);
+		cq->state |= MLX4_EN_CQ_STATENAPI_YIELD;
+		rc = false;
+	} else
+		/* we don't care if someone yielded */
+		cq->state = MLX4_EN_CQ_STATENAPI;
+	spin_unlock(&cq->poll_lock);
+	return rc;
+}
+
+/* returns true is someone tried to get the cq while napi had it */
+static inline int mlx4_en_cq_unlock_napi(struct mlx4_en_cq *cq)
+{
+	int rc = false;
+	spin_lock(&cq->poll_lock);
+	WARN_ON(cq->state & (MLX4_EN_CQ_STATEPOLL |
+			       MLX4_EN_CQ_STATENAPI_YIELD));
+
+	if (cq->state & MLX4_EN_CQ_STATEPOLL_YIELD)
+		rc = true;
+	cq->state = MLX4_EN_CQ_STATEIDLE;
+	spin_unlock(&cq->poll_lock);
+	return rc;
+}
+
+/* called from mlx4_en_low_latency_poll() */
+static inline int mlx4_en_cq_lock_poll(struct mlx4_en_cq *cq)
+{
+	int rc = true;
+	spin_lock_bh(&cq->poll_lock);
+	if ((cq->state & MLX4_CQ_LOCKED)) {
+		struct net_device *dev = cq->dev;
+		struct mlx4_en_priv *priv = netdev_priv(dev);
+		struct mlx4_en_rx_ring *rx_ring = &priv->rx_ring[cq->ring];
+
+		cq->state |= MLX4_EN_CQ_STATEPOLL_YIELD;
+		rc = false;
+#ifdef LL_EXTENDED_STATS
+		rx_ring->yields++;
+#endif
+	} else
+		/* preserve yield marks */
+		cq->state |= MLX4_EN_CQ_STATEPOLL;
+	spin_unlock_bh(&cq->poll_lock);
+	return rc;
+}
+
+/* returns true if someone tried to get the cq while it was locked */
+static inline int mlx4_en_cq_unlock_poll(struct mlx4_en_cq *cq)
+{
+	int rc = false;
+	spin_lock_bh(&cq->poll_lock);
+	WARN_ON(cq->state & (MLX4_EN_CQ_STATENAPI));
+
+	if (cq->state & MLX4_EN_CQ_STATEPOLL_YIELD)
+		rc = true;
+	cq->state = MLX4_EN_CQ_STATEIDLE;
+	spin_unlock_bh(&cq->poll_lock);
+	return rc;
+}
+
+/* true if a socket is polling, even if it did not get the lock */
+static inline int cq_ll_polling(struct mlx4_en_cq *cq)
+{
+	WARN_ON(!(cq->state & MLX4_CQ_LOCKED));
+	return cq->state & CQ_USER_PEND;
+}
+#else
+static inline void mlx4_en_cq_init_lock(struct mlx4_en_cq *cq)
+{
+}
+
+static inline int mlx4_en_cq_lock_napi(struct mlx4_en_cq *cq)
+{
+	return true;
+}
+
+static inline int mlx4_en_cq_unlock_napi(struct mlx4_en_cq *cq)
+{
+	return false;
+}
+
+static inline int mlx4_en_cq_lock_poll(struct mlx4_en_cq *cq)
+{
+	return false;
+}
+
+static inline int mlx4_en_cq_unlock_poll(struct mlx4_en_cq *cq)
+{
+	return false;
+}
+
+static inline int mlx4_en_cq_ll_polling(struct mlx4_en_cq *cq)
+{
+	return false;
+}
+#endif /* CONFIG_NET_LL_RX_POLL */
+
 #define MLX4_EN_WOL_DO_MODIFY (1ULL << 63)
 
 void mlx4_en_update_loopback_state(struct net_device *dev,
-- 
1.8.3.251.g1462b67

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 2/2] net/mlx4_en: Low Latency recv statistics
  2013-06-17 10:58 [PATCH net-next 0/2] Low latency Socket support Amir Vadai
  2013-06-17 10:58 ` [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support Amir Vadai
@ 2013-06-17 10:58 ` Amir Vadai
  2013-06-18  0:01   ` David Miller
  2013-06-17 11:23 ` [PATCH net-next 0/2] Low latency Socket support Eliezer Tamir
  2 siblings, 1 reply; 8+ messages in thread
From: Amir Vadai @ 2013-06-17 10:58 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, eliezer.tamir, Or Gerlitz, Amir Vadai

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 20 +++++++++++++++++++-
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |  9 +++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index c9e6b62..a9e6987 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -222,7 +222,12 @@ static int mlx4_en_get_sset_count(struct net_device *dev, int sset)
 	switch (sset) {
 	case ETH_SS_STATS:
 		return (priv->stats_bitmap ? bit_count : NUM_ALL_STATS) +
-			(priv->tx_ring_num + priv->rx_ring_num) * 2;
+			(priv->tx_ring_num * 2) +
+#ifdef LL_EXTENDED_STATS
+			(priv->rx_ring_num * 5);
+#else
+			(priv->rx_ring_num * 2);
+#endif
 	case ETH_SS_TEST:
 		return MLX4_EN_NUM_SELF_TEST - !(priv->mdev->dev->caps.flags
 					& MLX4_DEV_CAP_FLAG_UC_LOOPBACK) * 2;
@@ -271,6 +276,11 @@ static void mlx4_en_get_ethtool_stats(struct net_device *dev,
 	for (i = 0; i < priv->rx_ring_num; i++) {
 		data[index++] = priv->rx_ring[i].packets;
 		data[index++] = priv->rx_ring[i].bytes;
+#ifdef LL_EXTENDED_STATS
+		data[index++] = priv->rx_ring[i].yields;
+		data[index++] = priv->rx_ring[i].misses;
+		data[index++] = priv->rx_ring[i].cleaned;
+#endif
 	}
 	spin_unlock_bh(&priv->stats_lock);
 
@@ -334,6 +344,14 @@ static void mlx4_en_get_strings(struct net_device *dev,
 				"rx%d_packets", i);
 			sprintf(data + (index++) * ETH_GSTRING_LEN,
 				"rx%d_bytes", i);
+#ifdef LL_EXTENDED_STATS
+			sprintf(data + (index++) * ETH_GSTRING_LEN,
+				"rx%d_napi_yield", i);
+			sprintf(data + (index++) * ETH_GSTRING_LEN,
+				"rx%d_misses", i);
+			sprintf(data + (index++) * ETH_GSTRING_LEN,
+				"rx%d_cleaned", i);
+#endif
 		}
 		break;
 	}
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index ab3dad5..0127b47 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -84,6 +84,10 @@
 #define MLX4_EN_FILTER_HASH_SHIFT 4
 #define MLX4_EN_FILTER_EXPIRY_QUOTA 60
 
+#ifdef CONFIG_NET_LL_RX_POLL
+#define LL_EXTENDED_STATS
+#endif
+
 /* Typical TSO descriptor with 16 gather entries is 352 bytes... */
 #define MAX_DESC_SIZE		512
 #define MAX_DESC_TXBBS		(MAX_DESC_SIZE / TXBB_SIZE)
@@ -290,6 +294,11 @@ struct mlx4_en_rx_ring {
 	void *rx_info;
 	unsigned long bytes;
 	unsigned long packets;
+#ifdef LL_EXTENDED_STATS
+	unsigned long yields;
+	unsigned long misses;
+	unsigned long cleaned;
+#endif
 	unsigned long csum_ok;
 	unsigned long csum_none;
 	int hwtstamp_rx_filter;
-- 
1.8.3.251.g1462b67

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] Low latency Socket support
  2013-06-17 10:58 [PATCH net-next 0/2] Low latency Socket support Amir Vadai
  2013-06-17 10:58 ` [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support Amir Vadai
  2013-06-17 10:58 ` [PATCH net-next 2/2] net/mlx4_en: Low Latency recv statistics Amir Vadai
@ 2013-06-17 11:23 ` Eliezer Tamir
  2013-06-17 11:31   ` Amir Vadai
  2 siblings, 1 reply; 8+ messages in thread
From: Eliezer Tamir @ 2013-06-17 11:23 UTC (permalink / raw)
  To: Amir Vadai; +Cc: David S. Miller, netdev, Or Gerlitz

On 17/06/2013 13:58, Amir Vadai wrote:
> Hi Dave,
>
> Please pull those 2 patches, which add support in Low Latency Socket (LLS) to
> mlx4_en.
>
> Thanks,
> Amir

I would really like to hear some of your performance numbers.

-Eliezer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] Low latency Socket support
  2013-06-17 11:23 ` [PATCH net-next 0/2] Low latency Socket support Eliezer Tamir
@ 2013-06-17 11:31   ` Amir Vadai
  0 siblings, 0 replies; 8+ messages in thread
From: Amir Vadai @ 2013-06-17 11:31 UTC (permalink / raw)
  To: Eliezer Tamir
  Cc: David S. Miller, netdev, Or Gerlitz, Amir Ancel, Yevgeny Petrilin,
	Yaniv Saar

On 17/06/2013 14:23, Eliezer Tamir wrote:
> On 17/06/2013 13:58, Amir Vadai wrote:
>> Hi Dave,
>>
>> Please pull those 2 patches, which add support in Low Latency Socket
>> (LLS) to
>> mlx4_en.
>>
>> Thanks,
>> Amir
> 
> I would really like to hear some of your performance numbers.
> 
> -Eliezer

I will see what numbers I can share.

In general latency of both TCP/UDP, recvmsg only, single stream,
improved in ~50% and no bad impact on BW.

We're trying to find a solution to the epoll issue, and will share our
findings with netdev as soon as we have ones.

Amir

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support
  2013-06-17 10:58 ` [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support Amir Vadai
@ 2013-06-17 11:37   ` Eric Dumazet
  2013-06-17 12:43     ` Amir Vadai
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2013-06-17 11:37 UTC (permalink / raw)
  To: Amir Vadai; +Cc: David S. Miller, netdev, eliezer.tamir, Or Gerlitz

On Mon, 2013-06-17 at 13:58 +0300, Amir Vadai wrote:
> Add basic support for LLS.
> 
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_cq.c     |   2 +
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |  49 +++++++++-
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c     |   8 ++
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   | 124 +++++++++++++++++++++++++
>  4 files changed, 181 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> index 1e6c594..5e47efd 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> @@ -139,6 +139,7 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
>  
>  	if (!cq->is_tx) {
>  		netif_napi_add(cq->dev, &cq->napi, mlx4_en_poll_rx_cq, 64);
> +		napi_hash_add(&cq->napi);
>  		napi_enable(&cq->napi);
>  	}
>  
> @@ -162,6 +163,7 @@ void mlx4_en_deactivate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq)
>  {
>  	if (!cq->is_tx) {
>  		napi_disable(&cq->napi);
> +		napi_hash_del(&cq->napi);

Are we sure an RCU grace period is respected before deletion of memory
holding cq->napi ?

>  		netif_napi_del(&cq->napi);
>  	}
>  

...

>  
>  static const struct net_device_ops mlx4_netdev_ops_master = {
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 02aee1e..15b5980 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -31,6 +31,7 @@
>   *
>   */
>  
> +#include <net/ll_poll.h>
>  #include <linux/mlx4/cq.h>
>  #include <linux/slab.h>
>  #include <linux/mlx4/qp.h>
> @@ -737,6 +738,8 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>  					       timestamp);
>  		}
>  
> +		skb_mark_ll(skb, &cq->napi);
> +

It seems there is a second point in the driver calling napi_gro_frags(),
around line 696

It looks like LLS wont be supported for this case ?

>  		/* Push it up the stack */
>  		netif_receive_skb(skb);
>  
> @@ -781,8 +784,13 @@ int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget)
>  	struct mlx4_en_priv *priv = netdev_priv(dev);
>  	int done;
>  
> +	if (!mlx4_en_cq_lock_napi(cq))
> +		return budget;
> +
>  	done = mlx4_en_process_rx_cq(dev, cq, budget);
>  
> +	mlx4_en_cq_unlock_napi(cq);
> +
>  	/* If we used up all the quota - we're probably not done yet... */
>  	if (done == budget)
>  		INC_PERF_COUNTER(priv->pstats.napi_quota);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support
  2013-06-17 11:37   ` Eric Dumazet
@ 2013-06-17 12:43     ` Amir Vadai
  0 siblings, 0 replies; 8+ messages in thread
From: Amir Vadai @ 2013-06-17 12:43 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, netdev, eliezer.tamir, Or Gerlitz

On 17/06/2013 14:37, Eric Dumazet wrote:
> On Mon, 2013-06-17 at 13:58 +0300, Amir Vadai wrote:
>> Add basic support for LLS.
>>
>> Signed-off-by: Amir Vadai <amirv@mellanox.com>
>> ---
>>  drivers/net/ethernet/mellanox/mlx4/en_cq.c     |   2 +
>>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |  49 +++++++++-
>>  drivers/net/ethernet/mellanox/mlx4/en_rx.c     |   8 ++
>>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   | 124 +++++++++++++++++++++++++
>>  4 files changed, 181 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> index 1e6c594..5e47efd 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> @@ -139,6 +139,7 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
>>  
>>  	if (!cq->is_tx) {
>>  		netif_napi_add(cq->dev, &cq->napi, mlx4_en_poll_rx_cq, 64);
>> +		napi_hash_add(&cq->napi);
>>  		napi_enable(&cq->napi);
>>  	}
>>  
>> @@ -162,6 +163,7 @@ void mlx4_en_deactivate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq)
>>  {
>>  	if (!cq->is_tx) {
>>  		napi_disable(&cq->napi);
>> +		napi_hash_del(&cq->napi);
> 
> Are we sure an RCU grace period is respected before deletion of memory
> holding cq->napi ?
I assumed that when mlx4_en_deactivate_cq() is called, no users of the
object mlx4_en_cq which holds the napi context, exist. But now that I
look at it again, this is not true.
Will add a call to synchronize_rcu() in v1.
Thanks.


> 
>>  		netif_napi_del(&cq->napi);
>>  	}
>>  
> 
> ...
> 
>>  
>>  static const struct net_device_ops mlx4_netdev_ops_master = {
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> index 02aee1e..15b5980 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> @@ -31,6 +31,7 @@
>>   *
>>   */
>>  
>> +#include <net/ll_poll.h>
>>  #include <linux/mlx4/cq.h>
>>  #include <linux/slab.h>
>>  #include <linux/mlx4/qp.h>
>> @@ -737,6 +738,8 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>>  					       timestamp);
>>  		}
>>  
>> +		skb_mark_ll(skb, &cq->napi);
>> +
> 
> It seems there is a second point in the driver calling napi_gro_frags(),
> around line 696
> 
> It looks like LLS wont be supported for this case ?
> 
I don't want LLS to be supported when GRO is on.

>>  		/* Push it up the stack */
>>  		netif_receive_skb(skb);
>>  
>> @@ -781,8 +784,13 @@ int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget)
>>  	struct mlx4_en_priv *priv = netdev_priv(dev);
>>  	int done;
>>  
>> +	if (!mlx4_en_cq_lock_napi(cq))
>> +		return budget;
>> +
>>  	done = mlx4_en_process_rx_cq(dev, cq, budget);
>>  
>> +	mlx4_en_cq_unlock_napi(cq);
>> +
>>  	/* If we used up all the quota - we're probably not done yet... */
>>  	if (done == budget)
>>  		INC_PERF_COUNTER(priv->pstats.napi_quota);
> 
> 

Thanks,
Amir

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 2/2] net/mlx4_en: Low Latency recv statistics
  2013-06-17 10:58 ` [PATCH net-next 2/2] net/mlx4_en: Low Latency recv statistics Amir Vadai
@ 2013-06-18  0:01   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2013-06-18  0:01 UTC (permalink / raw)
  To: amirv; +Cc: netdev, eliezer.tamir, ogerlitz

From: Amir Vadai <amirv@mellanox.com>
Date: Mon, 17 Jun 2013 13:58:23 +0300

> +#ifdef CONFIG_NET_LL_RX_POLL
> +#define LL_EXTENDED_STATS
> +#endif

Don't convert standard Kconfig CPP defines into your own internal thing.

I'm not applying this series until you fix this.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-06-18  0:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-17 10:58 [PATCH net-next 0/2] Low latency Socket support Amir Vadai
2013-06-17 10:58 ` [PATCH net-next 1/2] net/mlx4_en: Add Low Latency Socket (LLS) support Amir Vadai
2013-06-17 11:37   ` Eric Dumazet
2013-06-17 12:43     ` Amir Vadai
2013-06-17 10:58 ` [PATCH net-next 2/2] net/mlx4_en: Low Latency recv statistics Amir Vadai
2013-06-18  0:01   ` David Miller
2013-06-17 11:23 ` [PATCH net-next 0/2] Low latency Socket support Eliezer Tamir
2013-06-17 11:31   ` Amir Vadai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).