All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-04-16 14:43 ` Asbjørn Sloth Tønnesen
  0 siblings, 0 replies; 14+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2024-04-16 14:43 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Asbjørn Sloth Tønnesen, netdev, linux-kernel,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Jesse Brandeburg, Tony Nguyen

This driver currently doesn't support any control flags.

Use flow_rule_has_control_flags() to check for control flags,
such as can be set through `tc flower ... ip_flags frag`.

In case any control flags are masked, flow_rule_has_control_flags()
sets a NL extended error message, and we return -EOPNOTSUPP.

Only compile-tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 0bdcdea0be3e..e219f757820d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8643,6 +8643,10 @@ static int i40e_parse_cls_flower(struct i40e_vsi *vsi,
 
 		flow_rule_match_control(rule, &match);
 		addr_type = match.key->addr_type;
+
+		if (flow_rule_has_control_flags(match.mask->flags,
+						f->common.extack))
+			return -EOPNOTSUPP;
 	}
 
 	if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
-- 
2.43.0


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

* [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-04-16 14:43 ` Asbjørn Sloth Tønnesen
  0 siblings, 0 replies; 14+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2024-04-16 14:43 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, linux-kernel, Eric Dumazet, Tony Nguyen,
	Asbjørn Sloth Tønnesen, Jakub Kicinski, Paolo Abeni,
	David S. Miller

This driver currently doesn't support any control flags.

Use flow_rule_has_control_flags() to check for control flags,
such as can be set through `tc flower ... ip_flags frag`.

In case any control flags are masked, flow_rule_has_control_flags()
sets a NL extended error message, and we return -EOPNOTSUPP.

Only compile-tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 0bdcdea0be3e..e219f757820d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8643,6 +8643,10 @@ static int i40e_parse_cls_flower(struct i40e_vsi *vsi,
 
 		flow_rule_match_control(rule, &match);
 		addr_type = match.key->addr_type;
+
+		if (flow_rule_has_control_flags(match.mask->flags,
+						f->common.extack))
+			return -EOPNOTSUPP;
 	}
 
 	if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
-- 
2.43.0


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

* Re: [PATCH iwl-next] i40e: flower: validate control flags
  2024-04-16 14:43 ` [Intel-wired-lan] " Asbjørn Sloth Tønnesen
@ 2024-04-18 18:16   ` Simon Horman
  -1 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2024-04-18 18:16 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen
  Cc: intel-wired-lan, netdev, linux-kernel, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jesse Brandeburg,
	Tony Nguyen

On Tue, Apr 16, 2024 at 02:43:19PM +0000, Asbjørn Sloth Tønnesen wrote:
> This driver currently doesn't support any control flags.
> 
> Use flow_rule_has_control_flags() to check for control flags,
> such as can be set through `tc flower ... ip_flags frag`.
> 
> In case any control flags are masked, flow_rule_has_control_flags()
> sets a NL extended error message, and we return -EOPNOTSUPP.
> 
> Only compile-tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-04-18 18:16   ` Simon Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2024-04-18 18:16 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen
  Cc: Tony Nguyen, netdev, linux-kernel, Eric Dumazet, intel-wired-lan,
	Jakub Kicinski, Paolo Abeni, David S. Miller

On Tue, Apr 16, 2024 at 02:43:19PM +0000, Asbjørn Sloth Tønnesen wrote:
> This driver currently doesn't support any control flags.
> 
> Use flow_rule_has_control_flags() to check for control flags,
> such as can be set through `tc flower ... ip_flags frag`.
> 
> In case any control flags are masked, flow_rule_has_control_flags()
> sets a NL extended error message, and we return -EOPNOTSUPP.
> 
> Only compile-tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Simon Horman <horms@kernel.org>


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

* RE: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
  2024-04-16 14:43 ` [Intel-wired-lan] " Asbjørn Sloth Tønnesen
@ 2024-05-06  5:32   ` Buvaneswaran, Sujai
  -1 siblings, 0 replies; 14+ messages in thread
From: Buvaneswaran, Sujai @ 2024-05-06  5:32 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, intel-wired-lan
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	Jakub Kicinski, Paolo Abeni, David S. Miller

Hi Asbjørn,

HW offload is not supported on the i40e interface. This patch cannot be tested on i40e interface.

Regards,
Sujai B

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Asbjørn Sloth Tønnesen
> Sent: Tuesday, April 16, 2024 8:13 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Asbjørn Sloth Tønnesen <ast@fiberby.net>;
> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> David S. Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
> 
> This driver currently doesn't support any control flags.
> 
> Use flow_rule_has_control_flags() to check for control flags, such as can be
> set through `tc flower ... ip_flags frag`.
> 
> In case any control flags are masked, flow_rule_has_control_flags() sets a NL
> extended error message, and we return -EOPNOTSUPP.
> 
> Only compile-tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 0bdcdea0be3e..e219f757820d 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -8643,6 +8643,10 @@ static int i40e_parse_cls_flower(struct i40e_vsi
> *vsi,
> 
>  		flow_rule_match_control(rule, &match);
>  		addr_type = match.key->addr_type;
> +
> +		if (flow_rule_has_control_flags(match.mask->flags,
> +						f->common.extack))
> +			return -EOPNOTSUPP;
>  	}
> 
>  	if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
> --
> 2.43.0


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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-05-06  5:32   ` Buvaneswaran, Sujai
  0 siblings, 0 replies; 14+ messages in thread
From: Buvaneswaran, Sujai @ 2024-05-06  5:32 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, intel-wired-lan
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	Jakub Kicinski, Paolo Abeni, David S. Miller

Hi Asbjørn,

HW offload is not supported on the i40e interface. This patch cannot be tested on i40e interface.

Regards,
Sujai B

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Asbjørn Sloth Tønnesen
> Sent: Tuesday, April 16, 2024 8:13 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Asbjørn Sloth Tønnesen <ast@fiberby.net>;
> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> David S. Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
> 
> This driver currently doesn't support any control flags.
> 
> Use flow_rule_has_control_flags() to check for control flags, such as can be
> set through `tc flower ... ip_flags frag`.
> 
> In case any control flags are masked, flow_rule_has_control_flags() sets a NL
> extended error message, and we return -EOPNOTSUPP.
> 
> Only compile-tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 0bdcdea0be3e..e219f757820d 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -8643,6 +8643,10 @@ static int i40e_parse_cls_flower(struct i40e_vsi
> *vsi,
> 
>  		flow_rule_match_control(rule, &match);
>  		addr_type = match.key->addr_type;
> +
> +		if (flow_rule_has_control_flags(match.mask->flags,
> +						f->common.extack))
> +			return -EOPNOTSUPP;
>  	}
> 
>  	if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
> --
> 2.43.0


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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
  2024-05-06  5:32   ` Buvaneswaran, Sujai
@ 2024-05-06  8:44     ` Asbjørn Sloth Tønnesen
  -1 siblings, 0 replies; 14+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2024-05-06  8:44 UTC (permalink / raw)
  To: Buvaneswaran, Sujai
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	Jakub Kicinski, Paolo Abeni, David S. Miller, intel-wired-lan

Hi Sujai,

Thank you for testing.

On 5/6/24 5:32 AM, Buvaneswaran, Sujai wrote:
> HW offload is not supported on the i40e interface. This patch cannot be tested on i40e interface.

To me it looks like it's supported (otherwise there is a lot of dead flower code in i40e_main.c),
although it's a bit limited in functionality, and is called "cloud filters".

static const struct net_device_ops i40e_netdev_ops = {
	[...]
	.ndo_setup_tc           = __i40e_setup_tc,
	[...]
};

There is a path from __i40e_setup_tc() to i40e_parse_cls_flower(),
so it should be possible to test this patch.

Most of the gatekeeping is in i40e_configure_clsflower().

I think you should be able to get past the gatekeeping with this:

ethtool -K $iface ntuple off
ethtool -K $iface hw-tc-offload on
tc qdisc add dev $iface ingress
tc filter add dev $iface protocol ip parent ffff: prio 1 flower dst_mac 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1

The above filter is based on the first example in:
   [jkirsher/next-queue PATCH v5 6/6] i40e: Enable cloud filters via tc-flower
   https://lore.kernel.org/netdev/150909696126.48377.794676088838721605.stgit@anamdev.jf.intel.com/

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541

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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-05-06  8:44     ` Asbjørn Sloth Tønnesen
  0 siblings, 0 replies; 14+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2024-05-06  8:44 UTC (permalink / raw)
  To: Buvaneswaran, Sujai
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	intel-wired-lan, Jakub Kicinski, Paolo Abeni, David S. Miller

Hi Sujai,

Thank you for testing.

On 5/6/24 5:32 AM, Buvaneswaran, Sujai wrote:
> HW offload is not supported on the i40e interface. This patch cannot be tested on i40e interface.

To me it looks like it's supported (otherwise there is a lot of dead flower code in i40e_main.c),
although it's a bit limited in functionality, and is called "cloud filters".

static const struct net_device_ops i40e_netdev_ops = {
	[...]
	.ndo_setup_tc           = __i40e_setup_tc,
	[...]
};

There is a path from __i40e_setup_tc() to i40e_parse_cls_flower(),
so it should be possible to test this patch.

Most of the gatekeeping is in i40e_configure_clsflower().

I think you should be able to get past the gatekeeping with this:

ethtool -K $iface ntuple off
ethtool -K $iface hw-tc-offload on
tc qdisc add dev $iface ingress
tc filter add dev $iface protocol ip parent ffff: prio 1 flower dst_mac 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1

The above filter is based on the first example in:
   [jkirsher/next-queue PATCH v5 6/6] i40e: Enable cloud filters via tc-flower
   https://lore.kernel.org/netdev/150909696126.48377.794676088838721605.stgit@anamdev.jf.intel.com/

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541

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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
  2024-05-06  8:44     ` Asbjørn Sloth Tønnesen
@ 2024-05-06 17:54       ` Samudrala, Sridhar
  -1 siblings, 0 replies; 14+ messages in thread
From: Samudrala, Sridhar @ 2024-05-06 17:54 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, Buvaneswaran, Sujai
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	Jakub Kicinski, Paolo Abeni, David S. Miller, intel-wired-lan



On 5/6/2024 1:44 AM, Asbjørn Sloth Tønnesen wrote:
> Hi Sujai,
> 
> Thank you for testing.
> 
> On 5/6/24 5:32 AM, Buvaneswaran, Sujai wrote:
>> HW offload is not supported on the i40e interface. This patch cannot 
>> be tested on i40e interface.
> 
> To me it looks like it's supported (otherwise there is a lot of dead 
> flower code in i40e_main.c),
> although it's a bit limited in functionality, and is called "cloud 
> filters".
> 
> static const struct net_device_ops i40e_netdev_ops = {
>      [...]
>      .ndo_setup_tc           = __i40e_setup_tc,
>      [...]
> };
> 
> There is a path from __i40e_setup_tc() to i40e_parse_cls_flower(),
> so it should be possible to test this patch.
> 
> Most of the gatekeeping is in i40e_configure_clsflower().
> 
> I think you should be able to get past the gatekeeping with this:
> 
> ethtool -K $iface ntuple off
> ethtool -K $iface hw-tc-offload on
> tc qdisc add dev $iface ingress

One step is missing before adding the filter.
In order to use hw_tc action, queue groups need to be created and can be 
done using

tc qdisc add dev $iface root mqprio num_tc 2 map 0 1 queues 2@0 8@2 hw 1 
mode channel

> tc filter add dev $iface protocol ip parent ffff: prio 1 flower dst_mac 
> 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1
> 
> The above filter is based on the first example in:
>    [jkirsher/next-queue PATCH v5 6/6] i40e: Enable cloud filters via 
> tc-flower
>    
> https://lore.kernel.org/netdev/150909696126.48377.794676088838721605.stgit@anamdev.jf.intel.com/
> 

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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-05-06 17:54       ` Samudrala, Sridhar
  0 siblings, 0 replies; 14+ messages in thread
From: Samudrala, Sridhar @ 2024-05-06 17:54 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, Buvaneswaran, Sujai
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	intel-wired-lan, Jakub Kicinski, Paolo Abeni, David S. Miller



On 5/6/2024 1:44 AM, Asbjørn Sloth Tønnesen wrote:
> Hi Sujai,
> 
> Thank you for testing.
> 
> On 5/6/24 5:32 AM, Buvaneswaran, Sujai wrote:
>> HW offload is not supported on the i40e interface. This patch cannot 
>> be tested on i40e interface.
> 
> To me it looks like it's supported (otherwise there is a lot of dead 
> flower code in i40e_main.c),
> although it's a bit limited in functionality, and is called "cloud 
> filters".
> 
> static const struct net_device_ops i40e_netdev_ops = {
>      [...]
>      .ndo_setup_tc           = __i40e_setup_tc,
>      [...]
> };
> 
> There is a path from __i40e_setup_tc() to i40e_parse_cls_flower(),
> so it should be possible to test this patch.
> 
> Most of the gatekeeping is in i40e_configure_clsflower().
> 
> I think you should be able to get past the gatekeeping with this:
> 
> ethtool -K $iface ntuple off
> ethtool -K $iface hw-tc-offload on
> tc qdisc add dev $iface ingress

One step is missing before adding the filter.
In order to use hw_tc action, queue groups need to be created and can be 
done using

tc qdisc add dev $iface root mqprio num_tc 2 map 0 1 queues 2@0 8@2 hw 1 
mode channel

> tc filter add dev $iface protocol ip parent ffff: prio 1 flower dst_mac 
> 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1
> 
> The above filter is based on the first example in:
>    [jkirsher/next-queue PATCH v5 6/6] i40e: Enable cloud filters via 
> tc-flower
>    
> https://lore.kernel.org/netdev/150909696126.48377.794676088838721605.stgit@anamdev.jf.intel.com/
> 

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

* RE: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
  2024-05-06 17:54       ` Samudrala, Sridhar
@ 2024-05-08 10:59         ` Buvaneswaran, Sujai
  -1 siblings, 0 replies; 14+ messages in thread
From: Buvaneswaran, Sujai @ 2024-05-08 10:59 UTC (permalink / raw)
  To: Samudrala, Sridhar, Asbjørn Sloth Tønnesen
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	Jakub Kicinski, Paolo Abeni, David S. Miller, intel-wired-lan

Hi,

I'm able to test this patch on i40e interface from ADQ perspective. Getting 'Not supported' message when tried to configure tc rule with control flags.

[root@BP-node3-BINDU ~]# rmmod i40e
rmmod: ERROR: Module i40e is in use by: irdma
[root@BP-node3-BINDU ~]#  modprobe i40e
[root@BP-node3-BINDU ~]#  ethtool -K ens801f0np0 ntuple off
[root@BP-node3-BINDU ~]#  ethtool -K ens801f0np0 hw-tc-offload on
[root@BP-node3-BINDU ~]#  tc qdisc add dev ens801f0np0 ingress
[root@BP-node3-BINDU ~]#  tc qdisc add dev ens801f0np0 root mqprio num_tc 2 map 0 1 queues 2@0 8@2 hw 1 mode channel
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 ip_flags frag skip_sw hw_tc 1
RTNETLINK answers: Operation not supported
We have an error talking to the kernel
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 ip_flags frag/firstfrag  skip_sw hw_tc 1
RTNETLINK answers: Operation not supported
We have an error talking to the kernel
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 skip_sw hw_tc 1
[root@BP-node3-BINDU ~]# tc filter show dev ens801f0np0 root
filter parent ffff: protocol ip pref 1 flower chain 0 
filter parent ffff: protocol ip pref 1 flower chain 0 handle 0x1 hw_tc 1 
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.10
  dst_port 12000
  skip_sw
  in_hw in_hw_count 1
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 ip_flags frag/firstfrag hw_tc 1
[root@BP-node3-BINDU ~]# tc filter show dev ens801f0np0 root
filter parent ffff: protocol ip pref 1 flower chain 0 
filter parent ffff: protocol ip pref 1 flower chain 0 handle 0x1 hw_tc 1 
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.10
  dst_port 12000
  skip_sw
  in_hw in_hw_count 1
filter parent ffff: protocol ip pref 1 flower chain 0 handle 0x2 hw_tc 1 
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.10
  dst_port 12000
  ip_flags frag/firstfrag
  not_in_hw

Regards,
Sujai B

> -----Original Message-----
> From: Samudrala, Sridhar <sridhar.samudrala@intel.com>
> Sent: Monday, May 6, 2024 11:25 PM
> To: Asbjørn Sloth Tønnesen <ast@fiberby.net>; Buvaneswaran, Sujai
> <sujai.buvaneswaran@intel.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>;
> intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control
> flags
> 
> 
> 
> On 5/6/2024 1:44 AM, Asbjørn Sloth Tønnesen wrote:
> > Hi Sujai,
> >
> > Thank you for testing.
> >
> > On 5/6/24 5:32 AM, Buvaneswaran, Sujai wrote:
> >> HW offload is not supported on the i40e interface. This patch cannot
> >> be tested on i40e interface.
> >
> > To me it looks like it's supported (otherwise there is a lot of dead
> > flower code in i40e_main.c), although it's a bit limited in
> > functionality, and is called "cloud filters".
> >
> > static const struct net_device_ops i40e_netdev_ops = {
> >      [...]
> >      .ndo_setup_tc           = __i40e_setup_tc,
> >      [...]
> > };
> >
> > There is a path from __i40e_setup_tc() to i40e_parse_cls_flower(), so
> > it should be possible to test this patch.
> >
> > Most of the gatekeeping is in i40e_configure_clsflower().
> >
> > I think you should be able to get past the gatekeeping with this:
> >
> > ethtool -K $iface ntuple off
> > ethtool -K $iface hw-tc-offload on
> > tc qdisc add dev $iface ingress
> 
> One step is missing before adding the filter.
> In order to use hw_tc action, queue groups need to be created and can be
> done using
> 
> tc qdisc add dev $iface root mqprio num_tc 2 map 0 1 queues 2@0 8@2 hw 1
> mode channel
> 
> > tc filter add dev $iface protocol ip parent ffff: prio 1 flower
> > dst_mac
> > 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1
> >
> > The above filter is based on the first example in:
> >    [jkirsher/next-queue PATCH v5 6/6] i40e: Enable cloud filters via
> > tc-flower
> >
> >
> https://lore.kernel.org/netdev/150909696126.48377.794676088838721605.s
> > tgit@anamdev.jf.intel.com/
> >

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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-05-08 10:59         ` Buvaneswaran, Sujai
  0 siblings, 0 replies; 14+ messages in thread
From: Buvaneswaran, Sujai @ 2024-05-08 10:59 UTC (permalink / raw)
  To: Samudrala, Sridhar, Asbjørn Sloth Tønnesen
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	intel-wired-lan, Jakub Kicinski, Paolo Abeni, David S. Miller

Hi,

I'm able to test this patch on i40e interface from ADQ perspective. Getting 'Not supported' message when tried to configure tc rule with control flags.

[root@BP-node3-BINDU ~]# rmmod i40e
rmmod: ERROR: Module i40e is in use by: irdma
[root@BP-node3-BINDU ~]#  modprobe i40e
[root@BP-node3-BINDU ~]#  ethtool -K ens801f0np0 ntuple off
[root@BP-node3-BINDU ~]#  ethtool -K ens801f0np0 hw-tc-offload on
[root@BP-node3-BINDU ~]#  tc qdisc add dev ens801f0np0 ingress
[root@BP-node3-BINDU ~]#  tc qdisc add dev ens801f0np0 root mqprio num_tc 2 map 0 1 queues 2@0 8@2 hw 1 mode channel
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 ip_flags frag skip_sw hw_tc 1
RTNETLINK answers: Operation not supported
We have an error talking to the kernel
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 ip_flags frag/firstfrag  skip_sw hw_tc 1
RTNETLINK answers: Operation not supported
We have an error talking to the kernel
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 skip_sw hw_tc 1
[root@BP-node3-BINDU ~]# tc filter show dev ens801f0np0 root
filter parent ffff: protocol ip pref 1 flower chain 0 
filter parent ffff: protocol ip pref 1 flower chain 0 handle 0x1 hw_tc 1 
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.10
  dst_port 12000
  skip_sw
  in_hw in_hw_count 1
[root@BP-node3-BINDU ~]# tc filter add dev ens801f0np0 protocol ip parent ffff: prio 1 flower dst_ip 192.168.1.10 ip_proto tcp dst_port 12000 ip_flags frag/firstfrag hw_tc 1
[root@BP-node3-BINDU ~]# tc filter show dev ens801f0np0 root
filter parent ffff: protocol ip pref 1 flower chain 0 
filter parent ffff: protocol ip pref 1 flower chain 0 handle 0x1 hw_tc 1 
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.10
  dst_port 12000
  skip_sw
  in_hw in_hw_count 1
filter parent ffff: protocol ip pref 1 flower chain 0 handle 0x2 hw_tc 1 
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.10
  dst_port 12000
  ip_flags frag/firstfrag
  not_in_hw

Regards,
Sujai B

> -----Original Message-----
> From: Samudrala, Sridhar <sridhar.samudrala@intel.com>
> Sent: Monday, May 6, 2024 11:25 PM
> To: Asbjørn Sloth Tønnesen <ast@fiberby.net>; Buvaneswaran, Sujai
> <sujai.buvaneswaran@intel.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>;
> intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control
> flags
> 
> 
> 
> On 5/6/2024 1:44 AM, Asbjørn Sloth Tønnesen wrote:
> > Hi Sujai,
> >
> > Thank you for testing.
> >
> > On 5/6/24 5:32 AM, Buvaneswaran, Sujai wrote:
> >> HW offload is not supported on the i40e interface. This patch cannot
> >> be tested on i40e interface.
> >
> > To me it looks like it's supported (otherwise there is a lot of dead
> > flower code in i40e_main.c), although it's a bit limited in
> > functionality, and is called "cloud filters".
> >
> > static const struct net_device_ops i40e_netdev_ops = {
> >      [...]
> >      .ndo_setup_tc           = __i40e_setup_tc,
> >      [...]
> > };
> >
> > There is a path from __i40e_setup_tc() to i40e_parse_cls_flower(), so
> > it should be possible to test this patch.
> >
> > Most of the gatekeeping is in i40e_configure_clsflower().
> >
> > I think you should be able to get past the gatekeeping with this:
> >
> > ethtool -K $iface ntuple off
> > ethtool -K $iface hw-tc-offload on
> > tc qdisc add dev $iface ingress
> 
> One step is missing before adding the filter.
> In order to use hw_tc action, queue groups need to be created and can be
> done using
> 
> tc qdisc add dev $iface root mqprio num_tc 2 map 0 1 queues 2@0 8@2 hw 1
> mode channel
> 
> > tc filter add dev $iface protocol ip parent ffff: prio 1 flower
> > dst_mac
> > 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1
> >
> > The above filter is based on the first example in:
> >    [jkirsher/next-queue PATCH v5 6/6] i40e: Enable cloud filters via
> > tc-flower
> >
> >
> https://lore.kernel.org/netdev/150909696126.48377.794676088838721605.s
> > tgit@anamdev.jf.intel.com/
> >

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

* RE: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
  2024-04-16 14:43 ` [Intel-wired-lan] " Asbjørn Sloth Tønnesen
@ 2024-05-08 11:16   ` Buvaneswaran, Sujai
  -1 siblings, 0 replies; 14+ messages in thread
From: Buvaneswaran, Sujai @ 2024-05-08 11:16 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, intel-wired-lan
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	Jakub Kicinski, Paolo Abeni, David S. Miller

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Asbjørn Sloth Tønnesen
> Sent: Tuesday, April 16, 2024 8:13 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Asbjørn Sloth Tønnesen <ast@fiberby.net>;
> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> David S. Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
> 
> This driver currently doesn't support any control flags.
> 
> Use flow_rule_has_control_flags() to check for control flags, such as can be
> set through `tc flower ... ip_flags frag`.
> 
> In case any control flags are masked, flow_rule_has_control_flags() sets a NL
> extended error message, and we return -EOPNOTSUPP.
> 
> Only compile-tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
Tested-by: Sujai Buvaneswaran <sujai.buvaneswaran@intel.com>

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

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
@ 2024-05-08 11:16   ` Buvaneswaran, Sujai
  0 siblings, 0 replies; 14+ messages in thread
From: Buvaneswaran, Sujai @ 2024-05-08 11:16 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, intel-wired-lan
  Cc: netdev, linux-kernel, Eric Dumazet, Nguyen, Anthony L,
	Jakub Kicinski, Paolo Abeni, David S. Miller

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Asbjørn Sloth Tønnesen
> Sent: Tuesday, April 16, 2024 8:13 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Asbjørn Sloth Tønnesen <ast@fiberby.net>;
> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> David S. Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-next] i40e: flower: validate control flags
> 
> This driver currently doesn't support any control flags.
> 
> Use flow_rule_has_control_flags() to check for control flags, such as can be
> set through `tc flower ... ip_flags frag`.
> 
> In case any control flags are masked, flow_rule_has_control_flags() sets a NL
> extended error message, and we return -EOPNOTSUPP.
> 
> Only compile-tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
Tested-by: Sujai Buvaneswaran <sujai.buvaneswaran@intel.com>

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

end of thread, other threads:[~2024-05-08 11:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-16 14:43 [PATCH iwl-next] i40e: flower: validate control flags Asbjørn Sloth Tønnesen
2024-04-16 14:43 ` [Intel-wired-lan] " Asbjørn Sloth Tønnesen
2024-04-18 18:16 ` Simon Horman
2024-04-18 18:16   ` [Intel-wired-lan] " Simon Horman
2024-05-06  5:32 ` Buvaneswaran, Sujai
2024-05-06  5:32   ` Buvaneswaran, Sujai
2024-05-06  8:44   ` Asbjørn Sloth Tønnesen
2024-05-06  8:44     ` Asbjørn Sloth Tønnesen
2024-05-06 17:54     ` Samudrala, Sridhar
2024-05-06 17:54       ` Samudrala, Sridhar
2024-05-08 10:59       ` Buvaneswaran, Sujai
2024-05-08 10:59         ` Buvaneswaran, Sujai
2024-05-08 11:16 ` Buvaneswaran, Sujai
2024-05-08 11:16   ` Buvaneswaran, Sujai

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.