2024-04-16 14:44:48

by Asbjørn Sloth Tønnesen

[permalink] [raw]
Subject: [PATCH iwl-next] iavf: 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 <[email protected]>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 13361a780ece..f14355d52f47 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -3757,6 +3757,10 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,

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



2024-04-18 18:39:46

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH iwl-next] iavf: flower: validate control flags

On Tue, Apr 16, 2024 at 02:43:25PM +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 <[email protected]>

Reviewed-by: Simon Horman <[email protected]>


2024-05-06 05:31:26

by Buvaneswaran, Sujai

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate control flags

Hi Asbjørn,

HW offload is not directly supported on the iavf VF interface. VF traffic can be offloaded only through VF port representor device which uses ice driver.

[root@cbl-mariner ~]# ethtool -i ens5f0v0
driver: iavf
version: 6.9.0-rc5+
firmware-version: N/A
expansion-rom-version:
bus-info: 0000:b1:01.0
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no
[root@cbl-mariner ~]# tc qdisc add dev ens5f0v0 ingress
[root@cbl-mariner ~]# tc filter add dev ens5f0v0 ingress protocol ip flower skip_sw ip_flags frag/firstfrag action drop
Error: TC offload is disabled on net device.
We have an error talking to the kernel
[root@cbl-mariner ~]# tc filter add dev ens5f0v0 ingress protocol ip flower ip_flags frag/firstfrag action drop
[root@cbl-mariner ~]# tc filter show dev ens5f0v0 ingress
filter protocol ip pref 49152 flower chain 0
filter protocol ip pref 49152 flower chain 0 handle 0x1
eth_type ipv4
ip_flags frag/firstfrag
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 1 ref 1 bind 1

Regards,
Sujai B

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Asbjørn Sloth Tønnesen
> Sent: Tuesday, April 16, 2024 8:13 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; Eric Dumazet
> <[email protected]>; Nguyen, Anthony L
> <[email protected]>; Asbjørn Sloth Tønnesen <[email protected]>;
> Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>;
> David S. Miller <[email protected]>
> Subject: [Intel-wired-lan] [PATCH iwl-next] iavf: 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 <[email protected]>
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 13361a780ece..f14355d52f47 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -3757,6 +3757,10 @@ static int iavf_parse_cls_flower(struct
> iavf_adapter *adapter,
>
> 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

2024-05-06 09:20:47

by Asbjørn Sloth Tønnesen

[permalink] [raw]
Subject: Re: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate control flags

Hi Sujai,

Thank you for testing.

On 5/6/24 5:30 AM, Buvaneswaran, Sujai wrote:
> HW offload is not directly supported on the iavf VF interface. VF traffic can be offloaded only through VF port representor device which uses ice driver.

Again, there is a awful lot of flower code in iavf_main.c, if it's not supported.

Try with:
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

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

2024-05-06 09:34:06

by Buvaneswaran, Sujai

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate control flags

Hi Asbjørn,

I have tried the above steps as well and still issue is seen while enabling HW offload on iavf interface.

[root@cbl-mariner ~]# ethtool -K ens5f0v0 hw-tc-offload on
Actual changes:
hw-tc-offload: off [requested on]
Could not change any device features
[root@cbl-mariner ~]# tc qdisc add dev ens5f0v0 ingress
[root@cbl-mariner ~]# tc filter add dev ens5f0v0 protocol ip parent ffff: prio 1 flower dst_mac 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1
Error: TC offload is disabled on net device.
We have an error talking to the kernel

Thanks,
Sujai B

> -----Original Message-----
> From: Asbjørn Sloth Tønnesen <[email protected]>
> Sent: Monday, May 6, 2024 2:50 PM
> To: Buvaneswaran, Sujai <[email protected]>; intel-wired-
> [email protected]
> Cc: [email protected]; [email protected]; Eric Dumazet
> <[email protected]>; Nguyen, Anthony L
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> Abeni <[email protected]>; David S. Miller <[email protected]>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate control
> flags
>
> Hi Sujai,
>
> Thank you for testing.
>
> On 5/6/24 5:30 AM, Buvaneswaran, Sujai wrote:
> > HW offload is not directly supported on the iavf VF interface. VF traffic can
> be offloaded only through VF port representor device which uses ice driver.
>
> Again, there is a awful lot of flower code in iavf_main.c, if it's not supported.
>
> Try with:
> 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
>
> --
> Best regards
> Asbjørn Sloth Tønnesen
> Network Engineer
> Fiberby - AS42541

2024-05-08 11:15:51

by Buvaneswaran, Sujai

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate control flags

I have tested this patch from ADQ perspective and it is working fine.

[root@BP-node3-BINDU ~]# tc filter add dev ens801f0v0 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 ens801f0v0 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 ens801f0v0 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 ens801f0v0 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 ens801f0v0 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 ens801f0v0 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

Thanks,
Sujai B

> -----Original Message-----
> From: Buvaneswaran, Sujai
> Sent: Monday, May 6, 2024 3:03 PM
> To: Asbjørn Sloth Tønnesen <[email protected]>; intel-wired-
> [email protected]
> Cc: [email protected]; [email protected]; Eric Dumazet
> <[email protected]>; Nguyen, Anthony L
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> Abeni <[email protected]>; David S. Miller <[email protected]>
> Subject: RE: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate control
> flags
>
> Hi Asbjørn,
>
> I have tried the above steps as well and still issue is seen while enabling HW
> offload on iavf interface.
>
> [root@cbl-mariner ~]# ethtool -K ens5f0v0 hw-tc-offload on Actual changes:
> hw-tc-offload: off [requested on]
> Could not change any device features
> [root@cbl-mariner ~]# tc qdisc add dev ens5f0v0 ingress [root@cbl-mariner
> ~]# tc filter add dev ens5f0v0 protocol ip parent ffff: prio 1 flower dst_mac
> 3c:fd:fe:a0:d6:70 ip_flags frag skip_sw hw_tc 1
> Error: TC offload is disabled on net device.
> We have an error talking to the kernel
>
> Thanks,
> Sujai B
>
> > -----Original Message-----
> > From: Asbjørn Sloth Tønnesen <[email protected]>
> > Sent: Monday, May 6, 2024 2:50 PM
> > To: Buvaneswaran, Sujai <[email protected]>; intel-wired-
> > [email protected]
> > Cc: [email protected]; [email protected]; Eric Dumazet
> > <[email protected]>; Nguyen, Anthony L
> <[email protected]>;
> > Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>;
> > David S. Miller <[email protected]>
> > Subject: Re: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate
> > control flags
> >
> > Hi Sujai,
> >
> > Thank you for testing.
> >
> > On 5/6/24 5:30 AM, Buvaneswaran, Sujai wrote:
> > > HW offload is not directly supported on the iavf VF interface. VF
> > > traffic can
> > be offloaded only through VF port representor device which uses ice driver.
> >
> > Again, there is a awful lot of flower code in iavf_main.c, if it's not
> supported.
> >
> > Try with:
> > 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
> >
> > --
> > Best regards
> > Asbjørn Sloth Tønnesen
> > Network Engineer
> > Fiberby - AS42541

2024-05-08 11:17:19

by Buvaneswaran, Sujai

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH iwl-next] iavf: flower: validate control flags

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Asbjørn Sloth Tønnesen
> Sent: Tuesday, April 16, 2024 8:13 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; Eric Dumazet
> <[email protected]>; Nguyen, Anthony L
> <[email protected]>; Asbjørn Sloth Tønnesen <[email protected]>;
> Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>;
> David S. Miller <[email protected]>
> Subject: [Intel-wired-lan] [PATCH iwl-next] iavf: 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 <[email protected]>
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
Tested-by: Sujai Buvaneswaran <[email protected]>