* [PATCH net-next] net: ixgbe: fix error handling in tc cls_u32 offload routines.
@ 2016-03-03 23:42 ` Jeff Kirsher
0 siblings, 0 replies; 7+ messages in thread
From: Sridhar Samudrala @ 2016-03-03 23:19 UTC (permalink / raw)
To: john.r.fastabend, davem, jeffrey.t.kirsher, netdev
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>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: ixgbe: fix error handling in tc cls_u32 offload routines.
2016-03-03 23:42 ` [Intel-wired-lan] [net-next] " Jeff Kirsher
(?)
@ 2016-03-03 23:37 ` Jeff Kirsher
2016-03-04 19:15 ` David Miller
-1 siblings, 1 reply; 7+ messages in thread
From: Jeff Kirsher @ 2016-03-03 23:37 UTC (permalink / raw)
To: Sridhar Samudrala, john.r.fastabend, davem, netdev
[-- Attachment #1: Type: text/plain, Size: 575 bytes --]
On Thu, 2016-03-03 at 15:19 -0800, 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>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 52
> +++++++++++++++++----------
> 1 file changed, 34 insertions(+), 18 deletions(-)
Dave I will add this to my queue
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Intel-wired-lan] [net-next] ixgbe: fix error handling in tc cls_u32 offload routines
@ 2016-03-03 23:42 ` Jeff Kirsher
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Kirsher @ 2016-03-03 23:42 UTC (permalink / raw)
To: intel-wired-lan
From: "Samudrala, Sridhar" <sridhar.samudrala@intel.com>
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>
---
Sending this to the correct mailing list (get_maintainer.pl is your
friend, please use it in the future)
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)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Intel-wired-lan] [net-next] ixgbe: fix error handling in tc cls_u32 offload routines
2016-03-03 23:42 ` [Intel-wired-lan] [net-next] " Jeff Kirsher
(?)
(?)
@ 2016-03-04 0:29 ` John Fastabend
2016-03-04 0:30 ` John Fastabend
-1 siblings, 1 reply; 7+ messages in thread
From: John Fastabend @ 2016-03-04 0:29 UTC (permalink / raw)
To: intel-wired-lan
On 16-03-03 03:42 PM, Jeff Kirsher wrote:
> From: "Samudrala, Sridhar" <sridhar.samudrala@intel.com>
>
> 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>
> ---
>
> Sending this to the correct mailing list (get_maintainer.pl is your
> friend, please use it in the future)
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 52 +++++++++++++++++----------
> 1 file changed, 34 insertions(+), 18 deletions(-)
>
Looks like a good cleanup to me.
Acked-by: John Fastabend <john.r.fastabend@intel.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Intel-wired-lan] [net-next] ixgbe: fix error handling in tc cls_u32 offload routines
2016-03-04 0:29 ` [Intel-wired-lan] [net-next] " John Fastabend
@ 2016-03-04 0:30 ` John Fastabend
2016-03-04 0:59 ` Samudrala, Sridhar
0 siblings, 1 reply; 7+ messages in thread
From: John Fastabend @ 2016-03-04 0:30 UTC (permalink / raw)
To: intel-wired-lan
On 16-03-03 04:29 PM, John Fastabend wrote:
> On 16-03-03 03:42 PM, Jeff Kirsher wrote:
>> From: "Samudrala, Sridhar" <sridhar.samudrala@intel.com>
>>
>> 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>
>> ---
>>
>> Sending this to the correct mailing list (get_maintainer.pl is your
>> friend, please use it in the future)
>>
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 52 +++++++++++++++++----------
>> 1 file changed, 34 insertions(+), 18 deletions(-)
>>
>
>
> Looks like a good cleanup to me.
>
> Acked-by: John Fastabend <john.r.fastabend@intel.com>
>
Also just checking Sridhar you tested both the adding just to
the first list e.g. matching ip address for example and doing
matches deeper in the stack ala match on sport/dport?
Thanks,
John
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Intel-wired-lan] [net-next] ixgbe: fix error handling in tc cls_u32 offload routines
2016-03-04 0:30 ` John Fastabend
@ 2016-03-04 0:59 ` Samudrala, Sridhar
0 siblings, 0 replies; 7+ messages in thread
From: Samudrala, Sridhar @ 2016-03-04 0:59 UTC (permalink / raw)
To: intel-wired-lan
On 3/3/2016 4:30 PM, John Fastabend wrote:
> On 16-03-03 04:29 PM, John Fastabend wrote:
>> On 16-03-03 03:42 PM, Jeff Kirsher wrote:
>>> From: "Samudrala, Sridhar" <sridhar.samudrala@intel.com>
>>>
>>> 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>
>>> ---
>>>
>>> Sending this to the correct mailing list (get_maintainer.pl is your
>>> friend, please use it in the future)
>>>
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 52 +++++++++++++++++----------
>>> 1 file changed, 34 insertions(+), 18 deletions(-)
>>>
>>
>> Looks like a good cleanup to me.
>>
>> Acked-by: John Fastabend <john.r.fastabend@intel.com>
>>
> Also just checking Sridhar you tested both the adding just to
> the first list e.g. matching ip address for example and doing
> matches deeper in the stack ala match on sport/dport?
Yes.
I tested adding matches on src/dst ip on the root hash table and matches
on tcp src port on the link hash table.
tc filter add dev p4p1 parent ffff: protocol ip prio 99 handle 800:0:1
u32 ht 800: match ip dst 15.0.0.1/32 match ip src 15.0.0.2/32 action drop
This creates the following flow director rule as seen via ethtool
Filter: 1
Rule Type: Raw IPv4
Src IP addr: 15.0.0.2 mask: 0.0.0.0
Dest IP addr: 15.0.0.1 mask: 0.0.0.0
TOS: 0x0 mask: 0xff
Protocol: 0 mask: 0xff
L4 bytes: 0x0 mask: 0xffffffff
VLAN EtherType: 0x0 mask: 0xffff
VLAN: 0x0 mask: 0xffff
User-defined: 0x0 mask: 0xffffffffffffffff
Action: Drop
To do tcp matches, i had to delete ip match rule in the root hash
table, create a new hash table linked to the root and add a filter
entry to that table.
c filter del dev p4p1 parent ffff: protocol ip prio 99 handle 800:0:1 u32
tc filter add dev p4p1 parent ffff: protocol ip prio 99 handle 1: u32
divisor 1
tc filter add dev p4p1 protocol ip parent ffff: prio 99 u32 ht 800:
order 2 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 u32 ht 1: order
3 match tcp src 23 ffff action drop
This creates the following flow director rule
Filter: 3
Rule Type: TCP over IPv4
Src IP addr: 0.0.0.0 mask: 255.255.255.255
Dest IP addr: 0.0.0.0 mask: 255.255.255.255
TOS: 0x0 mask: 0xff
Src port: 23 mask: 0x0
Dest port: 0 mask: 0xffff
VLAN EtherType: 0x0 mask: 0xffff
VLAN: 0x0 mask: 0xffff
User-defined: 0x0 mask: 0xffffffffffffffff
Action: Drop
Looks like we cannot have both these rules at the same time.
Also, more work is required to support matches on both ip as well as tcp
headers in the same rule.
Thanks
Sridhar
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: ixgbe: fix error handling in tc cls_u32 offload routines.
2016-03-03 23:37 ` [PATCH net-next] net: " Jeff Kirsher
@ 2016-03-04 19:15 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2016-03-04 19:15 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: sridhar.samudrala, john.r.fastabend, netdev
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 03 Mar 2016 15:37:52 -0800
> On Thu, 2016-03-03 at 15:19 -0800, 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>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 52
>> +++++++++++++++++----------
>> 1 file changed, 34 insertions(+), 18 deletions(-)
>
> Dave I will add this to my queue
Ok.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-03-04 19:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-03 23:19 [PATCH net-next] net: ixgbe: fix error handling in tc cls_u32 offload routines Sridhar Samudrala
2016-03-03 23:42 ` [Intel-wired-lan] [net-next] " Jeff Kirsher
2016-03-03 23:37 ` [PATCH net-next] net: " Jeff Kirsher
2016-03-04 19:15 ` David Miller
2016-03-04 0:29 ` [Intel-wired-lan] [net-next] " John Fastabend
2016-03-04 0:30 ` John Fastabend
2016-03-04 0:59 ` Samudrala, Sridhar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.