@@ -607,16 +633,45 @@ static int tc_del_flow(struct stmmac_priv *priv,returnret;}+staticstructstmmac_rfs_entry*tc_find_rfs(structstmmac_priv*priv,+structflow_cls_offload*cls,+boolget_free)+{+inti;++for(i=0;i<priv->rfs_entries_total;i++){+structstmmac_rfs_entry*entry=&priv->rfs_entries[i];++if(entry->cookie==cls->cookie)+returnentry;+if(get_free&&entry->in_use==false)+returnentry;+}++returnNULL;+}+#define VLAN_PRIO_FULL_MASK (0x07)staticinttc_add_vlan_flow(structstmmac_priv*priv,structflow_cls_offload*cls){+structstmmac_rfs_entry*entry=tc_find_rfs(priv,cls,false);structflow_rule*rule=flow_cls_offload_flow_rule(cls);structflow_dissector*dissector=rule->match.dissector;inttc=tc_classid_to_hwtc(priv->dev,cls->classid);structflow_match_vlanmatch;+if(!entry){+entry=tc_find_rfs(priv,cls,true);+if(!entry)+return-ENOENT;+}++if(priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN]>=+priv->rfs_entries_max[STMMAC_RFS_T_VLAN])+return-ENOENT;+/* Nothing to do here */if(!dissector_uses_key(dissector,FLOW_DISSECTOR_KEY_VLAN))return-EINVAL;
@@ -638,6 +693,12 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,prio=BIT(match.key->vlan_priority);stmmac_rx_queue_prio(priv,priv->hw,prio,tc);++entry->in_use=true;+entry->cookie=cls->cookie;+entry->tc=tc;+entry->type=STMMAC_RFS_T_VLAN;+priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN]++;}return0;
@@ -646,20 +707,19 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,staticinttc_del_vlan_flow(structstmmac_priv*priv,structflow_cls_offload*cls){-structflow_rule*rule=flow_cls_offload_flow_rule(cls);-structflow_dissector*dissector=rule->match.dissector;-inttc=tc_classid_to_hwtc(priv->dev,cls->classid);+structstmmac_rfs_entry*entry=tc_find_rfs(priv,cls,false);-/* Nothing to do here */-if(!dissector_uses_key(dissector,FLOW_DISSECTOR_KEY_VLAN))-return-EINVAL;+if(!entry||!entry->in_use||entry->type!=STMMAC_RFS_T_VLAN)+return-ENOENT;-if(tc<0){-netdev_err(priv->dev,"Invalid traffic class\n");-return-EINVAL;-}+stmmac_rx_queue_prio(priv,priv->hw,0,entry->tc);++entry->in_use=false;+entry->cookie=0;+entry->tc=0;+entry->type=0;-stmmac_rx_queue_prio(priv,priv->hw,0,tc);+priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN]--;return0;}
From: Ong Boon Leong <hidden> Date: 2021-12-09 15:22:31
This patch adds basic support for EtherType RX frame steering for
LLDP and PTP using the hardware offload capabilities.
Signed-off-by: Ong Boon Leong <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 +
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 121 ++++++++++++++++++
2 files changed, 124 insertions(+)
@@ -464,6 +468,7 @@ static int tc_add_basic_flow(struct stmmac_priv *priv,return-EINVAL;flow_rule_match_basic(rule,&match);+entry->ip_proto=match.key->ip_proto;return0;}
@@ -724,6 +729,114 @@ static int tc_del_vlan_flow(struct stmmac_priv *priv,return0;}+staticinttc_add_ethtype_flow(structstmmac_priv*priv,+structflow_cls_offload*cls)+{+structstmmac_rfs_entry*entry=tc_find_rfs(priv,cls,false);+structflow_rule*rule=flow_cls_offload_flow_rule(cls);+structflow_dissector*dissector=rule->match.dissector;+inttc=tc_classid_to_hwtc(priv->dev,cls->classid);+structflow_match_basicmatch;++if(!entry){+entry=tc_find_rfs(priv,cls,true);+if(!entry)+return-ENOENT;+}++/* Nothing to do here */+if(!dissector_uses_key(dissector,FLOW_DISSECTOR_KEY_BASIC))+return-EINVAL;++if(tc<0){+netdev_err(priv->dev,"Invalid traffic class\n");+return-EINVAL;+}++flow_rule_match_basic(rule,&match);++if(match.mask->n_proto){+__be16etype=ntohs(match.key->n_proto);++if(match.mask->n_proto!=ETHER_TYPE_FULL_MASK){+netdev_err(priv->dev,"Only full mask is supported for EthType filter");+return-EINVAL;+}+switch(etype){+caseETH_P_LLDP:+if(priv->rfs_entries_cnt[STMMAC_RFS_T_LLDP]>=+priv->rfs_entries_max[STMMAC_RFS_T_LLDP])+return-ENOENT;++entry->type=STMMAC_RFS_T_LLDP;+priv->rfs_entries_cnt[STMMAC_RFS_T_LLDP]++;++stmmac_rx_queue_routing(priv,priv->hw,+PACKET_DCBCPQ,tc);+break;+caseETH_P_1588:+if(priv->rfs_entries_cnt[STMMAC_RFS_T_1588]>=+priv->rfs_entries_max[STMMAC_RFS_T_1588])+return-ENOENT;++entry->type=STMMAC_RFS_T_1588;+priv->rfs_entries_cnt[STMMAC_RFS_T_1588]++;++stmmac_rx_queue_routing(priv,priv->hw,+PACKET_PTPQ,tc);+break;+default:+netdev_err(priv->dev,"EthType(0x%x) is not supported",etype);+return-EINVAL;+}++entry->in_use=true;+entry->cookie=cls->cookie;+entry->tc=tc;+entry->etype=etype;++return0;+}++return-EINVAL;+}++staticinttc_del_ethtype_flow(structstmmac_priv*priv,+structflow_cls_offload*cls)+{+structstmmac_rfs_entry*entry=tc_find_rfs(priv,cls,false);++if(!entry||!entry->in_use||+entry->type<STMMAC_RFS_T_LLDP||+entry->type>STMMAC_RFS_T_1588)+return-ENOENT;++switch(entry->etype){+caseETH_P_LLDP:+stmmac_rx_queue_routing(priv,priv->hw,+PACKET_DCBCPQ,0);+priv->rfs_entries_cnt[STMMAC_RFS_T_LLDP]--;+break;+caseETH_P_1588:+stmmac_rx_queue_routing(priv,priv->hw,+PACKET_PTPQ,0);+priv->rfs_entries_cnt[STMMAC_RFS_T_1588]--;+break;+default:+netdev_err(priv->dev,"EthType(0x%x) is not supported",+entry->etype);+return-EINVAL;+}++entry->in_use=false;+entry->cookie=0;+entry->tc=0;+entry->etype=0;+entry->type=0;++return0;+}+staticinttc_add_flow_cls(structstmmac_priv*priv,structflow_cls_offload*cls){
@@ -733,6 +846,10 @@ static int tc_add_flow_cls(struct stmmac_priv *priv,if(!ret)returnret;+ret=tc_add_ethtype_flow(priv,cls);+if(!ret)+returnret;+returntc_add_vlan_flow(priv,cls);}
@@ -745,6 +862,10 @@ static int tc_del_flow_cls(struct stmmac_priv *priv,if(!ret)returnret;+ret=tc_del_ethtype_flow(priv,cls);+if(!ret)+returnret;+returntc_del_vlan_flow(priv,cls);}
You submitted this patch to net as well. I guess, it should be merged to
net. After net is merged into net-next we can proceed with the EtherType
steering?
Thanks,
Kurt
From: Kurt Kanzenbach <hidden> Date: 2021-12-10 09:37:25
Hi BL,
On Thu Dec 09 2021, Ong Boon Leong wrote:
This patch adds basic support for EtherType RX frame steering for
LLDP and PTP using the hardware offload capabilities.
Maybe add an example here for users?
|tc filter add dev eno1 parent ffff: protocol 0x88f7 flower hw_tc 4
|tc filter add dev eno1 parent ffff: protocol 0x88cc flower hw_tc 4
Signed-off-by: Ong Boon Leong <redacted>
Something is not quite correct. The use of the etype variable generates
new warnings. For instance:
|drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:768:25: warning: restricted __be16 degrades to integer
|drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:768:25: warning: restricted __be16 degrades to integer
|drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:817:22: warning: restricted __be16 degrades to integer
|drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:817:22: warning: restricted __be16 degrades to integer
However, the steering works as expected. Thanks!
Thanks,
Kurt
From: Kurt Kanzenbach <hidden> Date: 2021-12-10 10:11:51
On Thu Dec 09 2021, Ong Boon Leong wrote:
This patch adds basic support for EtherType RX frame steering for
LLDP and PTP using the hardware offload capabilities.
Signed-off-by: Ong Boon Leong <redacted>
[snip]
+ if (match.mask->n_proto) {
+ __be16 etype = ntohs(match.key->n_proto);
n_proto is be16. The ntohs() call will produce an u16.
Delta patch below.
Thanks,
Kurt
@@ -759,7 +759,7 @@ static int tc_add_ethtype_flow(struct stmmac_priv *priv,flow_rule_match_basic(rule,&match);if(match.mask->n_proto){-__be16etype=ntohs(match.key->n_proto);+u16etype=ntohs(match.key->n_proto);if(match.mask->n_proto!=ETHER_TYPE_FULL_MASK){netdev_err(priv->dev,"Only full mask is supported for EthType filter");
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-12-10 11:59:03
Hi David, Jakub,
On Thu, Dec 09, 2021 at 11:16:29PM +0800, Ong Boon Leong wrote:
Hi,
Patch 1/2: Fixes issue in tc filter delete flower for VLAN priority
steering. Patch has been sent to 'net' ML. Link as follow:
https://patchwork.kernel.org/project/netdevbpf/patch/20211209130335.81114-1-boon.leong.ong@intel.com/
Patch 2/2: Patch to add LLDP and IEEE1588 EtherType RX frame steering
in tc flower that is implemented on-top of patch 1/2.
Below are the test steps for checking out the newly added feature:-
# Setup traffic class and ingress filter
$ IFDEVNAME=eth0
$ tc qdisc add dev $IFDEVNAME ingress
$ tc qdisc add dev $IFDEVNAME root mqprio num_tc 8 \
map 0 1 2 3 4 5 6 7 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 hw 0
# Add two VLAN priority based RX Frame Steering
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 1 hw_tc 1
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 2 hw_tc 2
# For LLDP
$ tc filter add dev $IFDEVNAME parent ffff: protocol 0x88cc \
flower hw_tc 5
# For PTP
$ tc filter add dev $IFDEVNAME parent ffff: protocol 0x88f7 \
flower hw_tc 6
# Show the ingress tc filters
$ tc filter show dev $IFDEVNAME ingress
filter parent ffff: protocol ptp pref 49149 flower chain 0
filter parent ffff: protocol ptp pref 49149 flower chain 0 handle 0x1 hw_tc 6
eth_type 88f7
in_hw in_hw_count 1
filter parent ffff: protocol LLDP pref 49150 flower chain 0
filter parent ffff: protocol LLDP pref 49150 flower chain 0 handle 0x1 hw_tc 5
eth_type 88cc
in_hw in_hw_count 1
filter parent ffff: protocol 802.1Q pref 49151 flower chain 0
filter parent ffff: protocol 802.1Q pref 49151 flower chain 0 handle 0x1 hw_tc 2
vlan_prio 2
in_hw in_hw_count 1
filter parent ffff: protocol 802.1Q pref 49152 flower chain 0
filter parent ffff: protocol 802.1Q pref 49152 flower chain 0 handle 0x1 hw_tc 1
vlan_prio 1
in_hw in_hw_count 1
# Delete tc filters
$ tc filter del dev $IFDEVNAME parent ffff: pref 49149
$ tc filter del dev $IFDEVNAME parent ffff: pref 49150
$ tc filter del dev $IFDEVNAME parent ffff: pref 49151
$ tc filter del dev $IFDEVNAME parent ffff: pref 49152
Thanks,
BL
Ong Boon Leong (2):
net: stmmac: fix tc flower deletion for VLAN priority Rx steering
net: stmmac: add tc flower filter for EtherType matching
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 20 ++
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 189 +++++++++++++++++-
2 files changed, 205 insertions(+), 4 deletions(-)
--
2.25.1
Is it the canonical approach to perform flow steering via tc-flower hw_tc,
as opposed to ethtool --config-nfc? My understanding from reading the
documentation is that tc-flower hw_tc only selects the hardware traffic
class for a packet, and that this has to do with prioritization
(although the concept in itself is a bit ill-defined as far as I
understand it, how does it relate to things like offloaded skbedit priority?).
But selecting a traffic class, in itself, doesn't (directly or
necessarily) select a ring per se, as ethtool does? Just like ethtool
doesn't select packet priority, just RX queue. When the RX queue
priority is configurable (see the "snps,priority" device tree property
in stmmac_mtl_setup) and more RX queues have the same priority, I'm not
sure what hw_tc is supposed to do in terms of RX queue selection?
While we're at it, shouldn't we also check that no actions are being requested and fail if there are, instead of silently ignoring them?
quoted hunk
+ if (!entry) {+ entry = tc_find_rfs(priv, cls, true);+ if (!entry)+ return -ENOENT;+ }++ if (priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN] >=+ priv->rfs_entries_max[STMMAC_RFS_T_VLAN])+ return -ENOENT;+ /* Nothing to do here */ if (!dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_VLAN)) return -EINVAL;
I was about to post a very similar fix for that same problem (except I was adding support for other packet steering types)...
I can confirm your patch works. Note that a simpler way to reproduce is simply to add a filter, then remove all the filters, e.g.:
$ IFDEVNAME=eth0
$ tc qdisc add dev $IFDEVNAME ingress
$ tc qdisc add dev $IFDEVNAME root mqprio num_tc 8 \
map 0 1 2 3 4 5 6 7 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 hw 0
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 0 hw_tc 0
$ tc filter del dev $IFDEVNAME ingress
Yannick
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-12-10 19:39:42
On Fri, 10 Dec 2021 13:57:30 +0200 Vladimir Oltean wrote:
Is it the canonical approach to perform flow steering via tc-flower hw_tc,
as opposed to ethtool --config-nfc? My understanding from reading the
documentation is that tc-flower hw_tc only selects the hardware traffic
class for a packet, and that this has to do with prioritization
(although the concept in itself is a bit ill-defined as far as I
understand it, how does it relate to things like offloaded skbedit priority?).
But selecting a traffic class, in itself, doesn't (directly or
necessarily) select a ring per se, as ethtool does? Just like ethtool
doesn't select packet priority, just RX queue. When the RX queue
priority is configurable (see the "snps,priority" device tree property
in stmmac_mtl_setup) and more RX queues have the same priority, I'm not
sure what hw_tc is supposed to do in terms of RX queue selection?
You didn't mention the mqprio, but I think that's the piece that maps
TCs to queue pairs. You can have multiple queues in a TC.
Obviously that's still pretty weird what the flow rules should select
is an RSS context. mqprio is a qdisc, which means Tx, not Rx.
Adding Amritha who I believe added the concept of selecting Rx queues
via hw_tc. Can you comment?
-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Friday, December 10, 2021 11:38 AM
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Ong, Boon Leong <redacted>; David S . Miller
[off-list ref]; Giuseppe Cavallaro [off-list ref];
Alexandre Torgue [off-list ref]; Jose Abreu
[off-list ref]; Maxime Coquelin
[off-list ref]; alexandre.torgue@foss.st.com;
Kanzenbach, Kurt [off-list ref];
netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com;
linux-arm-kernel@lists.infradead.org; Nambiar, Amritha
[off-list ref]
Subject: Re: [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame
steering
On Fri, 10 Dec 2021 13:57:30 +0200 Vladimir Oltean wrote:
quoted
Is it the canonical approach to perform flow steering via tc-flower hw_tc,
as opposed to ethtool --config-nfc? My understanding from reading the
documentation is that tc-flower hw_tc only selects the hardware traffic
class for a packet, and that this has to do with prioritization
(although the concept in itself is a bit ill-defined as far as I
understand it, how does it relate to things like offloaded skbedit priority?).
But selecting a traffic class, in itself, doesn't (directly or
necessarily) select a ring per se, as ethtool does? Just like ethtool
doesn't select packet priority, just RX queue. When the RX queue
priority is configurable (see the "snps,priority" device tree property
in stmmac_mtl_setup) and more RX queues have the same priority, I'm not
sure what hw_tc is supposed to do in terms of RX queue selection?
You didn't mention the mqprio, but I think that's the piece that maps
TCs to queue pairs. You can have multiple queues in a TC.
Obviously that's still pretty weird what the flow rules should select
is an RSS context. mqprio is a qdisc, which means Tx, not Rx.
Adding Amritha who I believe added the concept of selecting Rx queues
via hw_tc. Can you comment?
So tc-mpqrio is the piece that is needed to set up the queue-groups. The offload
mode "hw 2" in mqprio will offload the TCs, the queue configurations and
bandwidth rate limits. The prio-tc map in mqprio will map a user priority to the
TC/queue-group. The priority to traffic class mapping and the user specified
queue ranges are used to configure the traffic class when the 'hw' option is set to 2.
Drivers can then configure queue-pairs based on the offsets and queue ranges
in mqprio.
The hw_tc option in tc-flower for ingress filter is used to direct Rx traffic to the
queue-group (configured via mqprio). Queue selection within the queue group can
be achieved using RSS.
I agree mqprio qdisc should be used to set up Tx queues only, but the limitation was the
absence of a single interface that could configure both Tx and Rx queue-groups/queue-sets
(ethtool did not support directing flows to a queue-group, but only a specific individual
queue, TC does not support Rx queue-group configuration either). The hw_tc in mqprio is a
range of class ids reserved to identify hardware traffic classes normally reported
via dev->num_tc. For Rx queue-group configuration, the gap is that the ingress/clsact qdisc
does not expose a set of virtual qdiscs similar to HW traffic classes in mqprio.
This was discussed in Slide 20 from Netdev 0x14
(https://legacy.netdevconf.info/0x14/pub/slides/28/Application%20Device%20Queues%20for%20system-level%20network%20IO%20performance%20improvements.pdf)
-Amritha
You submitted this patch to net as well. I guess, it should be merged to
net. After net is merged into net-next we can proceed with the EtherType
steering?
Yes, my intention is to make sure anyone who wants to the EthType steering
will be aware of this patch. That is why I am sending the patch in both
net and net-next with a cover letter to inform about the dependency.
I was about to post a very similar fix for that same problem (except I
was adding support for other packet steering types)...
I can confirm your patch works.
Thanks for testing it.
Note that a simpler way to reproduce is
simply to add a filter, then remove all the filters, e.g.:
$ IFDEVNAME=eth0
$ tc qdisc add dev $IFDEVNAME ingress
$ tc qdisc add dev $IFDEVNAME root mqprio num_tc 8 \
map 0 1 2 3 4 5 6 7 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 hw 0
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 0 hw_tc 0
$ tc filter del dev $IFDEVNAME ingress
@@ -759,7 +759,7 @@ static int tc_add_ethtype_flow(struct stmmac_priv
*priv,
flow_rule_match_basic(rule, &match);
if (match.mask->n_proto) {
- __be16 etype = ntohs(match.key->n_proto);
+ u16 etype = ntohs(match.key->n_proto);
if (match.mask->n_proto != ETHER_TYPE_FULL_MASK) {
netdev_err(priv->dev, "Only full mask is supported for EthType
filter");
Thanks for the suggestion. I will incorporate in v2 patch after we conclude
if the tc flower hw_tc interface used for specifying RxQ queue is agreeable
by community.