[PATCH net-next v2 0/2] Display classifier filters using ethtool

[PATCH net-next v2 0/2] Display classifier filters using ethtool

[Govindarajulu Varadarajan]

This series adds ethtool support to show classifier filters added by driver.

v2:
The patch 1/2 removes the $ifdef's around the filter structure. Making it
available always. So that .get_rxnfc() can be implimented without any #ifdefs

Please apply this to net-next.

Govindarajulu Varadarajan (2):
  enic: remove #ifdef CONFIG_RFS_ACCEL around filter structures
  enic: Add ethtool support to show classifier filters added by the
    driver

 drivers/net/ethernet/cisco/enic/enic.h         |  5 --
 drivers/net/ethernet/cisco/enic/enic_clsf.c    | 87 +++++++++++++----------
 drivers/net/ethernet/cisco/enic/enic_clsf.h    | 23 ++++--
 drivers/net/ethernet/cisco/enic/enic_ethtool.c | 98 ++++++++++++++++++++++++++
 4 files changed, 168 insertions(+), 45 deletions(-)

-- 
2.0.2

[PATCH net-next v2 1/2] enic: remove #ifdef CONFIG_RFS_ACCEL around filter structures

[Govindarajulu Varadarajan]

This patch removes the #ifdef CONFIG_RFS_ACCEL around the classifier filter
structures. This makes the filter structures available when CONFIG_RFS_ACCEL = n.

Introduce enic_rfs_timer_start() & enic_rfs_timer_stop() to start/stop the
timer. These two functions are nop when CONFIG_RFS_ACCEL = n.

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 drivers/net/ethernet/cisco/enic/enic.h      |  5 ---
 drivers/net/ethernet/cisco/enic/enic_clsf.c | 69 ++++++++++++++---------------
 drivers/net/ethernet/cisco/enic/enic_clsf.h | 22 +++++++--
 3 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 4ecbbb3..962510f 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -99,7 +99,6 @@ struct enic_port_profile {
 	u8 mac_addr[ETH_ALEN];
 };
 
-#ifdef CONFIG_RFS_ACCEL
 /* enic_rfs_fltr_node - rfs filter node in hash table
  *	@@keys: IPv4 5 tuple
  *	@flow_id: flow_id of clsf filter provided by kernel
@@ -135,8 +134,6 @@ struct enic_rfs_flw_tbl {
 	struct timer_list rfs_may_expire;
 };
 
-#endif /* CONFIG_RFS_ACCEL */
-
 /* Per-instance private data structure */
 struct enic {
 	struct net_device *netdev;
@@ -188,9 +185,7 @@ struct enic {
 	/* completion queue cache line section */
 	____cacheline_aligned struct vnic_cq cq[ENIC_CQ_MAX];
 	unsigned int cq_count;
-#ifdef CONFIG_RFS_ACCEL
 	struct enic_rfs_flw_tbl rfs_h;
-#endif
 };
 
 static inline struct device *enic_get_dev(struct enic *enic)
diff --git a/drivers/net/ethernet/cisco/enic/enic_clsf.c b/drivers/net/ethernet/cisco/enic/enic_clsf.c
index bc451ba..ee6acbf 100644
--- a/drivers/net/ethernet/cisco/enic/enic_clsf.c
+++ b/drivers/net/ethernet/cisco/enic/enic_clsf.c
@@ -65,37 +65,6 @@ int enic_delfltr(struct enic *enic, u16 filter_id)
 	return ret;
 }
 
-#ifdef CONFIG_RFS_ACCEL
-void enic_flow_may_expire(unsigned long data)
-{
-	struct enic *enic = (struct enic *)data;
-	bool res;
-	int j;
-
-	spin_lock(&enic->rfs_h.lock);
-	for (j = 0; j < ENIC_CLSF_EXPIRE_COUNT; j++) {
-		struct hlist_head *hhead;
-		struct hlist_node *tmp;
-		struct enic_rfs_fltr_node *n;
-
-		hhead = &enic->rfs_h.ht_head[enic->rfs_h.toclean++];
-		hlist_for_each_entry_safe(n, tmp, hhead, node) {
-			res = rps_may_expire_flow(enic->netdev, n->rq_id,
-						  n->flow_id, n->fltr_id);
-			if (res) {
-				res = enic_delfltr(enic, n->fltr_id);
-				if (unlikely(res))
-					continue;
-				hlist_del(&n->node);
-				kfree(n);
-				enic->rfs_h.free++;
-			}
-		}
-	}
-	spin_unlock(&enic->rfs_h.lock);
-	mod_timer(&enic->rfs_h.rfs_may_expire, jiffies + HZ/4);
-}
-
 /* enic_rfs_flw_tbl_init - initialize enic->rfs_h members
  *	@enic: enic data
  */
@@ -109,17 +78,14 @@ void enic_rfs_flw_tbl_init(struct enic *enic)
 	enic->rfs_h.max = enic->config.num_arfs;
 	enic->rfs_h.free = enic->rfs_h.max;
 	enic->rfs_h.toclean = 0;
-	init_timer(&enic->rfs_h.rfs_may_expire);
-	enic->rfs_h.rfs_may_expire.function = enic_flow_may_expire;
-	enic->rfs_h.rfs_may_expire.data = (unsigned long)enic;
-	mod_timer(&enic->rfs_h.rfs_may_expire, jiffies + HZ/4);
+	enic_rfs_timer_start(enic);
 }
 
 void enic_rfs_flw_tbl_free(struct enic *enic)
 {
 	int i;
 
-	del_timer_sync(&enic->rfs_h.rfs_may_expire);
+	enic_rfs_timer_stop(enic);
 	spin_lock(&enic->rfs_h.lock);
 	enic->rfs_h.free = 0;
 	for (i = 0; i < (1 << ENIC_RFS_FLW_BITSHIFT); i++) {
@@ -137,6 +103,37 @@ void enic_rfs_flw_tbl_free(struct enic *enic)
 	spin_unlock(&enic->rfs_h.lock);
 }
 
+#ifdef CONFIG_RFS_ACCEL
+void enic_flow_may_expire(unsigned long data)
+{
+	struct enic *enic = (struct enic *)data;
+	bool res;
+	int j;
+
+	spin_lock(&enic->rfs_h.lock);
+	for (j = 0; j < ENIC_CLSF_EXPIRE_COUNT; j++) {
+		struct hlist_head *hhead;
+		struct hlist_node *tmp;
+		struct enic_rfs_fltr_node *n;
+
+		hhead = &enic->rfs_h.ht_head[enic->rfs_h.toclean++];
+		hlist_for_each_entry_safe(n, tmp, hhead, node) {
+			res = rps_may_expire_flow(enic->netdev, n->rq_id,
+						  n->flow_id, n->fltr_id);
+			if (res) {
+				res = enic_delfltr(enic, n->fltr_id);
+				if (unlikely(res))
+					continue;
+				hlist_del(&n->node);
+				kfree(n);
+				enic->rfs_h.free++;
+			}
+		}
+	}
+	spin_unlock(&enic->rfs_h.lock);
+	mod_timer(&enic->rfs_h.rfs_may_expire, jiffies + HZ/4);
+}
+
 static struct enic_rfs_fltr_node *htbl_key_search(struct hlist_head *h,
 						  struct flow_keys *k)
 {
diff --git a/drivers/net/ethernet/cisco/enic/enic_clsf.h b/drivers/net/ethernet/cisco/enic/enic_clsf.h
index d572704..221f364 100644
--- a/drivers/net/ethernet/cisco/enic/enic_clsf.h
+++ b/drivers/net/ethernet/cisco/enic/enic_clsf.h
@@ -8,15 +8,29 @@
 
 int enic_addfltr_5t(struct enic *enic, struct flow_keys *keys, u16 rq);
 int enic_delfltr(struct enic *enic, u16 filter_id);
-
-#ifdef CONFIG_RFS_ACCEL
 void enic_rfs_flw_tbl_init(struct enic *enic);
 void enic_rfs_flw_tbl_free(struct enic *enic);
+
+#ifdef CONFIG_RFS_ACCEL
 int enic_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb,
 		       u16 rxq_index, u32 flow_id);
+void enic_flow_may_expire(unsigned long data);
+
+static inline void enic_rfs_timer_start(struct enic *enic)
+{
+	init_timer(&enic->rfs_h.rfs_may_expire);
+	enic->rfs_h.rfs_may_expire.function = enic_flow_may_expire;
+	enic->rfs_h.rfs_may_expire.data = (unsigned long)enic;
+	mod_timer(&enic->rfs_h.rfs_may_expire, jiffies + HZ/4);
+}
+
+static inline void enic_rfs_timer_stop(struct enic *enic)
+{
+	del_timer_sync(&enic->rfs_h.rfs_may_expire);
+}
 #else
-static inline void enic_rfs_flw_tbl_init(struct enic *enic) {}
-static inline void enic_rfs_flw_tbl_free(struct enic *enic) {}
+static inline void enic_rfs_timer_start(struct enic *enic) {}
+static inline void enic_rfs_timer_stop(struct enic *enic) {}
 #endif /* CONFIG_RFS_ACCEL */
 
 #endif /* _ENIC_CLSF_H_ */
-- 
2.0.2

[PATCH net-next v2 2/2] enic: Add ethtool support to show classifier filters added by the driver

[Govindarajulu Varadarajan]

This patch impliments ethtool_ops->get_rxnfc() to display the classifier
filter added by the driver.

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 drivers/net/ethernet/cisco/enic/enic_clsf.c    | 18 +++++
 drivers/net/ethernet/cisco/enic/enic_clsf.h    |  1 +
 drivers/net/ethernet/cisco/enic/enic_ethtool.c | 98 ++++++++++++++++++++++++++
 3 files changed, 117 insertions(+)

diff --git a/drivers/net/ethernet/cisco/enic/enic_clsf.c b/drivers/net/ethernet/cisco/enic/enic_clsf.c
index ee6acbf..69dfd3c 100644
--- a/drivers/net/ethernet/cisco/enic/enic_clsf.c
+++ b/drivers/net/ethernet/cisco/enic/enic_clsf.c
@@ -103,6 +103,24 @@ void enic_rfs_flw_tbl_free(struct enic *enic)
 	spin_unlock(&enic->rfs_h.lock);
 }
 
+struct enic_rfs_fltr_node *htbl_fltr_search(struct enic *enic, u16 fltr_id)
+{
+	int i;
+
+	for (i = 0; i < (1 << ENIC_RFS_FLW_BITSHIFT); i++) {
+		struct hlist_head *hhead;
+		struct hlist_node *tmp;
+		struct enic_rfs_fltr_node *n;
+
+		hhead = &enic->rfs_h.ht_head[i];
+		hlist_for_each_entry_safe(n, tmp, hhead, node)
+			if (n->fltr_id == fltr_id)
+				return n;
+	}
+
+	return NULL;
+}
+
 #ifdef CONFIG_RFS_ACCEL
 void enic_flow_may_expire(unsigned long data)
 {
diff --git a/drivers/net/ethernet/cisco/enic/enic_clsf.h b/drivers/net/ethernet/cisco/enic/enic_clsf.h
index 221f364..6aa9f89 100644
--- a/drivers/net/ethernet/cisco/enic/enic_clsf.h
+++ b/drivers/net/ethernet/cisco/enic/enic_clsf.h
@@ -10,6 +10,7 @@ int enic_addfltr_5t(struct enic *enic, struct flow_keys *keys, u16 rq);
 int enic_delfltr(struct enic *enic, u16 filter_id);
 void enic_rfs_flw_tbl_init(struct enic *enic);
 void enic_rfs_flw_tbl_free(struct enic *enic);
+struct enic_rfs_fltr_node *htbl_fltr_search(struct enic *enic, u16 fltr_id);
 
 #ifdef CONFIG_RFS_ACCEL
 int enic_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb,
diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index c75f84b..523c9ce 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -22,6 +22,7 @@
 #include "enic_res.h"
 #include "enic.h"
 #include "enic_dev.h"
+#include "enic_clsf.h"
 
 struct enic_stat {
 	char name[ETH_GSTRING_LEN];
@@ -282,6 +283,102 @@ static int enic_set_coalesce(struct net_device *netdev,
 	return 0;
 }
 
+static int enic_grxclsrlall(struct enic *enic, struct ethtool_rxnfc *cmd,
+			    u32 *rule_locs)
+{
+	int j, ret = 0, cnt = 0;
+
+	cmd->data = enic->rfs_h.max - enic->rfs_h.free;
+	for (j = 0; j < (1 << ENIC_RFS_FLW_BITSHIFT); j++) {
+		struct hlist_head *hhead;
+		struct hlist_node *tmp;
+		struct enic_rfs_fltr_node *n;
+
+		hhead = &enic->rfs_h.ht_head[j];
+		hlist_for_each_entry_safe(n, tmp, hhead, node) {
+			if (cnt == cmd->rule_cnt)
+				return -EMSGSIZE;
+			rule_locs[cnt] = n->fltr_id;
+			cnt++;
+		}
+	}
+	cmd->rule_cnt = cnt;
+
+	return ret;
+}
+
+static int enic_grxclsrule(struct enic *enic, struct ethtool_rxnfc *cmd)
+{
+	struct ethtool_rx_flow_spec *fsp =
+				(struct ethtool_rx_flow_spec *)&cmd->fs;
+	struct enic_rfs_fltr_node *n;
+
+	n = htbl_fltr_search(enic, (u16)fsp->location);
+	if (!n)
+		return -EINVAL;
+	switch (n->keys.ip_proto) {
+	case IPPROTO_TCP:
+		fsp->flow_type = TCP_V4_FLOW;
+		break;
+	case IPPROTO_UDP:
+		fsp->flow_type = UDP_V4_FLOW;
+		break;
+	default:
+		return -EINVAL;
+		break;
+	}
+
+	fsp->h_u.tcp_ip4_spec.ip4src = n->keys.src;
+	fsp->m_u.tcp_ip4_spec.ip4src = (__u32)~0;
+
+	fsp->h_u.tcp_ip4_spec.ip4dst = n->keys.dst;
+	fsp->m_u.tcp_ip4_spec.ip4dst = (__u32)~0;
+
+	fsp->h_u.tcp_ip4_spec.psrc = n->keys.port16[0];
+	fsp->m_u.tcp_ip4_spec.psrc = (__u16)~0;
+
+	fsp->h_u.tcp_ip4_spec.pdst = n->keys.port16[1];
+	fsp->m_u.tcp_ip4_spec.pdst = (__u16)~0;
+
+	fsp->ring_cookie = n->rq_id;
+
+	return 0;
+}
+
+static int enic_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
+			  u32 *rule_locs)
+{
+	struct enic *enic = netdev_priv(dev);
+	int ret = 0;
+
+	switch (cmd->cmd) {
+	case ETHTOOL_GRXRINGS:
+		cmd->data = enic->rq_count;
+		break;
+	case ETHTOOL_GRXCLSRLCNT:
+		spin_lock_bh(&enic->rfs_h.lock);
+		cmd->rule_cnt = enic->rfs_h.max - enic->rfs_h.free;
+		cmd->data = enic->rfs_h.max;
+		spin_unlock_bh(&enic->rfs_h.lock);
+		break;
+	case ETHTOOL_GRXCLSRLALL:
+		spin_lock_bh(&enic->rfs_h.lock);
+		ret = enic_grxclsrlall(enic, cmd, rule_locs);
+		spin_unlock_bh(&enic->rfs_h.lock);
+		break;
+	case ETHTOOL_GRXCLSRULE:
+		spin_lock_bh(&enic->rfs_h.lock);
+		ret = enic_grxclsrule(enic, cmd);
+		spin_unlock_bh(&enic->rfs_h.lock);
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+		break;
+	}
+
+	return ret;
+}
+
 static const struct ethtool_ops enic_ethtool_ops = {
 	.get_settings = enic_get_settings,
 	.get_drvinfo = enic_get_drvinfo,
@@ -293,6 +390,7 @@ static const struct ethtool_ops enic_ethtool_ops = {
 	.get_ethtool_stats = enic_get_ethtool_stats,
 	.get_coalesce = enic_get_coalesce,
 	.set_coalesce = enic_set_coalesce,
+	.get_rxnfc = enic_get_rxnfc,
 };
 
 void enic_set_ethtool_ops(struct net_device *netdev)
-- 
2.0.2

Re: [PATCH net-next v2 0/2] Display classifier filters using ethtool

[David Miller]

From: Govindarajulu Varadarajan <_govind@gmx.com>
Date: Mon, 21 Jul 2014 17:22:16 +0530

> This series adds ethtool support to show classifier filters added by driver.
> 
> v2:
> The patch 1/2 removes the $ifdef's around the filter structure. Making it
> available always. So that .get_rxnfc() can be implimented without any #ifdefs
> 
> Please apply this to net-next.

Series applied, thanks.

Re: [PATCH net-next v2 2/2] enic: Add ethtool support to show classifier filters added by the driver

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]

On Mon, 2014-07-21 at 17:22 +0530, Govindarajulu Varadarajan wrote:
> This patch impliments ethtool_ops->get_rxnfc() to display the classifier
> filter added by the driver.
[...]

ARFS and ethtool RXNFC are two different interfaces for setting filters
and they shouldn't be confused with each other.

In the sfc driver, I made the filters inserted through each interface
invisible from the other.  The mlx4_en driver also appears to keep the
two sets of filters separate.

I can see that it is useful to be able to find out which filters have
been inserted through ARFS, but it should be possible to distinguish
them from those inserted through RXNFC.  Please don't include those
filters when ETHTOOL_GRXCLSRLCNT or ETHTOOL_GRXCLSRLALL are used.

Either add new command numbers for getting ARFS filters (the same
structures will do), or define some kind of extension flag for this
purpose.

Ben.

-- 
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
                                                         - Carolyn Scheppner

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH net-next v2 2/2] enic: Add ethtool support to show classifier filters added by the driver

[Govindarajulu Varadarajan]

On Sat, 6 Sep 2014, Ben Hutchings wrote:

> On Mon, 2014-07-21 at 17:22 +0530, Govindarajulu Varadarajan wrote:
>> This patch impliments ethtool_ops->get_rxnfc() to display the classifier
>> filter added by the driver.
> [...]
>
> ARFS and ethtool RXNFC are two different interfaces for setting filters
> and they shouldn't be confused with each other.
>
> In the sfc driver, I made the filters inserted through each interface
> invisible from the other.  The mlx4_en driver also appears to keep the
> two sets of filters separate.
>
> I can see that it is useful to be able to find out which filters have
> been inserted through ARFS, but it should be possible to distinguish
> them from those inserted through RXNFC.  Please don't include those
> filters when ETHTOOL_GRXCLSRLCNT or ETHTOOL_GRXCLSRLALL are used.
>
> Either add new command numbers for getting ARFS filters (the same
> structures will do), or define some kind of extension flag for this
> purpose.
>

Will work on adding new ethtool cmd with same infrastructure as RXNFC for
displaying filters added by ARFS, removing existing get_rxnfc for enic and
use new ethtool cmd.

Thanks for your feedback.