[Intel-wired-lan] [PATCH v2 net-next 1/3] net: ixgbe: fix error handling in tc cls_u32 offload routines.

[Intel-wired-lan] [PATCH v2 net-next 1/3] net: ixgbe: fix error handling in tc cls_u32 offload routines.

[Sridhar Samudrala]

Check for handle ids when adding/deleting hash nodes OR adding/deleting
filter entries and limit them to max number of links or header nodes
supported(IXGBE_MAX_LINK_HANDLE).

Start from bit 0 when setting hash table bit-map.(adapter->tables)

Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>

---

Jeff,
This patchset replaces the following 2 patches that are in your queue.
https://patchwork.ozlabs.org/patch/591644/
https://patchwork.ozlabs.org/patch/592194/

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 52 +++++++++++++++++----------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b893ff8..10ccd96 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8209,10 +8209,17 @@ int ixgbe_setup_tc(struct net_device *dev, u8 tc)
 static int ixgbe_delete_clsu32(struct ixgbe_adapter *adapter,
 			       struct tc_cls_u32_offload *cls)
 {
+	u32 uhtid = TC_U32_USERHTID(cls->knode.handle);
+	u32 loc;
 	int err;
 
+	if ((uhtid != 0x800) && (uhtid >= IXGBE_MAX_LINK_HANDLE))
+		return -EINVAL;
+
+	loc = cls->knode.handle & 0xfffff;
+
 	spin_lock(&adapter->fdir_perfect_lock);
-	err = ixgbe_update_ethtool_fdir_entry(adapter, NULL, cls->knode.handle);
+	err = ixgbe_update_ethtool_fdir_entry(adapter, NULL, loc);
 	spin_unlock(&adapter->fdir_perfect_lock);
 	return err;
 }
@@ -8221,20 +8228,30 @@ static int ixgbe_configure_clsu32_add_hnode(struct ixgbe_adapter *adapter,
 					    __be16 protocol,
 					    struct tc_cls_u32_offload *cls)
 {
+	u32 uhtid = TC_U32_USERHTID(cls->hnode.handle);
+
+	if (uhtid >= IXGBE_MAX_LINK_HANDLE)
+		return -EINVAL;
+
 	/* This ixgbe devices do not support hash tables at the moment
 	 * so abort when given hash tables.
 	 */
 	if (cls->hnode.divisor > 0)
 		return -EINVAL;
 
-	set_bit(TC_U32_USERHTID(cls->hnode.handle), &adapter->tables);
+	set_bit(uhtid-1, &adapter->tables);
 	return 0;
 }
 
 static int ixgbe_configure_clsu32_del_hnode(struct ixgbe_adapter *adapter,
 					    struct tc_cls_u32_offload *cls)
 {
-	clear_bit(TC_U32_USERHTID(cls->hnode.handle), &adapter->tables);
+	u32 uhtid = TC_U32_USERHTID(cls->hnode.handle);
+
+	if (uhtid >= IXGBE_MAX_LINK_HANDLE)
+		return -EINVAL;
+
+	clear_bit(uhtid-1, &adapter->tables);
 	return 0;
 }
 
@@ -8252,27 +8269,29 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter,
 #endif
 	int i, err = 0;
 	u8 queue;
-	u32 handle;
+	u32 uhtid, link_uhtid;
 
 	memset(&mask, 0, sizeof(union ixgbe_atr_input));
-	handle = cls->knode.handle;
+	uhtid = TC_U32_USERHTID(cls->knode.handle);
+	link_uhtid = TC_U32_USERHTID(cls->knode.link_handle);
 
-	/* At the moment cls_u32 jumps to transport layer and skips past
+	/* At the moment cls_u32 jumps to network layer and skips past
 	 * L2 headers. The canonical method to match L2 frames is to use
 	 * negative values. However this is error prone at best but really
 	 * just broken because there is no way to "know" what sort of hdr
-	 * is in front of the transport layer. Fix cls_u32 to support L2
+	 * is in front of the network layer. Fix cls_u32 to support L2
 	 * headers when needed.
 	 */
 	if (protocol != htons(ETH_P_IP))
 		return -EINVAL;
 
-	if (cls->knode.link_handle ||
-	    cls->knode.link_handle >= IXGBE_MAX_LINK_HANDLE) {
+	if (link_uhtid) {
 		struct ixgbe_nexthdr *nexthdr = ixgbe_ipv4_jumps;
-		u32 uhtid = TC_U32_USERHTID(cls->knode.link_handle);
 
-		if (!test_bit(uhtid, &adapter->tables))
+		if (link_uhtid >= IXGBE_MAX_LINK_HANDLE)
+			return -EINVAL;
+
+		if (!test_bit(link_uhtid-1, &adapter->tables))
 			return -EINVAL;
 
 		for (i = 0; nexthdr[i].jump; i++) {
@@ -8288,10 +8307,7 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter,
 			    nexthdr->mask != cls->knode.sel->keys[0].mask)
 				return -EINVAL;
 
-			if (uhtid >= IXGBE_MAX_LINK_HANDLE)
-				return -EINVAL;
-
-			adapter->jump_tables[uhtid] = nexthdr->jump;
+			adapter->jump_tables[link_uhtid] = nexthdr->jump;
 		}
 		return 0;
 	}
@@ -8308,13 +8324,13 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter,
 	 * To add support for new nodes update ixgbe_model.h parse structures
 	 * this function _should_ be generic try not to hardcode values here.
 	 */
-	if (TC_U32_USERHTID(handle) == 0x800) {
+	if (uhtid == 0x800) {
 		field_ptr = adapter->jump_tables[0];
 	} else {
-		if (TC_U32_USERHTID(handle) >= ARRAY_SIZE(adapter->jump_tables))
+		if (uhtid >= IXGBE_MAX_LINK_HANDLE)
 			return -EINVAL;
 
-		field_ptr = adapter->jump_tables[TC_U32_USERHTID(handle)];
+		field_ptr = adapter->jump_tables[uhtid];
 	}
 
 	if (!field_ptr)
-- 
2.1.0

[Intel-wired-lan] [PATCH v2 net-next 2/3] net: ixgbe: Fix cls_u32 offload support for fields with masks.

[Sridhar Samudrala]

Remove the incorrect check for mask in ixgbe_configure_clsu32 and
drop the 'mask' field that is not required in struct ixgbe_mat_field

Verified with the following filters:

 #tc qdisc add dev p4p1 ingress
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 800:0:1 u32 ht 800: \
	match ip dst 10.0.0.1/8 match ip src 10.0.0.2/8 action drop
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 800:0:2 u32 ht 800: \
	match ip dst 11.0.0.1/16 match ip src 11.0.0.2/16 action drop
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 800:0:3 u32 ht 800: \
	match ip dst 12.0.0.1/24 match ip src 12.0.0.2/24 action drop
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 800:0:4 u32 ht 800: \
	match ip dst 13.0.0.1/32 match ip src 13.0.0.2/32 action drop

Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 3 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_model.h | 9 ++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 10ccd96..c520f98 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8348,8 +8348,7 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter,
 		int j;
 
 		for (j = 0; field_ptr[j].val; j++) {
-			if (field_ptr[j].off == off &&
-			    field_ptr[j].mask == m) {
+			if (field_ptr[j].off == off) {
 				field_ptr[j].val(input, &mask, val, m);
 				input->filter.formatted.flow_type |=
 					field_ptr[j].type;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
index ce48872..61f7290 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
@@ -32,7 +32,6 @@
 
 struct ixgbe_mat_field {
 	unsigned int off;
-	unsigned int mask;
 	int (*val)(struct ixgbe_fdir_filter *input,
 		   union ixgbe_atr_input *mask,
 		   u32 val, u32 m);
@@ -58,9 +57,9 @@ static inline int ixgbe_mat_prgm_dip(struct ixgbe_fdir_filter *input,
 }
 
 static struct ixgbe_mat_field ixgbe_ipv4_fields[] = {
-	{ .off = 12, .mask = -1, .val = ixgbe_mat_prgm_sip,
+	{ .off = 12, .val = ixgbe_mat_prgm_sip,
 	  .type = IXGBE_ATR_FLOW_TYPE_IPV4},
-	{ .off = 16, .mask = -1, .val = ixgbe_mat_prgm_dip,
+	{ .off = 16, .val = ixgbe_mat_prgm_dip,
 	  .type = IXGBE_ATR_FLOW_TYPE_IPV4},
 	{ .val = NULL } /* terminal node */
 };
@@ -84,9 +83,9 @@ static inline int ixgbe_mat_prgm_dport(struct ixgbe_fdir_filter *input,
 };
 
 static struct ixgbe_mat_field ixgbe_tcp_fields[] = {
-	{.off = 0, .mask = 0xffff, .val = ixgbe_mat_prgm_sport,
+	{.off = 0, .val = ixgbe_mat_prgm_sport,
 	 .type = IXGBE_ATR_FLOW_TYPE_TCPV4},
-	{.off = 2, .mask = 0xffff, .val = ixgbe_mat_prgm_dport,
+	{.off = 2, .val = ixgbe_mat_prgm_dport,
 	 .type = IXGBE_ATR_FLOW_TYPE_TCPV4},
 	{ .val = NULL } /* terminal node */
 };
-- 
2.1.0

[Intel-wired-lan] [PATCH v2 net-next 3/3] net: ixgbe: Fix cls_u32 offload support for L4 ports.

[Sridhar Samudrala]

Fix support for 16 bit source/dest port matches in ixgbe model.
u32 uses a single 32-bit key value for both source and destination ports
starting at offset 0. So replace the 2 functions with a single function
that takes this key value/mask to program both source and dest ports.

Verified with the following filter:

 #tc qdisc add dev p4p1 ingress
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 1: u32 divisor 1
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 800:0:10 u32 ht 800: link 1: \
	offset at 0 mask 0f00 shift 6 plus 0 eat match ip protocol 6 ff
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 1:0:10 u32 ht 1: \
	match tcp src 1024 ffff match tcp dst 80 ffff action drop
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 1:0:11 u32 ht 1: \
	match tcp src 1025 ffff action drop
 #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
	handle 1:0:12 u32 ht 1: \
	match tcp dst 81 ffff action drop

Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_model.h | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
index 61f7290..74c53ad 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
@@ -64,28 +64,20 @@ static struct ixgbe_mat_field ixgbe_ipv4_fields[] = {
 	{ .val = NULL } /* terminal node */
 };
 
-static inline int ixgbe_mat_prgm_sport(struct ixgbe_fdir_filter *input,
+static inline int ixgbe_mat_prgm_ports(struct ixgbe_fdir_filter *input,
 				       union ixgbe_atr_input *mask,
 				       u32 val, u32 m)
 {
 	input->filter.formatted.src_port = val & 0xffff;
 	mask->formatted.src_port = m & 0xffff;
-	return 0;
-};
+	input->filter.formatted.dst_port = val >> 16;
+	mask->formatted.dst_port = m >> 16;
 
-static inline int ixgbe_mat_prgm_dport(struct ixgbe_fdir_filter *input,
-				       union ixgbe_atr_input *mask,
-				       u32 val, u32 m)
-{
-	input->filter.formatted.dst_port = val & 0xffff;
-	mask->formatted.dst_port = m & 0xffff;
 	return 0;
 };
 
 static struct ixgbe_mat_field ixgbe_tcp_fields[] = {
-	{.off = 0, .val = ixgbe_mat_prgm_sport,
-	 .type = IXGBE_ATR_FLOW_TYPE_TCPV4},
-	{.off = 2, .val = ixgbe_mat_prgm_dport,
+	{.off = 0, .val = ixgbe_mat_prgm_ports,
 	 .type = IXGBE_ATR_FLOW_TYPE_TCPV4},
 	{ .val = NULL } /* terminal node */
 };
-- 
2.1.0

[Intel-wired-lan] [PATCH v2 net-next 1/3] net: ixgbe: fix error handling in tc cls_u32 offload routines.

[John Fastabend]

On 16-03-07 09:41 AM, Sridhar Samudrala wrote:
> Check for handle ids when adding/deleting hash nodes OR adding/deleting
> filter entries and limit them to max number of links or header nodes
> supported(IXGBE_MAX_LINK_HANDLE).
> 
> Start from bit 0 when setting hash table bit-map.(adapter->tables)
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> 
> ---
> 

Looks good.

Acked-by: John Fastabend <john.r.fastabend@intel.com>

[Intel-wired-lan] [PATCH v2 net-next 2/3] net: ixgbe: Fix cls_u32 offload support for fields with masks.

[John Fastabend]

On 16-03-07 09:41 AM, Sridhar Samudrala wrote:
> Remove the incorrect check for mask in ixgbe_configure_clsu32 and
> drop the 'mask' field that is not required in struct ixgbe_mat_field
> 
> Verified with the following filters:
> 
>  #tc qdisc add dev p4p1 ingress
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:1 u32 ht 800: \
> 	match ip dst 10.0.0.1/8 match ip src 10.0.0.2/8 action drop
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:2 u32 ht 800: \
> 	match ip dst 11.0.0.1/16 match ip src 11.0.0.2/16 action drop
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:3 u32 ht 800: \
> 	match ip dst 12.0.0.1/24 match ip src 12.0.0.2/24 action drop
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:4 u32 ht 800: \
> 	match ip dst 13.0.0.1/32 match ip src 13.0.0.2/32 action drop
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---


Nice cleanup now we can support various masks as well on the other
fields.

Acked-by: John Fastabend <john.r.fastabend@intel.com>

[Intel-wired-lan] [PATCH v2 net-next 3/3] net: ixgbe: Fix cls_u32 offload support for L4 ports.

[John Fastabend]

On 16-03-07 09:41 AM, Sridhar Samudrala wrote:
> Fix support for 16 bit source/dest port matches in ixgbe model.
> u32 uses a single 32-bit key value for both source and destination ports
> starting at offset 0. So replace the 2 functions with a single function
> that takes this key value/mask to program both source and dest ports.
> 
> Verified with the following filter:
> 
>  #tc qdisc add dev p4p1 ingress
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 1: u32 divisor 1
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:10 u32 ht 800: link 1: \
> 	offset at 0 mask 0f00 shift 6 plus 0 eat match ip protocol 6 ff
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 1:0:10 u32 ht 1: \
> 	match tcp src 1024 ffff match tcp dst 80 ffff action drop
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 1:0:11 u32 ht 1: \
> 	match tcp src 1025 ffff action drop
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 1:0:12 u32 ht 1: \
> 	match tcp dst 81 ffff action drop
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---


Thanks nice looking series.

Acked-by: John Fastabend <john.r.fastabend@intel.com>

[Intel-wired-lan] [PATCH v2 net-next 2/3] net: ixgbe: Fix cls_u32 offload support for fields with masks.

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Sridhar Samudrala
> Sent: Monday, March 07, 2016 9:42 AM
> To: intel-wired-lan at lists.osuosl.org; Fastabend, John R
> <john.r.fastabend@intel.com>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [PATCH v2 net-next 2/3] net: ixgbe: Fix cls_u32
> offload support for fields with masks.
> 
> Remove the incorrect check for mask in ixgbe_configure_clsu32 and drop the
> 'mask' field that is not required in struct ixgbe_mat_field
> 
> Verified with the following filters:
> 
>  #tc qdisc add dev p4p1 ingress
>  #tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:1 u32 ht 800: \
> 	match ip dst 10.0.0.1/8 match ip src 10.0.0.2/8 action drop  #tc filter
> add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:2 u32 ht 800: \
> 	match ip dst 11.0.0.1/16 match ip src 11.0.0.2/16 action drop  #tc filter
> add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:3 u32 ht 800: \
> 	match ip dst 12.0.0.1/24 match ip src 12.0.0.2/24 action drop  #tc filter
> add dev p4p1 parent ffff: protocol ip prio 99 \
> 	handle 800:0:4 u32 ht 800: \
> 	match ip dst 13.0.0.1/32 match ip src 13.0.0.2/32 action drop
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 3 +--
> drivers/net/ethernet/intel/ixgbe/ixgbe_model.h | 9 ++++-----
>  2 files changed, 5 insertions(+), 7 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Adding/removing filters works as expected and filters are shown correctly.

[Intel-wired-lan] [PATCH v2 net-next 1/3] net: ixgbe: fix error handling in tc cls_u32 offload routines.

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Sridhar Samudrala
> Sent: Monday, March 07, 2016 9:42 AM
> To: intel-wired-lan at lists.osuosl.org; Fastabend, John R
> <john.r.fastabend@intel.com>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [PATCH v2 net-next 1/3] net: ixgbe: fix error
> handling in tc cls_u32 offload routines.
> 
> Check for handle ids when adding/deleting hash nodes OR adding/deleting
> filter entries and limit them to max number of links or header nodes
> supported(IXGBE_MAX_LINK_HANDLE).
> 
> Start from bit 0 when setting hash table bit-map.(adapter->tables)
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> 
> ---
> 
> Jeff,
> This patchset replaces the following 2 patches that are in your queue.
> https://patchwork.ozlabs.org/patch/591644/
> https://patchwork.ozlabs.org/patch/592194/
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 52 +++++++++++++++++--
> --------
>  1 file changed, 34 insertions(+), 18 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>