* [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering
2021-12-09 15:16 [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering Ong Boon Leong
@ 2021-12-09 15:16 ` Ong Boon Leong
2021-12-10 9:25 ` Kurt Kanzenbach
2021-12-10 16:30 ` Yannick Vignon
2021-12-09 15:16 ` [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching Ong Boon Leong
2021-12-10 11:57 ` [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering Vladimir Oltean
2 siblings, 2 replies; 16+ messages in thread
From: Ong Boon Leong @ 2021-12-09 15:16 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue, Kurt Kanzenbach
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong
To replicate the issue:-
1) Add 2 flower filters for VLAN Priority based frame steering:-
$ 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 add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 1 hw_tc 1
2) Get the 'pref' id
$ tc filter show dev $IFDEVNAME ingress
3) Delete a specific tc flower record
$ tc filter del dev $IFDEVNAME parent ffff: pref 49151
From dmesg, we will observe kernel NULL pointer ooops
[ 197.170464] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 197.171367] #PF: supervisor read access in kernel mode
[ 197.171367] #PF: error_code(0x0000) - not-present page
[ 197.171367] PGD 0 P4D 0
[ 197.171367] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 197.171367] CPU: 0 PID: 3216 Comm: tc Tainted: G U E 5.16.0-rc2+ #12
[ 197.171367] Hardware name: Intel Corporation Elkhart Lake Embedded Platform/ElkhartLake LPDDR4x T3 CRB, BIOS EHLSFWI1.R00.3273.A04.2107240322 07/24/2021
[ 197.171367] RIP: 0010:tc_setup_cls+0x20b/0x4a0 [stmmac]
[ 197.171367] Code: fe ff ff c7 43 14 00 00 00 00 48 c7 03 00 00 00 00 c7 43 1c 00 00 00 00 49 8b 44 24 28 48 8b bd b0 00 00 00 41 0f b7 54 24 58 <48> 8b 00 0f bf 8f 38 08 00 00 81 ea e0 ff 00 00 8b 00 25 00 04 00
[ 197.171367] RSP: 0018:ffff940940a037c0 EFLAGS: 00010246
[ 197.171367] RAX: 0000000000000000 RBX: ffff92e826cae2c8 RCX: ffff92e825f39000
[ 197.171367] RDX: 0000000000000000 RSI: ffff92e826cae2a8 RDI: ffff92e82f0c0000
[ 197.171367] RBP: ffff92e82f0c0940 R08: 0000000000000000 R09: ffff92e825f39434
[ 197.171367] R10: ffff92e826c5af00 R11: ffff940940a038a8 R12: ffff940940a038a8
[ 197.171367] R13: 0000000000000000 R14: 0000000000000000 R15: ffff92e830a5b600
[ 197.171367] FS: 00007fa7b0c47740(0000) GS:ffff92e964200000(0000) knlGS:0000000000000000
[ 197.171367] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 197.171367] CR2: 0000000000000000 CR3: 0000000124c50000 CR4: 0000000000350ef0
[ 197.171367] Call Trace:
[ 197.171367] <TASK>
[ 197.171367] ? __stmmac_disable_all_queues+0xa8/0xe0 [stmmac]
[ 197.171367] stmmac_setup_tc_block_cb+0x70/0x110 [stmmac]
[ 197.171367] tc_setup_cb_destroy+0xb3/0x180
[ 197.171367] fl_hw_destroy_filter+0x94/0xc0 [cls_flower]
[ 197.171367] __fl_delete+0x16a/0x180 [cls_flower]
[ 197.171367] fl_destroy+0xb9/0x140 [cls_flower]
[ 197.171367] tcf_proto_destroy+0x1d/0xa0
[ 197.171367] tc_del_tfilter+0x3c9/0x7b0
[ 197.171367] ? tc_dump_tfilter+0x310/0x310
[ 197.171367] rtnetlink_rcv_msg+0x2bf/0x370
[ 197.171367] ? preempt_count_add+0x68/0xa0
[ 197.171367] ? _raw_spin_lock_irqsave+0x19/0x40
[ 197.171367] ? _raw_spin_unlock_irqrestore+0x1f/0x31
[ 197.171367] ? rtnl_calcit.isra.0+0x130/0x130
[ 197.171367] netlink_rcv_skb+0x4e/0x100
[ 197.171367] netlink_unicast+0x18e/0x230
[ 197.171367] netlink_sendmsg+0x245/0x480
[ 197.171367] sock_sendmsg+0x5b/0x60
[ 197.171367] ____sys_sendmsg+0x20b/0x280
[ 197.171367] ? copy_msghdr_from_user+0x5c/0x90
[ 197.171367] ___sys_sendmsg+0x7c/0xc0
[ 197.171367] ? folio_add_lru+0x52/0x80
[ 197.171367] ? __sys_sendto+0xee/0x160
[ 197.171367] __sys_sendmsg+0x59/0xa0
[ 197.171367] do_syscall_64+0x40/0x90
[ 197.171367] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 197.171367] RIP: 0033:0x7fa7b0d64397
[ 197.171367] Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
[ 197.171367] RSP: 002b:00007ffdd88b58e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 197.171367] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fa7b0d64397
[ 197.171367] RDX: 0000000000000000 RSI: 00007ffdd88b5960 RDI: 0000000000000003
[ 197.171367] RBP: 0000000061b05c21 R08: 0000000000000001 R09: 0000564584e47890
[ 197.171367] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 197.171367] R13: 00007ffdd88b9a80 R14: 00000000bfff0000 R15: 0000564584e3e420
[ 197.171367] </TASK>
[ 197.171367] Modules linked in: cls_flower sch_mqprio sch_ingress dwmac_intel(E) stmmac(E) pcs_xpcs phylink marvell marvell10g libphy 8021q bnep bluetooth ecryptfs nfsd sch_fq_codel uio uhid snd_soc_dmic snd_sof_pci_intel_tgl x86_pkg_temp_thermal snd_sof_intel_hda_common kvm_intel iTCO_wdt iTCO_vendor_support soundwire_intel mei_hdcp kvm soundwire_generic_allocation soundwire_cadence soundwire_bus irqbypass snd_sof_xtensa_dsp ax88179_178a snd_soc_acpi_intel_match intel_rapl_msr pcspkr usbnet snd_soc_acpi mii snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec i2c_i801 snd_hda_core intel_ish_ipc tpm_crb 8250_lpss intel_ishtp tpm_tis i915 mei_me i2c_smbus mei tpm_tis_core dw_dmac_core tpm spi_dw_pci parport_pc intel_pmc_core spi_dw thermal parport ttm fuse configfs snd_sof_pci snd_sof snd_soc_core snd_compress ac97_bus ledtrig_audio snd_pcm snd_timer snd soundcore [last unloaded: libphy]
[ 197.171367] CR2: 0000000000000000
[ 197.171367] ---[ end trace 8b8d1c617c39093d ]---
This patch reimplements the tc flower rx frame steering for VLAN priority
by keeping a record of flow_cls_offload added. The implementation also
makes way to support EtherType based RX frame steering later.
Fixes: 0e039f5cf86c ("net: stmmac: add RX frame steering based on VLAN priority in tc flower")
Tested-by: Kurt Kanzenbach <kurt@linutronix.de>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 17 ++++
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 86 ++++++++++++++++---
2 files changed, 90 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 4f5292cadf5..18a262ef17f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -171,6 +171,19 @@ struct stmmac_flow_entry {
int is_l4;
};
+/* Rx Frame Steering */
+enum stmmac_rfs_type {
+ STMMAC_RFS_T_VLAN,
+ STMMAC_RFS_T_MAX,
+};
+
+struct stmmac_rfs_entry {
+ unsigned long cookie;
+ int in_use;
+ int type;
+ int tc;
+};
+
struct stmmac_priv {
/* Frequently used values are kept adjacent for cache effect */
u32 tx_coal_frames[MTL_MAX_TX_QUEUES];
@@ -288,6 +301,10 @@ struct stmmac_priv {
struct stmmac_tc_entry *tc_entries;
unsigned int flow_entries_max;
struct stmmac_flow_entry *flow_entries;
+ unsigned int rfs_entries_max[STMMAC_RFS_T_MAX];
+ unsigned int rfs_entries_cnt[STMMAC_RFS_T_MAX];
+ unsigned int rfs_entries_total;
+ struct stmmac_rfs_entry *rfs_entries;
/* Pulse Per Second output */
struct stmmac_pps_cfg pps[STMMAC_PPS_MAX];
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 1c4ea0b1b84..d0a2b289f46 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -232,11 +232,33 @@ static int tc_setup_cls_u32(struct stmmac_priv *priv,
}
}
+static int tc_rfs_init(struct stmmac_priv *priv)
+{
+ int i;
+
+ priv->rfs_entries_max[STMMAC_RFS_T_VLAN] = 8;
+
+ for (i = 0; i < STMMAC_RFS_T_MAX; i++)
+ priv->rfs_entries_total += priv->rfs_entries_max[i];
+
+ priv->rfs_entries = devm_kcalloc(priv->device,
+ priv->rfs_entries_total,
+ sizeof(*priv->rfs_entries),
+ GFP_KERNEL);
+ if (!priv->rfs_entries)
+ return -ENOMEM;
+
+ dev_info(priv->device, "Enabled RFS Flow TC (entries=%d)\n",
+ priv->rfs_entries_total);
+
+ return 0;
+}
+
static int tc_init(struct stmmac_priv *priv)
{
struct dma_features *dma_cap = &priv->dma_cap;
unsigned int count;
- int i;
+ int ret, i;
if (dma_cap->l3l4fnum) {
priv->flow_entries_max = dma_cap->l3l4fnum;
@@ -250,10 +272,14 @@ static int tc_init(struct stmmac_priv *priv)
for (i = 0; i < priv->flow_entries_max; i++)
priv->flow_entries[i].idx = i;
- dev_info(priv->device, "Enabled Flow TC (entries=%d)\n",
+ dev_info(priv->device, "Enabled L3L4 Flow TC (entries=%d)\n",
priv->flow_entries_max);
}
+ ret = tc_rfs_init(priv);
+ if (ret)
+ return -ENOMEM;
+
if (!priv->plat->fpe_cfg) {
priv->plat->fpe_cfg = devm_kzalloc(priv->device,
sizeof(*priv->plat->fpe_cfg),
@@ -607,16 +633,45 @@ static int tc_del_flow(struct stmmac_priv *priv,
return ret;
}
+static struct stmmac_rfs_entry *tc_find_rfs(struct stmmac_priv *priv,
+ struct flow_cls_offload *cls,
+ bool get_free)
+{
+ int i;
+
+ for (i = 0; i < priv->rfs_entries_total; i++) {
+ struct stmmac_rfs_entry *entry = &priv->rfs_entries[i];
+
+ if (entry->cookie == cls->cookie)
+ return entry;
+ if (get_free && entry->in_use == false)
+ return entry;
+ }
+
+ return NULL;
+}
+
#define VLAN_PRIO_FULL_MASK (0x07)
static int tc_add_vlan_flow(struct stmmac_priv *priv,
struct flow_cls_offload *cls)
{
+ struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
struct flow_dissector *dissector = rule->match.dissector;
int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
struct flow_match_vlan match;
+ 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]++;
}
return 0;
@@ -646,20 +707,19 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,
static int tc_del_vlan_flow(struct stmmac_priv *priv,
struct flow_cls_offload *cls)
{
- struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
- struct flow_dissector *dissector = rule->match.dissector;
- int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
+ struct stmmac_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]--;
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering
2021-12-09 15:16 ` [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering Ong Boon Leong
@ 2021-12-10 9:25 ` Kurt Kanzenbach
2021-12-11 13:51 ` Ong, Boon Leong
2021-12-10 16:30 ` Yannick Vignon
1 sibling, 1 reply; 16+ messages in thread
From: Kurt Kanzenbach @ 2021-12-10 9:25 UTC (permalink / raw)
To: Ong Boon Leong, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong,
Sebastian Andrzej Siewior
[-- Attachment #1: Type: text/plain, Size: 6034 bytes --]
Hi BL,
On Thu Dec 09 2021, Ong Boon Leong wrote:
> To replicate the issue:-
>
> 1) Add 2 flower filters for VLAN Priority based frame steering:-
> $ 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 add dev $IFDEVNAME parent ffff: protocol 802.1Q \
> flower vlan_prio 1 hw_tc 1
>
> 2) Get the 'pref' id
> $ tc filter show dev $IFDEVNAME ingress
>
> 3) Delete a specific tc flower record
> $ tc filter del dev $IFDEVNAME parent ffff: pref 49151
>
> From dmesg, we will observe kernel NULL pointer ooops
>
> [ 197.170464] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [ 197.171367] #PF: supervisor read access in kernel mode
> [ 197.171367] #PF: error_code(0x0000) - not-present page
> [ 197.171367] PGD 0 P4D 0
> [ 197.171367] Oops: 0000 [#1] PREEMPT SMP NOPTI
> [ 197.171367] CPU: 0 PID: 3216 Comm: tc Tainted: G U E 5.16.0-rc2+ #12
> [ 197.171367] Hardware name: Intel Corporation Elkhart Lake Embedded Platform/ElkhartLake LPDDR4x T3 CRB, BIOS EHLSFWI1.R00.3273.A04.2107240322 07/24/2021
> [ 197.171367] RIP: 0010:tc_setup_cls+0x20b/0x4a0 [stmmac]
> [ 197.171367] Code: fe ff ff c7 43 14 00 00 00 00 48 c7 03 00 00 00 00 c7 43 1c 00 00 00 00 49 8b 44 24 28 48 8b bd b0 00 00 00 41 0f b7 54 24 58 <48> 8b 00 0f bf 8f 38 08 00 00 81 ea e0 ff 00 00 8b 00 25 00 04 00
> [ 197.171367] RSP: 0018:ffff940940a037c0 EFLAGS: 00010246
> [ 197.171367] RAX: 0000000000000000 RBX: ffff92e826cae2c8 RCX: ffff92e825f39000
> [ 197.171367] RDX: 0000000000000000 RSI: ffff92e826cae2a8 RDI: ffff92e82f0c0000
> [ 197.171367] RBP: ffff92e82f0c0940 R08: 0000000000000000 R09: ffff92e825f39434
> [ 197.171367] R10: ffff92e826c5af00 R11: ffff940940a038a8 R12: ffff940940a038a8
> [ 197.171367] R13: 0000000000000000 R14: 0000000000000000 R15: ffff92e830a5b600
> [ 197.171367] FS: 00007fa7b0c47740(0000) GS:ffff92e964200000(0000) knlGS:0000000000000000
> [ 197.171367] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 197.171367] CR2: 0000000000000000 CR3: 0000000124c50000 CR4: 0000000000350ef0
> [ 197.171367] Call Trace:
> [ 197.171367] <TASK>
> [ 197.171367] ? __stmmac_disable_all_queues+0xa8/0xe0 [stmmac]
> [ 197.171367] stmmac_setup_tc_block_cb+0x70/0x110 [stmmac]
> [ 197.171367] tc_setup_cb_destroy+0xb3/0x180
> [ 197.171367] fl_hw_destroy_filter+0x94/0xc0 [cls_flower]
> [ 197.171367] __fl_delete+0x16a/0x180 [cls_flower]
> [ 197.171367] fl_destroy+0xb9/0x140 [cls_flower]
> [ 197.171367] tcf_proto_destroy+0x1d/0xa0
> [ 197.171367] tc_del_tfilter+0x3c9/0x7b0
> [ 197.171367] ? tc_dump_tfilter+0x310/0x310
> [ 197.171367] rtnetlink_rcv_msg+0x2bf/0x370
> [ 197.171367] ? preempt_count_add+0x68/0xa0
> [ 197.171367] ? _raw_spin_lock_irqsave+0x19/0x40
> [ 197.171367] ? _raw_spin_unlock_irqrestore+0x1f/0x31
> [ 197.171367] ? rtnl_calcit.isra.0+0x130/0x130
> [ 197.171367] netlink_rcv_skb+0x4e/0x100
> [ 197.171367] netlink_unicast+0x18e/0x230
> [ 197.171367] netlink_sendmsg+0x245/0x480
> [ 197.171367] sock_sendmsg+0x5b/0x60
> [ 197.171367] ____sys_sendmsg+0x20b/0x280
> [ 197.171367] ? copy_msghdr_from_user+0x5c/0x90
> [ 197.171367] ___sys_sendmsg+0x7c/0xc0
> [ 197.171367] ? folio_add_lru+0x52/0x80
> [ 197.171367] ? __sys_sendto+0xee/0x160
> [ 197.171367] __sys_sendmsg+0x59/0xa0
> [ 197.171367] do_syscall_64+0x40/0x90
> [ 197.171367] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 197.171367] RIP: 0033:0x7fa7b0d64397
> [ 197.171367] Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
> [ 197.171367] RSP: 002b:00007ffdd88b58e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [ 197.171367] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fa7b0d64397
> [ 197.171367] RDX: 0000000000000000 RSI: 00007ffdd88b5960 RDI: 0000000000000003
> [ 197.171367] RBP: 0000000061b05c21 R08: 0000000000000001 R09: 0000564584e47890
> [ 197.171367] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> [ 197.171367] R13: 00007ffdd88b9a80 R14: 00000000bfff0000 R15: 0000564584e3e420
> [ 197.171367] </TASK>
> [ 197.171367] Modules linked in: cls_flower sch_mqprio sch_ingress dwmac_intel(E) stmmac(E) pcs_xpcs phylink marvell marvell10g libphy 8021q bnep bluetooth ecryptfs nfsd sch_fq_codel uio uhid snd_soc_dmic snd_sof_pci_intel_tgl x86_pkg_temp_thermal snd_sof_intel_hda_common kvm_intel iTCO_wdt iTCO_vendor_support soundwire_intel mei_hdcp kvm soundwire_generic_allocation soundwire_cadence soundwire_bus irqbypass snd_sof_xtensa_dsp ax88179_178a snd_soc_acpi_intel_match intel_rapl_msr pcspkr usbnet snd_soc_acpi mii snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec i2c_i801 snd_hda_core intel_ish_ipc tpm_crb 8250_lpss intel_ishtp tpm_tis i915 mei_me i2c_smbus mei tpm_tis_core dw_dmac_core tpm spi_dw_pci parport_pc intel_pmc_core spi_dw thermal parport ttm fuse configfs snd_sof_pci snd_sof snd_soc_core snd_compress ac97_bus ledtrig_audio snd_pcm snd_timer snd soundcore [last unloaded: libphy]
> [ 197.171367] CR2: 0000000000000000
> [ 197.171367] ---[ end trace 8b8d1c617c39093d ]---
>
> This patch reimplements the tc flower rx frame steering for VLAN priority
> by keeping a record of flow_cls_offload added. The implementation also
> makes way to support EtherType based RX frame steering later.
>
> Fixes: 0e039f5cf86c ("net: stmmac: add RX frame steering based on VLAN priority in tc flower")
> Tested-by: Kurt Kanzenbach <kurt@linutronix.de>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering
2021-12-10 9:25 ` Kurt Kanzenbach
@ 2021-12-11 13:51 ` Ong, Boon Leong
0 siblings, 0 replies; 16+ messages in thread
From: Ong, Boon Leong @ 2021-12-11 13:51 UTC (permalink / raw)
To: Kanzenbach, Kurt, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue@foss.st.com
Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Sebastian Andrzej Siewior
>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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering
2021-12-09 15:16 ` [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering Ong Boon Leong
2021-12-10 9:25 ` Kurt Kanzenbach
@ 2021-12-10 16:30 ` Yannick Vignon
2021-12-11 13:59 ` Ong, Boon Leong
1 sibling, 1 reply; 16+ messages in thread
From: Yannick Vignon @ 2021-12-10 16:30 UTC (permalink / raw)
To: Ong Boon Leong, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue, Kurt Kanzenbach
Cc: netdev, linux-stm32, linux-arm-kernel
Hi,
On 12/9/2021 4:16 PM, Ong Boon Leong wrote:
> To replicate the issue:-
>
> 1) Add 2 flower filters for VLAN Priority based frame steering:-
> $ 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 add dev $IFDEVNAME parent ffff: protocol 802.1Q \
> flower vlan_prio 1 hw_tc 1
>
> 2) Get the 'pref' id
> $ tc filter show dev $IFDEVNAME ingress
>
> 3) Delete a specific tc flower record
> $ tc filter del dev $IFDEVNAME parent ffff: pref 49151
>
> From dmesg, we will observe kernel NULL pointer ooops
>
> [ 197.170464] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [ 197.171367] #PF: supervisor read access in kernel mode
> [ 197.171367] #PF: error_code(0x0000) - not-present page
> [ 197.171367] PGD 0 P4D 0
> [ 197.171367] Oops: 0000 [#1] PREEMPT SMP NOPTI
> [ 197.171367] CPU: 0 PID: 3216 Comm: tc Tainted: G U E 5.16.0-rc2+ #12
> [ 197.171367] Hardware name: Intel Corporation Elkhart Lake Embedded Platform/ElkhartLake LPDDR4x T3 CRB, BIOS EHLSFWI1.R00.3273.A04.2107240322 07/24/2021
> [ 197.171367] RIP: 0010:tc_setup_cls+0x20b/0x4a0 [stmmac]
> [ 197.171367] Code: fe ff ff c7 43 14 00 00 00 00 48 c7 03 00 00 00 00 c7 43 1c 00 00 00 00 49 8b 44 24 28 48 8b bd b0 00 00 00 41 0f b7 54 24 58 <48> 8b 00 0f bf 8f 38 08 00 00 81 ea e0 ff 00 00 8b 00 25 00 04 00
> [ 197.171367] RSP: 0018:ffff940940a037c0 EFLAGS: 00010246
> [ 197.171367] RAX: 0000000000000000 RBX: ffff92e826cae2c8 RCX: ffff92e825f39000
> [ 197.171367] RDX: 0000000000000000 RSI: ffff92e826cae2a8 RDI: ffff92e82f0c0000
> [ 197.171367] RBP: ffff92e82f0c0940 R08: 0000000000000000 R09: ffff92e825f39434
> [ 197.171367] R10: ffff92e826c5af00 R11: ffff940940a038a8 R12: ffff940940a038a8
> [ 197.171367] R13: 0000000000000000 R14: 0000000000000000 R15: ffff92e830a5b600
> [ 197.171367] FS: 00007fa7b0c47740(0000) GS:ffff92e964200000(0000) knlGS:0000000000000000
> [ 197.171367] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 197.171367] CR2: 0000000000000000 CR3: 0000000124c50000 CR4: 0000000000350ef0
> [ 197.171367] Call Trace:
> [ 197.171367] <TASK>
> [ 197.171367] ? __stmmac_disable_all_queues+0xa8/0xe0 [stmmac]
> [ 197.171367] stmmac_setup_tc_block_cb+0x70/0x110 [stmmac]
> [ 197.171367] tc_setup_cb_destroy+0xb3/0x180
> [ 197.171367] fl_hw_destroy_filter+0x94/0xc0 [cls_flower]
> [ 197.171367] __fl_delete+0x16a/0x180 [cls_flower]
> [ 197.171367] fl_destroy+0xb9/0x140 [cls_flower]
> [ 197.171367] tcf_proto_destroy+0x1d/0xa0
> [ 197.171367] tc_del_tfilter+0x3c9/0x7b0
> [ 197.171367] ? tc_dump_tfilter+0x310/0x310
> [ 197.171367] rtnetlink_rcv_msg+0x2bf/0x370
> [ 197.171367] ? preempt_count_add+0x68/0xa0
> [ 197.171367] ? _raw_spin_lock_irqsave+0x19/0x40
> [ 197.171367] ? _raw_spin_unlock_irqrestore+0x1f/0x31
> [ 197.171367] ? rtnl_calcit.isra.0+0x130/0x130
> [ 197.171367] netlink_rcv_skb+0x4e/0x100
> [ 197.171367] netlink_unicast+0x18e/0x230
> [ 197.171367] netlink_sendmsg+0x245/0x480
> [ 197.171367] sock_sendmsg+0x5b/0x60
> [ 197.171367] ____sys_sendmsg+0x20b/0x280
> [ 197.171367] ? copy_msghdr_from_user+0x5c/0x90
> [ 197.171367] ___sys_sendmsg+0x7c/0xc0
> [ 197.171367] ? folio_add_lru+0x52/0x80
> [ 197.171367] ? __sys_sendto+0xee/0x160
> [ 197.171367] __sys_sendmsg+0x59/0xa0
> [ 197.171367] do_syscall_64+0x40/0x90
> [ 197.171367] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 197.171367] RIP: 0033:0x7fa7b0d64397
> [ 197.171367] Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
> [ 197.171367] RSP: 002b:00007ffdd88b58e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [ 197.171367] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fa7b0d64397
> [ 197.171367] RDX: 0000000000000000 RSI: 00007ffdd88b5960 RDI: 0000000000000003
> [ 197.171367] RBP: 0000000061b05c21 R08: 0000000000000001 R09: 0000564584e47890
> [ 197.171367] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> [ 197.171367] R13: 00007ffdd88b9a80 R14: 00000000bfff0000 R15: 0000564584e3e420
> [ 197.171367] </TASK>
> [ 197.171367] Modules linked in: cls_flower sch_mqprio sch_ingress dwmac_intel(E) stmmac(E) pcs_xpcs phylink marvell marvell10g libphy 8021q bnep bluetooth ecryptfs nfsd sch_fq_codel uio uhid snd_soc_dmic snd_sof_pci_intel_tgl x86_pkg_temp_thermal snd_sof_intel_hda_common kvm_intel iTCO_wdt iTCO_vendor_support soundwire_intel mei_hdcp kvm soundwire_generic_allocation soundwire_cadence soundwire_bus irqbypass snd_sof_xtensa_dsp ax88179_178a snd_soc_acpi_intel_match intel_rapl_msr pcspkr usbnet snd_soc_acpi mii snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec i2c_i801 snd_hda_core intel_ish_ipc tpm_crb 8250_lpss intel_ishtp tpm_tis i915 mei_me i2c_smbus mei tpm_tis_core dw_dmac_core tpm spi_dw_pci parport_pc intel_pmc_core spi_dw thermal parport ttm fuse configfs snd_sof_pci snd_sof snd_soc_core snd_compress ac97_bus ledtrig_audio snd_pcm snd_timer snd soundcore [last unloaded: libphy]
> [ 197.171367] CR2: 0000000000000000
> [ 197.171367] ---[ end trace 8b8d1c617c39093d ]---
>
> This patch reimplements the tc flower rx frame steering for VLAN priority
> by keeping a record of flow_cls_offload added. The implementation also
> makes way to support EtherType based RX frame steering later.
>
> Fixes: 0e039f5cf86c ("net: stmmac: add RX frame steering based on VLAN priority in tc flower")
> Tested-by: Kurt Kanzenbach <kurt@linutronix.de>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 17 ++++
> .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 86 ++++++++++++++++---
> 2 files changed, 90 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index 4f5292cadf5..18a262ef17f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -171,6 +171,19 @@ struct stmmac_flow_entry {
> int is_l4;
> };
>
> +/* Rx Frame Steering */
> +enum stmmac_rfs_type {
> + STMMAC_RFS_T_VLAN,
> + STMMAC_RFS_T_MAX,
> +};
> +
> +struct stmmac_rfs_entry {
> + unsigned long cookie;
> + int in_use;
> + int type;
> + int tc;
> +};
> +
> struct stmmac_priv {
> /* Frequently used values are kept adjacent for cache effect */
> u32 tx_coal_frames[MTL_MAX_TX_QUEUES];
> @@ -288,6 +301,10 @@ struct stmmac_priv {
> struct stmmac_tc_entry *tc_entries;
> unsigned int flow_entries_max;
> struct stmmac_flow_entry *flow_entries;
> + unsigned int rfs_entries_max[STMMAC_RFS_T_MAX];
> + unsigned int rfs_entries_cnt[STMMAC_RFS_T_MAX];
> + unsigned int rfs_entries_total;
> + struct stmmac_rfs_entry *rfs_entries;
>
> /* Pulse Per Second output */
> struct stmmac_pps_cfg pps[STMMAC_PPS_MAX];
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 1c4ea0b1b84..d0a2b289f46 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -232,11 +232,33 @@ static int tc_setup_cls_u32(struct stmmac_priv *priv,
> }
> }
>
> +static int tc_rfs_init(struct stmmac_priv *priv)
> +{
> + int i;
> +
> + priv->rfs_entries_max[STMMAC_RFS_T_VLAN] = 8;
> +
> + for (i = 0; i < STMMAC_RFS_T_MAX; i++)
> + priv->rfs_entries_total += priv->rfs_entries_max[i];
> +
> + priv->rfs_entries = devm_kcalloc(priv->device,
> + priv->rfs_entries_total,
> + sizeof(*priv->rfs_entries),
> + GFP_KERNEL);
> + if (!priv->rfs_entries)
> + return -ENOMEM;
> +
> + dev_info(priv->device, "Enabled RFS Flow TC (entries=%d)\n",
> + priv->rfs_entries_total);
> +
> + return 0;
> +}
> +
> static int tc_init(struct stmmac_priv *priv)
> {
> struct dma_features *dma_cap = &priv->dma_cap;
> unsigned int count;
> - int i;
> + int ret, i;
>
> if (dma_cap->l3l4fnum) {
> priv->flow_entries_max = dma_cap->l3l4fnum;
> @@ -250,10 +272,14 @@ static int tc_init(struct stmmac_priv *priv)
> for (i = 0; i < priv->flow_entries_max; i++)
> priv->flow_entries[i].idx = i;
>
> - dev_info(priv->device, "Enabled Flow TC (entries=%d)\n",
> + dev_info(priv->device, "Enabled L3L4 Flow TC (entries=%d)\n",
> priv->flow_entries_max);
> }
>
> + ret = tc_rfs_init(priv);
> + if (ret)
> + return -ENOMEM;
> +
> if (!priv->plat->fpe_cfg) {
> priv->plat->fpe_cfg = devm_kzalloc(priv->device,
> sizeof(*priv->plat->fpe_cfg),
> @@ -607,16 +633,45 @@ static int tc_del_flow(struct stmmac_priv *priv,
> return ret;
> }
>
> +static struct stmmac_rfs_entry *tc_find_rfs(struct stmmac_priv *priv,
> + struct flow_cls_offload *cls,
> + bool get_free)
> +{
> + int i;
> +
> + for (i = 0; i < priv->rfs_entries_total; i++) {
> + struct stmmac_rfs_entry *entry = &priv->rfs_entries[i];
> +
> + if (entry->cookie == cls->cookie)
> + return entry;
> + if (get_free && entry->in_use == false)
> + return entry;
> + }
> +
> + return NULL;
> +}
> +
> #define VLAN_PRIO_FULL_MASK (0x07)
>
> static int tc_add_vlan_flow(struct stmmac_priv *priv,
> struct flow_cls_offload *cls)
> {
> + struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
> struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
> struct flow_dissector *dissector = rule->match.dissector;
> int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
> struct flow_match_vlan match;
>
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?
> + 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]++;
> }
>
> return 0;
> @@ -646,20 +707,19 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,
> static int tc_del_vlan_flow(struct stmmac_priv *priv,
> struct flow_cls_offload *cls)
> {
> - struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
> - struct flow_dissector *dissector = rule->match.dissector;
> - int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
> + struct stmmac_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]--;
>
> return 0;
> }
>
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
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering
2021-12-10 16:30 ` Yannick Vignon
@ 2021-12-11 13:59 ` Ong, Boon Leong
0 siblings, 0 replies; 16+ messages in thread
From: Ong, Boon Leong @ 2021-12-11 13:59 UTC (permalink / raw)
To: Yannick Vignon, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue@foss.st.com, Kanzenbach, Kurt
Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org
>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
Yes, you are right above. I will resend v2 for this patch
to fix the comment given by Sebastian on "net" patch version
here: https://patchwork.kernel.org/project/netdevbpf/patch/20211209130335.81114-1-boon.leong.ong@intel.com/
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching
2021-12-09 15:16 [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering Ong Boon Leong
2021-12-09 15:16 ` [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering Ong Boon Leong
@ 2021-12-09 15:16 ` Ong Boon Leong
2021-12-10 9:35 ` Kurt Kanzenbach
2021-12-10 10:10 ` Kurt Kanzenbach
2021-12-10 11:57 ` [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering Vladimir Oltean
2 siblings, 2 replies; 16+ messages in thread
From: Ong Boon Leong @ 2021-12-09 15:16 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue, Kurt Kanzenbach
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong
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 <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 +
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 121 ++++++++++++++++++
2 files changed, 124 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 18a262ef17f..ce12b2fbd80 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -174,11 +174,14 @@ struct stmmac_flow_entry {
/* Rx Frame Steering */
enum stmmac_rfs_type {
STMMAC_RFS_T_VLAN,
+ STMMAC_RFS_T_LLDP,
+ STMMAC_RFS_T_1588,
STMMAC_RFS_T_MAX,
};
struct stmmac_rfs_entry {
unsigned long cookie;
+ __be16 etype;
int in_use;
int type;
int tc;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index d0a2b289f46..de7ce4697a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -237,6 +237,8 @@ static int tc_rfs_init(struct stmmac_priv *priv)
int i;
priv->rfs_entries_max[STMMAC_RFS_T_VLAN] = 8;
+ priv->rfs_entries_max[STMMAC_RFS_T_LLDP] = 1;
+ priv->rfs_entries_max[STMMAC_RFS_T_1588] = 1;
for (i = 0; i < STMMAC_RFS_T_MAX; i++)
priv->rfs_entries_total += priv->rfs_entries_max[i];
@@ -451,6 +453,8 @@ static int tc_parse_flow_actions(struct stmmac_priv *priv,
return 0;
}
+#define ETHER_TYPE_FULL_MASK ((__force __be16)~0)
+
static int tc_add_basic_flow(struct stmmac_priv *priv,
struct flow_cls_offload *cls,
struct stmmac_flow_entry *entry)
@@ -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;
return 0;
}
@@ -724,6 +729,114 @@ static int tc_del_vlan_flow(struct stmmac_priv *priv,
return 0;
}
+static int tc_add_ethtype_flow(struct stmmac_priv *priv,
+ struct flow_cls_offload *cls)
+{
+ struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
+ struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
+ struct flow_dissector *dissector = rule->match.dissector;
+ int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
+ struct flow_match_basic match;
+
+ 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) {
+ __be16 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");
+ return -EINVAL;
+ }
+ switch (etype) {
+ case ETH_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;
+ case ETH_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;
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int tc_del_ethtype_flow(struct stmmac_priv *priv,
+ struct flow_cls_offload *cls)
+{
+ struct stmmac_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) {
+ case ETH_P_LLDP:
+ stmmac_rx_queue_routing(priv, priv->hw,
+ PACKET_DCBCPQ, 0);
+ priv->rfs_entries_cnt[STMMAC_RFS_T_LLDP]--;
+ break;
+ case ETH_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;
+
+ return 0;
+}
+
static int tc_add_flow_cls(struct stmmac_priv *priv,
struct flow_cls_offload *cls)
{
@@ -733,6 +846,10 @@ static int tc_add_flow_cls(struct stmmac_priv *priv,
if (!ret)
return ret;
+ ret = tc_add_ethtype_flow(priv, cls);
+ if (!ret)
+ return ret;
+
return tc_add_vlan_flow(priv, cls);
}
@@ -745,6 +862,10 @@ static int tc_del_flow_cls(struct stmmac_priv *priv,
if (!ret)
return ret;
+ ret = tc_del_ethtype_flow(priv, cls);
+ if (!ret)
+ return ret;
+
return tc_del_vlan_flow(priv, cls);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching
2021-12-09 15:16 ` [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching Ong Boon Leong
@ 2021-12-10 9:35 ` Kurt Kanzenbach
2021-12-10 10:10 ` Kurt Kanzenbach
1 sibling, 0 replies; 16+ messages in thread
From: Kurt Kanzenbach @ 2021-12-10 9:35 UTC (permalink / raw)
To: Ong Boon Leong, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong,
Sebastian Andrzej Siewior
[-- Attachment #1: Type: text/plain, Size: 993 bytes --]
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 <boon.leong.ong@intel.com>
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching
2021-12-09 15:16 ` [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching Ong Boon Leong
2021-12-10 9:35 ` Kurt Kanzenbach
@ 2021-12-10 10:10 ` Kurt Kanzenbach
2021-12-10 11:57 ` Sebastian Andrzej Siewior
2021-12-11 14:02 ` Ong, Boon Leong
1 sibling, 2 replies; 16+ messages in thread
From: Kurt Kanzenbach @ 2021-12-10 10:10 UTC (permalink / raw)
To: Ong Boon Leong, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong,
Sebastian Andrzej Siewior
[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]
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 <boon.leong.ong@intel.com>
[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
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 35ff7c835018..d64e42308eb6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -182,7 +182,7 @@ enum stmmac_rfs_type {
struct stmmac_rfs_entry {
unsigned long cookie;
- __be16 etype;
+ u16 etype;
int in_use;
int type;
int idx;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index cb7400943bb0..afa918185cf7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -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");
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching
2021-12-10 10:10 ` Kurt Kanzenbach
@ 2021-12-10 11:57 ` Sebastian Andrzej Siewior
2021-12-11 14:03 ` Ong, Boon Leong
2021-12-11 14:02 ` Ong, Boon Leong
1 sibling, 1 reply; 16+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-12-10 11:57 UTC (permalink / raw)
To: Kurt Kanzenbach
Cc: Ong Boon Leong, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue, netdev, linux-stm32, linux-arm-kernel
On 2021-12-10 11:10:04 [+0100], Kurt Kanzenbach wrote:
> > + if (match.mask->n_proto) {
> > + __be16 etype = ntohs(match.key->n_proto);
>
> n_proto is be16. The ntohs() call will produce an u16.
While at it, could we be please remove that __force in
ETHER_TYPE_FULL_MASK and use cpu_to_be16() macro?
> Thanks,
> Kurt
Sebastian
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching
2021-12-10 11:57 ` Sebastian Andrzej Siewior
@ 2021-12-11 14:03 ` Ong, Boon Leong
0 siblings, 0 replies; 16+ messages in thread
From: Ong, Boon Leong @ 2021-12-11 14:03 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Kanzenbach, Kurt
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue@foss.st.com, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org
>On 2021-12-10 11:10:04 [+0100], Kurt Kanzenbach wrote:
>> > + if (match.mask->n_proto) {
>> > + __be16 etype = ntohs(match.key->n_proto);
>>
>> n_proto is be16. The ntohs() call will produce an u16.
>
>While at it, could we be please remove that __force in
>ETHER_TYPE_FULL_MASK and use cpu_to_be16() macro?
>
>> Thanks,
>> Kurt
>
>Sebastian
Thanks. Will do.
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching
2021-12-10 10:10 ` Kurt Kanzenbach
2021-12-10 11:57 ` Sebastian Andrzej Siewior
@ 2021-12-11 14:02 ` Ong, Boon Leong
1 sibling, 0 replies; 16+ messages in thread
From: Ong, Boon Leong @ 2021-12-11 14:02 UTC (permalink / raw)
To: Kanzenbach, Kurt, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue@foss.st.com
Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Sebastian Andrzej Siewior
>[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
>
>diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>index 35ff7c835018..d64e42308eb6 100644
>--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>@@ -182,7 +182,7 @@ enum stmmac_rfs_type {
>
> struct stmmac_rfs_entry {
> unsigned long cookie;
>- __be16 etype;
>+ u16 etype;
> int in_use;
> int type;
> int idx;
>diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
>b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
>index cb7400943bb0..afa918185cf7 100644
>--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
>+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
>@@ -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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering
2021-12-09 15:16 [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering Ong Boon Leong
2021-12-09 15:16 ` [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering Ong Boon Leong
2021-12-09 15:16 ` [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching Ong Boon Leong
@ 2021-12-10 11:57 ` Vladimir Oltean
2021-12-10 19:38 ` Jakub Kicinski
2 siblings, 1 reply; 16+ messages in thread
From: Vladimir Oltean @ 2021-12-10 11:57 UTC (permalink / raw)
To: Ong Boon Leong, David S . Miller, Jakub Kicinski
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
alexandre.torgue, Kurt Kanzenbach, netdev, linux-stm32,
linux-arm-kernel
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?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering
2021-12-10 11:57 ` [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering Vladimir Oltean
@ 2021-12-10 19:38 ` Jakub Kicinski
2021-12-10 23:57 ` Nambiar, Amritha
0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2021-12-10 19:38 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Ong Boon Leong, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Maxime Coquelin, alexandre.torgue,
Kurt Kanzenbach, netdev, linux-stm32, linux-arm-kernel,
Amritha Nambiar
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?
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering
2021-12-10 19:38 ` Jakub Kicinski
@ 2021-12-10 23:57 ` Nambiar, Amritha
[not found] ` <20211210185517.30d27cfd@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
0 siblings, 1 reply; 16+ messages in thread
From: Nambiar, Amritha @ 2021-12-10 23:57 UTC (permalink / raw)
To: Jakub Kicinski, Vladimir Oltean
Cc: Ong, Boon Leong, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Maxime Coquelin,
alexandre.torgue@foss.st.com, Kanzenbach, Kurt,
netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Samudrala, Sridhar
> -----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 <boon.leong.ong@intel.com>; David S . Miller
> <davem@davemloft.net>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> Alexandre Torgue <alexandre.torgue@st.com>; Jose Abreu
> <joabreu@synopsys.com>; Maxime Coquelin
> <mcoquelin.stm32@gmail.com>; alexandre.torgue@foss.st.com;
> Kanzenbach, Kurt <kurt.kanzenbach@linutronix.de>;
> netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com;
> linux-arm-kernel@lists.infradead.org; Nambiar, Amritha
> <amritha.nambiar@intel.com>
> 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:
> > 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
^ permalink raw reply [flat|nested] 16+ messages in thread