Skip to content

Commit 85b15c2

Browse files
davem330davem330
committed
Merge branch 'net-sched-offload-failure-error-reporting'
Ido Schimmel says: ==================== net/sched: Better error reporting for offload failures This patchset improves error reporting to user space when offload fails during the flow action setup phase. That is, when failures occur in the actions themselves, even before calling device drivers. Requested / reported in [1]. This is done by passing extack to the offload_act_setup() callback and making use of it in the various actions. Patches #1-#2 change matchall and flower to log error messages to user space in accordance with the verbose flag. Patch #3 passes extack to the offload_act_setup() callback from the various call sites, including matchall and flower. Patches #4-#11 make use of extack in the various actions to report offload failures. Patch #12 adds an error message when the action does not support offload at all. Patches #13-#14 change matchall and flower to stop overwriting more specific error messages. [1] https://lore.kernel.org/netdev/20220317185249.5mff5u2x624pjewv@skbuf/ ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents 4a778f3 + fd23e0e commit 85b15c2

20 files changed

+128
-48
lines changed

‎include/net/act_api.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ struct tc_action_ops {
134134
(*get_psample_group)(const struct tc_action *a,
135135
tc_action_priv_destructor *destructor);
136136
int (*offload_act_setup)(struct tc_action *act, void *entry_data,
137-
u32 *index_inc, bool bind);
137+
u32 *index_inc, bool bind,
138+
struct netlink_ext_ack *extack);
138139
};
139140

140141
struct tc_action_net {

‎include/net/pkt_cls.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -547,10 +547,12 @@ tcf_match_indev(struct sk_buff *skb, int ifindex)
547547
}
548548

549549
int tc_setup_offload_action(struct flow_action *flow_action,
550-
const struct tcf_exts *exts);
550+
const struct tcf_exts *exts,
551+
struct netlink_ext_ack *extack);
551552
void tc_cleanup_offload_action(struct flow_action *flow_action);
552553
int tc_setup_action(struct flow_action *flow_action,
553-
struct tc_action *actions[]);
554+
struct tc_action *actions[],
555+
struct netlink_ext_ack *extack);
554556

555557
int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
556558
void *type_data, bool err_stop, bool rtnl_held);

‎include/net/tc_act/tc_gact.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,19 @@ static inline u32 tcf_gact_goto_chain_index(const struct tc_action *a)
5959
return READ_ONCE(a->tcfa_action) & TC_ACT_EXT_VAL_MASK;
6060
}
6161

62+
static inline bool is_tcf_gact_continue(const struct tc_action *a)
63+
{
64+
return __is_tcf_gact_act(a, TC_ACT_UNSPEC, false);
65+
}
66+
67+
static inline bool is_tcf_gact_reclassify(const struct tc_action *a)
68+
{
69+
return __is_tcf_gact_act(a, TC_ACT_RECLASSIFY, false);
70+
}
71+
72+
static inline bool is_tcf_gact_pipe(const struct tc_action *a)
73+
{
74+
return __is_tcf_gact_act(a, TC_ACT_PIPE, false);
75+
}
76+
6277
#endif /* __NET_TC_GACT_H */

‎include/net/tc_act/tc_skbedit.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,16 @@ static inline u32 tcf_skbedit_priority(const struct tc_action *a)
9494
return priority;
9595
}
9696

97+
/* Return true iff action is queue_mapping */
98+
static inline bool is_tcf_skbedit_queue_mapping(const struct tc_action *a)
99+
{
100+
return is_tcf_skbedit_with_flag(a, SKBEDIT_F_QUEUE_MAPPING);
101+
}
102+
103+
/* Return true iff action is inheritdsfield */
104+
static inline bool is_tcf_skbedit_inheritdsfield(const struct tc_action *a)
105+
{
106+
return is_tcf_skbedit_with_flag(a, SKBEDIT_F_INHERITDSFIELD);
107+
}
108+
97109
#endif /* __NET_TC_SKBEDIT_H */

‎net/sched/act_api.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ static int offload_action_init(struct flow_offload_action *fl_action,
195195
if (act->ops->offload_act_setup) {
196196
spin_lock_bh(&act->tcfa_lock);
197197
err = act->ops->offload_act_setup(act, fl_action, NULL,
198-
false);
198+
false, extack);
199199
spin_unlock_bh(&act->tcfa_lock);
200200
return err;
201201
}
@@ -271,7 +271,7 @@ static int tcf_action_offload_add_ex(struct tc_action *action,
271271
if (err)
272272
goto fl_err;
273273

274-
err = tc_setup_action(&fl_action->action, actions);
274+
err = tc_setup_action(&fl_action->action, actions, extack);
275275
if (err) {
276276
NL_SET_ERR_MSG_MOD(extack,
277277
"Failed to setup tc actions for offload");

‎net/sched/act_csum.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,8 @@ static size_t tcf_csum_get_fill_size(const struct tc_action *act)
696696
}
697697

698698
static int tcf_csum_offload_act_setup(struct tc_action *act, void *entry_data,
699-
u32 *index_inc, bool bind)
699+
u32 *index_inc, bool bind,
700+
struct netlink_ext_ack *extack)
700701
{
701702
if (bind) {
702703
struct flow_action_entry *entry = entry_data;

‎net/sched/act_ct.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1584,7 +1584,8 @@ static void tcf_stats_update(struct tc_action *a, u64 bytes, u64 packets,
15841584
}
15851585

15861586
static int tcf_ct_offload_act_setup(struct tc_action *act, void *entry_data,
1587-
u32 *index_inc, bool bind)
1587+
u32 *index_inc, bool bind,
1588+
struct netlink_ext_ack *extack)
15881589
{
15891590
if (bind) {
15901591
struct flow_action_entry *entry = entry_data;

‎net/sched/act_gact.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,8 @@ static size_t tcf_gact_get_fill_size(const struct tc_action *act)
253253
}
254254

255255
static int tcf_gact_offload_act_setup(struct tc_action *act, void *entry_data,
256-
u32 *index_inc, bool bind)
256+
u32 *index_inc, bool bind,
257+
struct netlink_ext_ack *extack)
257258
{
258259
if (bind) {
259260
struct flow_action_entry *entry = entry_data;
@@ -267,7 +268,17 @@ static int tcf_gact_offload_act_setup(struct tc_action *act, void *entry_data,
267268
} else if (is_tcf_gact_goto_chain(act)) {
268269
entry->id = FLOW_ACTION_GOTO;
269270
entry->chain_index = tcf_gact_goto_chain_index(act);
271+
} else if (is_tcf_gact_continue(act)) {
272+
NL_SET_ERR_MSG_MOD(extack, "Offload of \"continue\" action is not supported");
273+
return -EOPNOTSUPP;
274+
} else if (is_tcf_gact_reclassify(act)) {
275+
NL_SET_ERR_MSG_MOD(extack, "Offload of \"reclassify\" action is not supported");
276+
return -EOPNOTSUPP;
277+
} else if (is_tcf_gact_pipe(act)) {
278+
NL_SET_ERR_MSG_MOD(extack, "Offload of \"pipe\" action is not supported");
279+
return -EOPNOTSUPP;
270280
} else {
281+
NL_SET_ERR_MSG_MOD(extack, "Unsupported generic action offload");
271282
return -EOPNOTSUPP;
272283
}
273284
*index_inc = 1;

‎net/sched/act_gate.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,8 @@ static int tcf_gate_get_entries(struct flow_action_entry *entry,
619619
}
620620

621621
static int tcf_gate_offload_act_setup(struct tc_action *act, void *entry_data,
622-
u32 *index_inc, bool bind)
622+
u32 *index_inc, bool bind,
623+
struct netlink_ext_ack *extack)
623624
{
624625
int err;
625626

‎net/sched/act_mirred.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,8 @@ static void tcf_offload_mirred_get_dev(struct flow_action_entry *entry,
460460
}
461461

462462
static int tcf_mirred_offload_act_setup(struct tc_action *act, void *entry_data,
463-
u32 *index_inc, bool bind)
463+
u32 *index_inc, bool bind,
464+
struct netlink_ext_ack *extack)
464465
{
465466
if (bind) {
466467
struct flow_action_entry *entry = entry_data;
@@ -478,6 +479,7 @@ static int tcf_mirred_offload_act_setup(struct tc_action *act, void *entry_data,
478479
entry->id = FLOW_ACTION_MIRRED_INGRESS;
479480
tcf_offload_mirred_get_dev(entry, act);
480481
} else {
482+
NL_SET_ERR_MSG_MOD(extack, "Unsupported mirred offload");
481483
return -EOPNOTSUPP;
482484
}
483485
*index_inc = 1;

‎net/sched/act_mpls.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,8 @@ static int tcf_mpls_search(struct net *net, struct tc_action **a, u32 index)
385385
}
386386

387387
static int tcf_mpls_offload_act_setup(struct tc_action *act, void *entry_data,
388-
u32 *index_inc, bool bind)
388+
u32 *index_inc, bool bind,
389+
struct netlink_ext_ack *extack)
389390
{
390391
if (bind) {
391392
struct flow_action_entry *entry = entry_data;
@@ -410,7 +411,14 @@ static int tcf_mpls_offload_act_setup(struct tc_action *act, void *entry_data,
410411
entry->mpls_mangle.bos = tcf_mpls_bos(act);
411412
entry->mpls_mangle.ttl = tcf_mpls_ttl(act);
412413
break;
414+
case TCA_MPLS_ACT_DEC_TTL:
415+
NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"dec_ttl\" option is used");
416+
return -EOPNOTSUPP;
417+
case TCA_MPLS_ACT_MAC_PUSH:
418+
NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"mac_push\" option is used");
419+
return -EOPNOTSUPP;
413420
default:
421+
NL_SET_ERR_MSG_MOD(extack, "Unsupported MPLS mode offload");
414422
return -EOPNOTSUPP;
415423
}
416424
*index_inc = 1;

‎net/sched/act_pedit.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,8 @@ static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index)
488488
}
489489

490490
static int tcf_pedit_offload_act_setup(struct tc_action *act, void *entry_data,
491-
u32 *index_inc, bool bind)
491+
u32 *index_inc, bool bind,
492+
struct netlink_ext_ack *extack)
492493
{
493494
if (bind) {
494495
struct flow_action_entry *entry = entry_data;
@@ -503,6 +504,7 @@ static int tcf_pedit_offload_act_setup(struct tc_action *act, void *entry_data,
503504
entry->id = FLOW_ACTION_ADD;
504505
break;
505506
default:
507+
NL_SET_ERR_MSG_MOD(extack, "Unsupported pedit command offload");
506508
return -EOPNOTSUPP;
507509
}
508510
entry->mangle.htype = tcf_pedit_htype(act, k);

‎net/sched/act_police.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,8 @@ static int tcf_police_search(struct net *net, struct tc_action **a, u32 index)
419419
return tcf_idr_search(tn, a, index);
420420
}
421421

422-
static int tcf_police_act_to_flow_act(int tc_act, u32 *extval)
422+
static int tcf_police_act_to_flow_act(int tc_act, u32 *extval,
423+
struct netlink_ext_ack *extack)
423424
{
424425
int act_id = -EOPNOTSUPP;
425426

@@ -430,19 +431,28 @@ static int tcf_police_act_to_flow_act(int tc_act, u32 *extval)
430431
act_id = FLOW_ACTION_DROP;
431432
else if (tc_act == TC_ACT_PIPE)
432433
act_id = FLOW_ACTION_PIPE;
434+
else if (tc_act == TC_ACT_RECLASSIFY)
435+
NL_SET_ERR_MSG_MOD(extack, "Offload not supported when conform/exceed action is \"reclassify\"");
436+
else
437+
NL_SET_ERR_MSG_MOD(extack, "Unsupported conform/exceed action offload");
433438
} else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) {
434439
act_id = FLOW_ACTION_GOTO;
435440
*extval = tc_act & TC_ACT_EXT_VAL_MASK;
436441
} else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_JUMP)) {
437442
act_id = FLOW_ACTION_JUMP;
438443
*extval = tc_act & TC_ACT_EXT_VAL_MASK;
444+
} else if (tc_act == TC_ACT_UNSPEC) {
445+
NL_SET_ERR_MSG_MOD(extack, "Offload not supported when conform/exceed action is \"continue\"");
446+
} else {
447+
NL_SET_ERR_MSG_MOD(extack, "Unsupported conform/exceed action offload");
439448
}
440449

441450
return act_id;
442451
}
443452

444453
static int tcf_police_offload_act_setup(struct tc_action *act, void *entry_data,
445-
u32 *index_inc, bool bind)
454+
u32 *index_inc, bool bind,
455+
struct netlink_ext_ack *extack)
446456
{
447457
if (bind) {
448458
struct flow_action_entry *entry = entry_data;
@@ -466,14 +476,16 @@ static int tcf_police_offload_act_setup(struct tc_action *act, void *entry_data,
466476
entry->police.mtu = tcf_police_tcfp_mtu(act);
467477

468478
act_id = tcf_police_act_to_flow_act(police->tcf_action,
469-
&entry->police.exceed.extval);
479+
&entry->police.exceed.extval,
480+
extack);
470481
if (act_id < 0)
471482
return act_id;
472483

473484
entry->police.exceed.act_id = act_id;
474485

475486
act_id = tcf_police_act_to_flow_act(p->tcfp_result,
476-
&entry->police.notexceed.extval);
487+
&entry->police.notexceed.extval,
488+
extack);
477489
if (act_id < 0)
478490
return act_id;
479491

‎net/sched/act_sample.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,8 @@ static void tcf_offload_sample_get_group(struct flow_action_entry *entry,
291291
}
292292

293293
static int tcf_sample_offload_act_setup(struct tc_action *act, void *entry_data,
294-
u32 *index_inc, bool bind)
294+
u32 *index_inc, bool bind,
295+
struct netlink_ext_ack *extack)
295296
{
296297
if (bind) {
297298
struct flow_action_entry *entry = entry_data;

‎net/sched/act_skbedit.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,8 @@ static size_t tcf_skbedit_get_fill_size(const struct tc_action *act)
328328
}
329329

330330
static int tcf_skbedit_offload_act_setup(struct tc_action *act, void *entry_data,
331-
u32 *index_inc, bool bind)
331+
u32 *index_inc, bool bind,
332+
struct netlink_ext_ack *extack)
332333
{
333334
if (bind) {
334335
struct flow_action_entry *entry = entry_data;
@@ -342,7 +343,14 @@ static int tcf_skbedit_offload_act_setup(struct tc_action *act, void *entry_data
342343
} else if (is_tcf_skbedit_priority(act)) {
343344
entry->id = FLOW_ACTION_PRIORITY;
344345
entry->priority = tcf_skbedit_priority(act);
346+
} else if (is_tcf_skbedit_queue_mapping(act)) {
347+
NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"queue_mapping\" option is used");
348+
return -EOPNOTSUPP;
349+
} else if (is_tcf_skbedit_inheritdsfield(act)) {
350+
NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"inheritdsfield\" option is used");
351+
return -EOPNOTSUPP;
345352
} else {
353+
NL_SET_ERR_MSG_MOD(extack, "Unsupported skbedit option offload");
346354
return -EOPNOTSUPP;
347355
}
348356
*index_inc = 1;

‎net/sched/act_tunnel_key.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,8 @@ static int tcf_tunnel_encap_get_tunnel(struct flow_action_entry *entry,
808808
static int tcf_tunnel_key_offload_act_setup(struct tc_action *act,
809809
void *entry_data,
810810
u32 *index_inc,
811-
bool bind)
811+
bool bind,
812+
struct netlink_ext_ack *extack)
812813
{
813814
int err;
814815

@@ -823,6 +824,7 @@ static int tcf_tunnel_key_offload_act_setup(struct tc_action *act,
823824
} else if (is_tcf_tunnel_release(act)) {
824825
entry->id = FLOW_ACTION_TUNNEL_DECAP;
825826
} else {
827+
NL_SET_ERR_MSG_MOD(extack, "Unsupported tunnel key mode offload");
826828
return -EOPNOTSUPP;
827829
}
828830
*index_inc = 1;

‎net/sched/act_vlan.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,8 @@ static size_t tcf_vlan_get_fill_size(const struct tc_action *act)
369369
}
370370

371371
static int tcf_vlan_offload_act_setup(struct tc_action *act, void *entry_data,
372-
u32 *index_inc, bool bind)
372+
u32 *index_inc, bool bind,
373+
struct netlink_ext_ack *extack)
373374
{
374375
if (bind) {
375376
struct flow_action_entry *entry = entry_data;
@@ -398,6 +399,7 @@ static int tcf_vlan_offload_act_setup(struct tc_action *act, void *entry_data,
398399
tcf_vlan_push_eth(entry->vlan_push_eth.src, entry->vlan_push_eth.dst, act);
399400
break;
400401
default:
402+
NL_SET_ERR_MSG_MOD(extack, "Unsupported vlan action mode offload");
401403
return -EOPNOTSUPP;
402404
}
403405
*index_inc = 1;

‎net/sched/cls_api.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3513,20 +3513,25 @@ EXPORT_SYMBOL(tc_cleanup_offload_action);
35133513

35143514
static int tc_setup_offload_act(struct tc_action *act,
35153515
struct flow_action_entry *entry,
3516-
u32 *index_inc)
3516+
u32 *index_inc,
3517+
struct netlink_ext_ack *extack)
35173518
{
35183519
#ifdef CONFIG_NET_CLS_ACT
3519-
if (act->ops->offload_act_setup)
3520-
return act->ops->offload_act_setup(act, entry, index_inc, true);
3521-
else
3520+
if (act->ops->offload_act_setup) {
3521+
return act->ops->offload_act_setup(act, entry, index_inc, true,
3522+
extack);
3523+
} else {
3524+
NL_SET_ERR_MSG(extack, "Action does not support offload");
35223525
return -EOPNOTSUPP;
3526+
}
35233527
#else
35243528
return 0;
35253529
#endif
35263530
}
35273531

35283532
int tc_setup_action(struct flow_action *flow_action,
3529-
struct tc_action *actions[])
3533+
struct tc_action *actions[],
3534+
struct netlink_ext_ack *extack)
35303535
{
35313536
int i, j, index, err = 0;
35323537
struct tc_action *act;
@@ -3551,7 +3556,7 @@ int tc_setup_action(struct flow_action *flow_action,
35513556
entry->hw_stats = tc_act_hw_stats(act->hw_stats);
35523557
entry->hw_index = act->tcfa_index;
35533558
index = 0;
3554-
err = tc_setup_offload_act(act, entry, &index);
3559+
err = tc_setup_offload_act(act, entry, &index, extack);
35553560
if (!err)
35563561
j += index;
35573562
else
@@ -3570,13 +3575,14 @@ int tc_setup_action(struct flow_action *flow_action,
35703575
}
35713576

35723577
int tc_setup_offload_action(struct flow_action *flow_action,
3573-
const struct tcf_exts *exts)
3578+
const struct tcf_exts *exts,
3579+
struct netlink_ext_ack *extack)
35743580
{
35753581
#ifdef CONFIG_NET_CLS_ACT
35763582
if (!exts)
35773583
return 0;
35783584

3579-
return tc_setup_action(flow_action, exts->actions);
3585+
return tc_setup_action(flow_action, exts->actions, extack);
35803586
#else
35813587
return 0;
35823588
#endif

0 commit comments

Comments
 (0)