| Message ID | 20240416144320.15300-1-ast@fiberby.net |
|---|---|
| State | Under Review |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [iwl-next] i40e: flower: validate control flags | expand |
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>
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
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/
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/ >
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/ > >
> -----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>
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) {
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(+)