Skip to content

Commit aeb7c75

Browse files
elvinongbldavem330
authored andcommitted
net: stmmac: fix tc flower deletion for VLAN priority Rx steering
To replicate the issue:- 1) Add 1 flower filter 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 2) Get the 'pref' id $ tc filter show dev $IFDEVNAME ingress 3) Delete a specific tc flower record (say pref 49151) $ 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 <snip> [ 197.171367] RIP: 0010:tc_setup_cls+0x20b/0x4a0 [stmmac] <snip> [ 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] The above issue is due to previous incorrect implementation of tc_del_vlan_flow(), shown below, that uses flow_cls_offload_flow_rule() to get struct flow_rule *rule which is no longer valid for tc filter delete operation. struct flow_rule *rule = flow_cls_offload_flow_rule(cls); struct flow_dissector *dissector = rule->match.dissector; So, to ensure tc_del_vlan_flow() deletes the right VLAN cls record for earlier configured RX queue (configured by hw_tc) in tc_add_vlan_flow(), this patch introduces stmmac_rfs_entry as driver-side flow_cls_offload record for 'RX frame steering' tc flower, currently used for VLAN priority. The implementation has taken consideration for future extension to include other type RX frame steering such as EtherType based. v2: - Clean up overly extensive backtrace and rewrite git message to better explain the kernel NULL pointer issue. Fixes: 0e039f5 ("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> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent b0cdc5d commit aeb7c75

File tree

2 files changed

+90
-13
lines changed

2 files changed

+90
-13
lines changed

drivers/net/ethernet/stmicro/stmmac/stmmac.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,19 @@ struct stmmac_flow_entry {
172172
int is_l4;
173173
};
174174

175+
/* Rx Frame Steering */
176+
enum stmmac_rfs_type {
177+
STMMAC_RFS_T_VLAN,
178+
STMMAC_RFS_T_MAX,
179+
};
180+
181+
struct stmmac_rfs_entry {
182+
unsigned long cookie;
183+
int in_use;
184+
int type;
185+
int tc;
186+
};
187+
175188
struct stmmac_priv {
176189
/* Frequently used values are kept adjacent for cache effect */
177190
u32 tx_coal_frames[MTL_MAX_TX_QUEUES];
@@ -289,6 +302,10 @@ struct stmmac_priv {
289302
struct stmmac_tc_entry *tc_entries;
290303
unsigned int flow_entries_max;
291304
struct stmmac_flow_entry *flow_entries;
305+
unsigned int rfs_entries_max[STMMAC_RFS_T_MAX];
306+
unsigned int rfs_entries_cnt[STMMAC_RFS_T_MAX];
307+
unsigned int rfs_entries_total;
308+
struct stmmac_rfs_entry *rfs_entries;
292309

293310
/* Pulse Per Second output */
294311
struct stmmac_pps_cfg pps[STMMAC_PPS_MAX];

drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

Lines changed: 73 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -232,11 +232,33 @@ static int tc_setup_cls_u32(struct stmmac_priv *priv,
232232
}
233233
}
234234

235+
static int tc_rfs_init(struct stmmac_priv *priv)
236+
{
237+
int i;
238+
239+
priv->rfs_entries_max[STMMAC_RFS_T_VLAN] = 8;
240+
241+
for (i = 0; i < STMMAC_RFS_T_MAX; i++)
242+
priv->rfs_entries_total += priv->rfs_entries_max[i];
243+
244+
priv->rfs_entries = devm_kcalloc(priv->device,
245+
priv->rfs_entries_total,
246+
sizeof(*priv->rfs_entries),
247+
GFP_KERNEL);
248+
if (!priv->rfs_entries)
249+
return -ENOMEM;
250+
251+
dev_info(priv->device, "Enabled RFS Flow TC (entries=%d)\n",
252+
priv->rfs_entries_total);
253+
254+
return 0;
255+
}
256+
235257
static int tc_init(struct stmmac_priv *priv)
236258
{
237259
struct dma_features *dma_cap = &priv->dma_cap;
238260
unsigned int count;
239-
int i;
261+
int ret, i;
240262

241263
if (dma_cap->l3l4fnum) {
242264
priv->flow_entries_max = dma_cap->l3l4fnum;
@@ -250,10 +272,14 @@ static int tc_init(struct stmmac_priv *priv)
250272
for (i = 0; i < priv->flow_entries_max; i++)
251273
priv->flow_entries[i].idx = i;
252274

253-
dev_info(priv->device, "Enabled Flow TC (entries=%d)\n",
275+
dev_info(priv->device, "Enabled L3L4 Flow TC (entries=%d)\n",
254276
priv->flow_entries_max);
255277
}
256278

279+
ret = tc_rfs_init(priv);
280+
if (ret)
281+
return -ENOMEM;
282+
257283
if (!priv->plat->fpe_cfg) {
258284
priv->plat->fpe_cfg = devm_kzalloc(priv->device,
259285
sizeof(*priv->plat->fpe_cfg),
@@ -607,16 +633,45 @@ static int tc_del_flow(struct stmmac_priv *priv,
607633
return ret;
608634
}
609635

636+
static struct stmmac_rfs_entry *tc_find_rfs(struct stmmac_priv *priv,
637+
struct flow_cls_offload *cls,
638+
bool get_free)
639+
{
640+
int i;
641+
642+
for (i = 0; i < priv->rfs_entries_total; i++) {
643+
struct stmmac_rfs_entry *entry = &priv->rfs_entries[i];
644+
645+
if (entry->cookie == cls->cookie)
646+
return entry;
647+
if (get_free && entry->in_use == false)
648+
return entry;
649+
}
650+
651+
return NULL;
652+
}
653+
610654
#define VLAN_PRIO_FULL_MASK (0x07)
611655

612656
static int tc_add_vlan_flow(struct stmmac_priv *priv,
613657
struct flow_cls_offload *cls)
614658
{
659+
struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
615660
struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
616661
struct flow_dissector *dissector = rule->match.dissector;
617662
int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
618663
struct flow_match_vlan match;
619664

665+
if (!entry) {
666+
entry = tc_find_rfs(priv, cls, true);
667+
if (!entry)
668+
return -ENOENT;
669+
}
670+
671+
if (priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN] >=
672+
priv->rfs_entries_max[STMMAC_RFS_T_VLAN])
673+
return -ENOENT;
674+
620675
/* Nothing to do here */
621676
if (!dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_VLAN))
622677
return -EINVAL;
@@ -638,6 +693,12 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,
638693

639694
prio = BIT(match.key->vlan_priority);
640695
stmmac_rx_queue_prio(priv, priv->hw, prio, tc);
696+
697+
entry->in_use = true;
698+
entry->cookie = cls->cookie;
699+
entry->tc = tc;
700+
entry->type = STMMAC_RFS_T_VLAN;
701+
priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN]++;
641702
}
642703

643704
return 0;
@@ -646,20 +707,19 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,
646707
static int tc_del_vlan_flow(struct stmmac_priv *priv,
647708
struct flow_cls_offload *cls)
648709
{
649-
struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
650-
struct flow_dissector *dissector = rule->match.dissector;
651-
int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
710+
struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
652711

653-
/* Nothing to do here */
654-
if (!dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_VLAN))
655-
return -EINVAL;
712+
if (!entry || !entry->in_use || entry->type != STMMAC_RFS_T_VLAN)
713+
return -ENOENT;
656714

657-
if (tc < 0) {
658-
netdev_err(priv->dev, "Invalid traffic class\n");
659-
return -EINVAL;
660-
}
715+
stmmac_rx_queue_prio(priv, priv->hw, 0, entry->tc);
716+
717+
entry->in_use = false;
718+
entry->cookie = 0;
719+
entry->tc = 0;
720+
entry->type = 0;
661721

662-
stmmac_rx_queue_prio(priv, priv->hw, 0, tc);
722+
priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN]--;
663723

664724
return 0;
665725
}

0 commit comments

Comments
 (0)