* [PATCH net-next] net/sched: report errors with extack
@ 2024-01-31 16:58 Stephen Hemminger
2024-01-31 19:53 ` Jamal Hadi Salim
0 siblings, 1 reply; 2+ messages in thread
From: Stephen Hemminger @ 2024-01-31 16:58 UTC (permalink / raw)
To: netdev
Cc: Stephen Hemminger, Martin KaFai Lau, Daniel Borkmann,
John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni,
open list:BPF [NETWORKING] (tcx & tc BPF, sock_addr),
open list
While working a BPF action, found that the error handling was
limited. The support of external ack was only added to some
but not all actions.
When an action detects invalid parameters, it should
be adding an external ack to netlink so that the user is
able to diagnose the issue.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/act_bpf.c | 31 ++++++++++++++++++++++---------
net/sched/act_connmark.c | 8 ++++++--
net/sched/act_csum.c | 8 ++++++--
net/sched/act_gact.c | 14 +++++++++++---
net/sched/act_gate.c | 15 +++++++++++----
net/sched/act_ife.c | 8 ++++++--
net/sched/act_nat.c | 9 +++++++--
net/sched/act_police.c | 13 ++++++++++---
net/sched/act_sample.c | 8 ++++++--
net/sched/act_simple.c | 9 +++++++--
net/sched/act_skbedit.c | 13 ++++++++++---
net/sched/act_skbmod.c | 9 +++++++--
net/sched/act_vlan.c | 8 ++++++--
13 files changed, 115 insertions(+), 38 deletions(-)
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 6cfee6658103..f8a03d3bbf20 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -184,7 +184,8 @@ static const struct nla_policy act_bpf_policy[TCA_ACT_BPF_MAX + 1] = {
.len = sizeof(struct sock_filter) * BPF_MAXINSNS },
};
-static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
+static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg,
+ struct netlink_ext_ack *extack)
{
struct sock_filter *bpf_ops;
struct sock_fprog_kern fprog_tmp;
@@ -193,12 +194,16 @@ static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
int ret;
bpf_num_ops = nla_get_u16(tb[TCA_ACT_BPF_OPS_LEN]);
- if (bpf_num_ops > BPF_MAXINSNS || bpf_num_ops == 0)
+ if (bpf_num_ops > BPF_MAXINSNS || bpf_num_ops == 0) {
+ NL_SET_ERR_MSG_MOD(extack, "Invalid number of BPF instructions");
return -EINVAL;
+ }
bpf_size = bpf_num_ops * sizeof(*bpf_ops);
- if (bpf_size != nla_len(tb[TCA_ACT_BPF_OPS]))
+ if (bpf_size != nla_len(tb[TCA_ACT_BPF_OPS])) {
+ NL_SET_ERR_MSG_MOD(extack, "BPF instruction size");
return -EINVAL;
+ }
bpf_ops = kmemdup(nla_data(tb[TCA_ACT_BPF_OPS]), bpf_size, GFP_KERNEL);
if (bpf_ops == NULL)
@@ -221,7 +226,8 @@ static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
return 0;
}
-static int tcf_bpf_init_from_efd(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
+static int tcf_bpf_init_from_efd(struct nlattr **tb, struct tcf_bpf_cfg *cfg,
+ struct netlink_ext_ack *extack)
{
struct bpf_prog *fp;
char *name = NULL;
@@ -230,8 +236,10 @@ static int tcf_bpf_init_from_efd(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
bpf_fd = nla_get_u32(tb[TCA_ACT_BPF_FD]);
fp = bpf_prog_get_type(bpf_fd, BPF_PROG_TYPE_SCHED_ACT);
- if (IS_ERR(fp))
+ if (IS_ERR(fp)) {
+ NL_SET_ERR_MSG_MOD(extack, "BPF program type mismatch");
return PTR_ERR(fp);
+ }
if (tb[TCA_ACT_BPF_NAME]) {
name = nla_memdup(tb[TCA_ACT_BPF_NAME], GFP_KERNEL);
@@ -292,16 +300,20 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
int ret, res = 0;
u32 index;
- if (!nla)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Bpf requires attributes to be passed");
return -EINVAL;
+ }
ret = nla_parse_nested_deprecated(tb, TCA_ACT_BPF_MAX, nla,
act_bpf_policy, NULL);
if (ret < 0)
return ret;
- if (!tb[TCA_ACT_BPF_PARMS])
+ if (!tb[TCA_ACT_BPF_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required bpf parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_ACT_BPF_PARMS]);
index = parm->index;
@@ -336,14 +348,15 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
is_ebpf = tb[TCA_ACT_BPF_FD];
if (is_bpf == is_ebpf) {
+ NL_SET_ERR_MSG_MOD(extack, "Can not specify both BPF fd and ops");
ret = -EINVAL;
goto put_chain;
}
memset(&cfg, 0, sizeof(cfg));
- ret = is_bpf ? tcf_bpf_init_from_ops(tb, &cfg) :
- tcf_bpf_init_from_efd(tb, &cfg);
+ ret = is_bpf ? tcf_bpf_init_from_ops(tb, &cfg, extack) :
+ tcf_bpf_init_from_efd(tb, &cfg, extack);
if (ret < 0)
goto put_chain;
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index f8762756657d..0964d10dfc04 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -110,16 +110,20 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
int ret = 0, err;
u32 index;
- if (!nla)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Connmark requires attributes to be passed");
return -EINVAL;
+ }
ret = nla_parse_nested_deprecated(tb, TCA_CONNMARK_MAX, nla,
connmark_policy, NULL);
if (ret < 0)
return ret;
- if (!tb[TCA_CONNMARK_PARMS])
+ if (!tb[TCA_CONNMARK_PARMS]) {
+ NL_SET_ERR_MSG(extack, "Missing required connmark parameters");
return -EINVAL;
+ }
nparms = kzalloc(sizeof(*nparms), GFP_KERNEL);
if (!nparms)
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 7f8b1f2f2ed9..7c7f74e37528 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -55,16 +55,20 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
int ret = 0, err;
u32 index;
- if (nla == NULL)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Checksum requires attributes to be passed");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_CSUM_MAX, nla, csum_policy,
NULL);
if (err < 0)
return err;
- if (tb[TCA_CSUM_PARMS] == NULL)
+ if (!tb[TCA_CSUM_PARMS]) {
+ NL_SET_ERR_MSG(extack, "Missing required checksum parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_CSUM_PARMS]);
index = parm->index;
err = tcf_idr_check_alloc(tn, &index, a, bind);
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 4af3b7ec249f..5d04bcd5115e 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -68,16 +68,21 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
struct tc_gact_p *p_parm = NULL;
#endif
- if (nla == NULL)
+ if (!nla) {
+ NL_SET_ERR_MSG(extack, "Gact requires attributes to be passed");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_GACT_MAX, nla, gact_policy,
NULL);
if (err < 0)
return err;
- if (tb[TCA_GACT_PARMS] == NULL)
+ if (!tb[TCA_GACT_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required gact parameters");
return -EINVAL;
+ }
+
parm = nla_data(tb[TCA_GACT_PARMS]);
index = parm->index;
@@ -87,8 +92,11 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
#else
if (tb[TCA_GACT_PROB]) {
p_parm = nla_data(tb[TCA_GACT_PROB]);
- if (p_parm->ptype >= MAX_RAND)
+ if (p_parm->ptype >= MAX_RAND) {
+ NL_SET_ERR_MSG(extack, "Invalid ptype in gact prob");
return -EINVAL;
+ }
+
if (TC_ACT_EXT_CMP(p_parm->paction, TC_ACT_GOTO_CHAIN)) {
NL_SET_ERR_MSG(extack,
"goto chain not allowed on fallback");
diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index c681cd011afd..c9e32822938c 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -239,8 +239,10 @@ static int parse_gate_list(struct nlattr *list_attr,
int err, rem;
int i = 0;
- if (!list_attr)
+ if (!list_attr) {
+ NL_SET_ERR_MSG(extack, "Gate missing attributes");
return -EINVAL;
+ }
nla_for_each_nested(n, list_attr, rem) {
if (nla_type(n) != TCA_GATE_ONE_ENTRY) {
@@ -317,15 +319,19 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
ktime_t start;
u32 index;
- if (!nla)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Gate requires attributes to be passed");
return -EINVAL;
+ }
err = nla_parse_nested(tb, TCA_GATE_MAX, nla, gate_policy, extack);
if (err < 0)
return err;
- if (!tb[TCA_GATE_PARMS])
+ if (!tb[TCA_GATE_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required gate parameters");
return -EINVAL;
+ }
if (tb[TCA_GATE_CLOCKID]) {
clockid = nla_get_s32(tb[TCA_GATE_CLOCKID]);
@@ -343,7 +349,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
tk_offset = TK_OFFS_TAI;
break;
default:
- NL_SET_ERR_MSG(extack, "Invalid 'clockid'");
+ NL_SET_ERR_MSG_MOD(extack, "Invalid 'clockid'");
return -EINVAL;
}
}
@@ -409,6 +415,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
cycle = ktime_add_ns(cycle, entry->interval);
cycletime = cycle;
if (!cycletime) {
+ NL_SET_ERR_MSG_MOD(extack, "cycle time is zero");
err = -EINVAL;
goto chain_put;
}
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 0e867d13beb5..85a58cfb23f3 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -508,8 +508,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
if (err < 0)
return err;
- if (!tb[TCA_IFE_PARMS])
+ if (!tb[TCA_IFE_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required ife parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_IFE_PARMS]);
@@ -517,8 +519,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
* they cannot run as the same time. Check on all other values which
* are not supported right now.
*/
- if (parm->flags & ~IFE_ENCODE)
+ if (parm->flags & ~IFE_ENCODE) {
+ NL_SET_ERR_MSG_MOD(extack, "Invalid ife flag parameter");
return -EINVAL;
+ }
p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p)
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index a180e724634e..a990d0c626cd 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -46,16 +46,21 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
struct tcf_nat *p;
u32 index;
- if (nla == NULL)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Nat action requires attributes");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_NAT_MAX, nla, nat_policy,
NULL);
if (err < 0)
return err;
- if (tb[TCA_NAT_PARMS] == NULL)
+ if (!tb[TCA_NAT_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Nat action parameters missing");
return -EINVAL;
+ }
+
parm = nla_data(tb[TCA_NAT_PARMS]);
index = parm->index;
err = tcf_idr_check_alloc(tn, &index, a, bind);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index e119b4a3db9f..3eb41233257b 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -56,19 +56,26 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
u64 rate64, prate64;
u64 pps, ppsburst;
- if (nla == NULL)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Police requires attributes");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_POLICE_MAX, nla,
police_policy, NULL);
if (err < 0)
return err;
- if (tb[TCA_POLICE_TBF] == NULL)
+ if (!tb[TCA_POLICE_TBF]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required police action parameters");
return -EINVAL;
+ }
+
size = nla_len(tb[TCA_POLICE_TBF]);
- if (size != sizeof(*parm) && size != sizeof(struct tc_police_compat))
+ if (size != sizeof(*parm) && size != sizeof(struct tc_police_compat)) {
+ NL_SET_ERR_MSG_MOD(extack, "Invalid size for police action parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_POLICE_TBF]);
index = parm->index;
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index c5c61efe6db4..2442e001d92e 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -49,15 +49,19 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
bool exists = false;
int ret, err;
- if (!nla)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Sample requires attributes to be passed");
return -EINVAL;
+ }
ret = nla_parse_nested_deprecated(tb, TCA_SAMPLE_MAX, nla,
sample_policy, NULL);
if (ret < 0)
return ret;
- if (!tb[TCA_SAMPLE_PARMS])
+ if (!tb[TCA_SAMPLE_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required sample action parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_SAMPLE_PARMS]);
index = parm->index;
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 0a3e92888295..02b8e42c1bdd 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -100,16 +100,20 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
int ret = 0, err;
u32 index;
- if (nla == NULL)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Sample requires attributes to be passed");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_DEF_MAX, nla, simple_policy,
NULL);
if (err < 0)
return err;
- if (tb[TCA_DEF_PARMS] == NULL)
+ if (!tb[TCA_DEF_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required sample action parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_DEF_PARMS]);
index = parm->index;
@@ -121,6 +125,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
return ACT_P_BOUND;
if (tb[TCA_DEF_DATA] == NULL) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing simple action default data");
if (exists)
tcf_idr_release(*a, bind);
else
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 754f78b35bb8..671ca64a2c33 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -133,16 +133,20 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
int ret = 0, err;
u32 index;
- if (nla == NULL)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Skbedit requires attributes to be passed");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_SKBEDIT_MAX, nla,
skbedit_policy, NULL);
if (err < 0)
return err;
- if (tb[TCA_SKBEDIT_PARMS] == NULL)
+ if (!tb[TCA_SKBEDIT_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required skbedit parameters");
return -EINVAL;
+ }
if (tb[TCA_SKBEDIT_PRIORITY] != NULL) {
flags |= SKBEDIT_F_PRIORITY;
@@ -161,8 +165,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
if (tb[TCA_SKBEDIT_PTYPE] != NULL) {
ptype = nla_data(tb[TCA_SKBEDIT_PTYPE]);
- if (!skb_pkt_type_ok(*ptype))
+ if (!skb_pkt_type_ok(*ptype)) {
+ NL_SET_ERR_MSG_MOD(extack, "ptype is not a valid");
return -EINVAL;
+ }
flags |= SKBEDIT_F_PTYPE;
}
@@ -212,6 +218,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
return ACT_P_BOUND;
if (!flags) {
+ NL_SET_ERR_MSG_MOD(extack, "No skbedit action flag");
if (exists)
tcf_idr_release(*a, bind);
else
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index bcb673ab0008..c80828cdeb69 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -119,16 +119,20 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
u16 eth_type = 0;
int ret = 0, err;
- if (!nla)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Skbmod requires attributes to be passed");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_SKBMOD_MAX, nla,
skbmod_policy, NULL);
if (err < 0)
return err;
- if (!tb[TCA_SKBMOD_PARMS])
+ if (!tb[TCA_SKBMOD_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required skbmod parameters");
return -EINVAL;
+ }
if (tb[TCA_SKBMOD_DMAC]) {
daddr = nla_data(tb[TCA_SKBMOD_DMAC]);
@@ -160,6 +164,7 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
return ACT_P_BOUND;
if (!lflags) {
+ NL_SET_ERR_MSG_MOD(extack, "No skbmod action flag");
if (exists)
tcf_idr_release(*a, bind);
else
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 836183011a7c..b468a4c8a904 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -134,16 +134,20 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
int ret = 0, err;
u32 index;
- if (!nla)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Vlan requires attributes to be passed");
return -EINVAL;
+ }
err = nla_parse_nested_deprecated(tb, TCA_VLAN_MAX, nla, vlan_policy,
NULL);
if (err < 0)
return err;
- if (!tb[TCA_VLAN_PARMS])
+ if (!tb[TCA_VLAN_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required vlan action parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_VLAN_PARMS]);
index = parm->index;
err = tcf_idr_check_alloc(tn, &index, a, bind);
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net-next] net/sched: report errors with extack
2024-01-31 16:58 [PATCH net-next] net/sched: report errors with extack Stephen Hemminger
@ 2024-01-31 19:53 ` Jamal Hadi Salim
0 siblings, 0 replies; 2+ messages in thread
From: Jamal Hadi Salim @ 2024-01-31 19:53 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Martin KaFai Lau, Daniel Borkmann, John Fastabend,
Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Cong Wang, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni,
open list:BPF [NETWORKING] (tcx & tc BPF, sock_addr),
open list
On Wed, Jan 31, 2024 at 12:00 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> While working a BPF action, found that the error handling was
> limited. The support of external ack was only added to some
> but not all actions.
>
> When an action detects invalid parameters, it should
> be adding an external ack to netlink so that the user is
> able to diagnose the issue.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> net/sched/act_bpf.c | 31 ++++++++++++++++++++++---------
> net/sched/act_connmark.c | 8 ++++++--
> net/sched/act_csum.c | 8 ++++++--
> net/sched/act_gact.c | 14 +++++++++++---
> net/sched/act_gate.c | 15 +++++++++++----
> net/sched/act_ife.c | 8 ++++++--
> net/sched/act_nat.c | 9 +++++++--
> net/sched/act_police.c | 13 ++++++++++---
> net/sched/act_sample.c | 8 ++++++--
> net/sched/act_simple.c | 9 +++++++--
> net/sched/act_skbedit.c | 13 ++++++++++---
> net/sched/act_skbmod.c | 9 +++++++--
> net/sched/act_vlan.c | 8 ++++++--
> 13 files changed, 115 insertions(+), 38 deletions(-)
>
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index 6cfee6658103..f8a03d3bbf20 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -184,7 +184,8 @@ static const struct nla_policy act_bpf_policy[TCA_ACT_BPF_MAX + 1] = {
> .len = sizeof(struct sock_filter) * BPF_MAXINSNS },
> };
>
> -static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
> +static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg,
> + struct netlink_ext_ack *extack)
> {
> struct sock_filter *bpf_ops;
> struct sock_fprog_kern fprog_tmp;
> @@ -193,12 +194,16 @@ static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
> int ret;
>
> bpf_num_ops = nla_get_u16(tb[TCA_ACT_BPF_OPS_LEN]);
> - if (bpf_num_ops > BPF_MAXINSNS || bpf_num_ops == 0)
> + if (bpf_num_ops > BPF_MAXINSNS || bpf_num_ops == 0) {
> + NL_SET_ERR_MSG_MOD(extack, "Invalid number of BPF instructions");
> return -EINVAL;
> + }
>
> bpf_size = bpf_num_ops * sizeof(*bpf_ops);
> - if (bpf_size != nla_len(tb[TCA_ACT_BPF_OPS]))
> + if (bpf_size != nla_len(tb[TCA_ACT_BPF_OPS])) {
> + NL_SET_ERR_MSG_MOD(extack, "BPF instruction size");
> return -EINVAL;
> + }
>
> bpf_ops = kmemdup(nla_data(tb[TCA_ACT_BPF_OPS]), bpf_size, GFP_KERNEL);
> if (bpf_ops == NULL)
> @@ -221,7 +226,8 @@ static int tcf_bpf_init_from_ops(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
> return 0;
> }
>
> -static int tcf_bpf_init_from_efd(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
> +static int tcf_bpf_init_from_efd(struct nlattr **tb, struct tcf_bpf_cfg *cfg,
> + struct netlink_ext_ack *extack)
> {
> struct bpf_prog *fp;
> char *name = NULL;
> @@ -230,8 +236,10 @@ static int tcf_bpf_init_from_efd(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
> bpf_fd = nla_get_u32(tb[TCA_ACT_BPF_FD]);
>
> fp = bpf_prog_get_type(bpf_fd, BPF_PROG_TYPE_SCHED_ACT);
> - if (IS_ERR(fp))
> + if (IS_ERR(fp)) {
> + NL_SET_ERR_MSG_MOD(extack, "BPF program type mismatch");
> return PTR_ERR(fp);
> + }
>
> if (tb[TCA_ACT_BPF_NAME]) {
> name = nla_memdup(tb[TCA_ACT_BPF_NAME], GFP_KERNEL);
> @@ -292,16 +300,20 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
> int ret, res = 0;
> u32 index;
>
> - if (!nla)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Bpf requires attributes to be passed");
> return -EINVAL;
> + }
>
> ret = nla_parse_nested_deprecated(tb, TCA_ACT_BPF_MAX, nla,
> act_bpf_policy, NULL);
> if (ret < 0)
> return ret;
>
> - if (!tb[TCA_ACT_BPF_PARMS])
> + if (!tb[TCA_ACT_BPF_PARMS]) {
if (NL_REQ_ATTR_CHECK(extack, NULL, tb, TCA_ACT_BPF_PARMS) to set the
other extack fields
> + NL_SET_ERR_MSG_MOD(extack, "Missing required bpf parameters");
> return -EINVAL;
> + }
>
> parm = nla_data(tb[TCA_ACT_BPF_PARMS]);
> index = parm->index;
> @@ -336,14 +348,15 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
> is_ebpf = tb[TCA_ACT_BPF_FD];
>
> if (is_bpf == is_ebpf) {
> + NL_SET_ERR_MSG_MOD(extack, "Can not specify both BPF fd and ops");
> ret = -EINVAL;
> goto put_chain;
> }
>
> memset(&cfg, 0, sizeof(cfg));
>
> - ret = is_bpf ? tcf_bpf_init_from_ops(tb, &cfg) :
> - tcf_bpf_init_from_efd(tb, &cfg);
> + ret = is_bpf ? tcf_bpf_init_from_ops(tb, &cfg, extack) :
> + tcf_bpf_init_from_efd(tb, &cfg, extack);
> if (ret < 0)
> goto put_chain;
>
> diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
> index f8762756657d..0964d10dfc04 100644
> --- a/net/sched/act_connmark.c
> +++ b/net/sched/act_connmark.c
> @@ -110,16 +110,20 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
> int ret = 0, err;
> u32 index;
>
> - if (!nla)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Connmark requires attributes to be passed");
> return -EINVAL;
> + }
>
> ret = nla_parse_nested_deprecated(tb, TCA_CONNMARK_MAX, nla,
> connmark_policy, NULL);
> if (ret < 0)
> return ret;
>
> - if (!tb[TCA_CONNMARK_PARMS])
> + if (!tb[TCA_CONNMARK_PARMS]) {
Same thing..
> + NL_SET_ERR_MSG(extack, "Missing required connmark parameters");
> return -EINVAL;
> + }
>
> nparms = kzalloc(sizeof(*nparms), GFP_KERNEL);
> if (!nparms)
> diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
> index 7f8b1f2f2ed9..7c7f74e37528 100644
> --- a/net/sched/act_csum.c
> +++ b/net/sched/act_csum.c
> @@ -55,16 +55,20 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
> int ret = 0, err;
> u32 index;
>
> - if (nla == NULL)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Checksum requires attributes to be passed");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_CSUM_MAX, nla, csum_policy,
> NULL);
> if (err < 0)
> return err;
>
> - if (tb[TCA_CSUM_PARMS] == NULL)
> + if (!tb[TCA_CSUM_PARMS]) {
Same thing...
> + NL_SET_ERR_MSG(extack, "Missing required checksum parameters");
> return -EINVAL;
> + }
> parm = nla_data(tb[TCA_CSUM_PARMS]);
> index = parm->index;
> err = tcf_idr_check_alloc(tn, &index, a, bind);
> diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
> index 4af3b7ec249f..5d04bcd5115e 100644
> --- a/net/sched/act_gact.c
> +++ b/net/sched/act_gact.c
> @@ -68,16 +68,21 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
> struct tc_gact_p *p_parm = NULL;
> #endif
>
> - if (nla == NULL)
> + if (!nla) {
> + NL_SET_ERR_MSG(extack, "Gact requires attributes to be passed");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_GACT_MAX, nla, gact_policy,
> NULL);
> if (err < 0)
> return err;
>
> - if (tb[TCA_GACT_PARMS] == NULL)
> + if (!tb[TCA_GACT_PARMS]) {
Same..
> + NL_SET_ERR_MSG_MOD(extack, "Missing required gact parameters");
> return -EINVAL;
> + }
> +
> parm = nla_data(tb[TCA_GACT_PARMS]);
> index = parm->index;
>
> @@ -87,8 +92,11 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
> #else
> if (tb[TCA_GACT_PROB]) {
> p_parm = nla_data(tb[TCA_GACT_PROB]);
> - if (p_parm->ptype >= MAX_RAND)
> + if (p_parm->ptype >= MAX_RAND) {
> + NL_SET_ERR_MSG(extack, "Invalid ptype in gact prob");
> return -EINVAL;
> + }
> +
> if (TC_ACT_EXT_CMP(p_parm->paction, TC_ACT_GOTO_CHAIN)) {
> NL_SET_ERR_MSG(extack,
> "goto chain not allowed on fallback");
> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index c681cd011afd..c9e32822938c 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -239,8 +239,10 @@ static int parse_gate_list(struct nlattr *list_attr,
> int err, rem;
> int i = 0;
>
> - if (!list_attr)
> + if (!list_attr) {
> + NL_SET_ERR_MSG(extack, "Gate missing attributes");
> return -EINVAL;
> + }
>
> nla_for_each_nested(n, list_attr, rem) {
> if (nla_type(n) != TCA_GATE_ONE_ENTRY) {
> @@ -317,15 +319,19 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
> ktime_t start;
> u32 index;
>
> - if (!nla)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Gate requires attributes to be passed");
> return -EINVAL;
> + }
>
> err = nla_parse_nested(tb, TCA_GATE_MAX, nla, gate_policy, extack);
> if (err < 0)
> return err;
>
> - if (!tb[TCA_GATE_PARMS])
> + if (!tb[TCA_GATE_PARMS]) {
Here...
> + NL_SET_ERR_MSG_MOD(extack, "Missing required gate parameters");
> return -EINVAL;
> + }
>
> if (tb[TCA_GATE_CLOCKID]) {
> clockid = nla_get_s32(tb[TCA_GATE_CLOCKID]);
> @@ -343,7 +349,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
> tk_offset = TK_OFFS_TAI;
> break;
> default:
> - NL_SET_ERR_MSG(extack, "Invalid 'clockid'");
> + NL_SET_ERR_MSG_MOD(extack, "Invalid 'clockid'");
> return -EINVAL;
> }
> }
> @@ -409,6 +415,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
> cycle = ktime_add_ns(cycle, entry->interval);
> cycletime = cycle;
> if (!cycletime) {
> + NL_SET_ERR_MSG_MOD(extack, "cycle time is zero");
> err = -EINVAL;
> goto chain_put;
> }
> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> index 0e867d13beb5..85a58cfb23f3 100644
> --- a/net/sched/act_ife.c
> +++ b/net/sched/act_ife.c
> @@ -508,8 +508,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
> if (err < 0)
> return err;
>
> - if (!tb[TCA_IFE_PARMS])
> + if (!tb[TCA_IFE_PARMS]) {
Here...
Going to stop here - there are more further down. You get the gist..
cheers,
jamal
> + NL_SET_ERR_MSG_MOD(extack, "Missing required ife parameters");
> return -EINVAL;
> + }
>
> parm = nla_data(tb[TCA_IFE_PARMS]);
>
> @@ -517,8 +519,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
> * they cannot run as the same time. Check on all other values which
> * are not supported right now.
> */
> - if (parm->flags & ~IFE_ENCODE)
> + if (parm->flags & ~IFE_ENCODE) {
> + NL_SET_ERR_MSG_MOD(extack, "Invalid ife flag parameter");
> return -EINVAL;
> + }
>
> p = kzalloc(sizeof(*p), GFP_KERNEL);
> if (!p)
> diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> index a180e724634e..a990d0c626cd 100644
> --- a/net/sched/act_nat.c
> +++ b/net/sched/act_nat.c
> @@ -46,16 +46,21 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
> struct tcf_nat *p;
> u32 index;
>
> - if (nla == NULL)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Nat action requires attributes");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_NAT_MAX, nla, nat_policy,
> NULL);
> if (err < 0)
> return err;
>
> - if (tb[TCA_NAT_PARMS] == NULL)
> + if (!tb[TCA_NAT_PARMS]) {
> + NL_SET_ERR_MSG_MOD(extack, "Nat action parameters missing");
> return -EINVAL;
> + }
> +
> parm = nla_data(tb[TCA_NAT_PARMS]);
> index = parm->index;
> err = tcf_idr_check_alloc(tn, &index, a, bind);
> diff --git a/net/sched/act_police.c b/net/sched/act_police.c
> index e119b4a3db9f..3eb41233257b 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -56,19 +56,26 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
> u64 rate64, prate64;
> u64 pps, ppsburst;
>
> - if (nla == NULL)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Police requires attributes");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_POLICE_MAX, nla,
> police_policy, NULL);
> if (err < 0)
> return err;
>
> - if (tb[TCA_POLICE_TBF] == NULL)
> + if (!tb[TCA_POLICE_TBF]) {
> + NL_SET_ERR_MSG_MOD(extack, "Missing required police action parameters");
> return -EINVAL;
> + }
> +
> size = nla_len(tb[TCA_POLICE_TBF]);
> - if (size != sizeof(*parm) && size != sizeof(struct tc_police_compat))
> + if (size != sizeof(*parm) && size != sizeof(struct tc_police_compat)) {
> + NL_SET_ERR_MSG_MOD(extack, "Invalid size for police action parameters");
> return -EINVAL;
> + }
>
> parm = nla_data(tb[TCA_POLICE_TBF]);
> index = parm->index;
> diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
> index c5c61efe6db4..2442e001d92e 100644
> --- a/net/sched/act_sample.c
> +++ b/net/sched/act_sample.c
> @@ -49,15 +49,19 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
> bool exists = false;
> int ret, err;
>
> - if (!nla)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Sample requires attributes to be passed");
> return -EINVAL;
> + }
> ret = nla_parse_nested_deprecated(tb, TCA_SAMPLE_MAX, nla,
> sample_policy, NULL);
> if (ret < 0)
> return ret;
>
> - if (!tb[TCA_SAMPLE_PARMS])
> + if (!tb[TCA_SAMPLE_PARMS]) {
> + NL_SET_ERR_MSG_MOD(extack, "Missing required sample action parameters");
> return -EINVAL;
> + }
>
> parm = nla_data(tb[TCA_SAMPLE_PARMS]);
> index = parm->index;
> diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
> index 0a3e92888295..02b8e42c1bdd 100644
> --- a/net/sched/act_simple.c
> +++ b/net/sched/act_simple.c
> @@ -100,16 +100,20 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
> int ret = 0, err;
> u32 index;
>
> - if (nla == NULL)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Sample requires attributes to be passed");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_DEF_MAX, nla, simple_policy,
> NULL);
> if (err < 0)
> return err;
>
> - if (tb[TCA_DEF_PARMS] == NULL)
> + if (!tb[TCA_DEF_PARMS]) {
> + NL_SET_ERR_MSG_MOD(extack, "Missing required sample action parameters");
> return -EINVAL;
> + }
>
> parm = nla_data(tb[TCA_DEF_PARMS]);
> index = parm->index;
> @@ -121,6 +125,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
> return ACT_P_BOUND;
>
> if (tb[TCA_DEF_DATA] == NULL) {
> + NL_SET_ERR_MSG_MOD(extack, "Missing simple action default data");
> if (exists)
> tcf_idr_release(*a, bind);
> else
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index 754f78b35bb8..671ca64a2c33 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -133,16 +133,20 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
> int ret = 0, err;
> u32 index;
>
> - if (nla == NULL)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Skbedit requires attributes to be passed");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_SKBEDIT_MAX, nla,
> skbedit_policy, NULL);
> if (err < 0)
> return err;
>
> - if (tb[TCA_SKBEDIT_PARMS] == NULL)
> + if (!tb[TCA_SKBEDIT_PARMS]) {
> + NL_SET_ERR_MSG_MOD(extack, "Missing required skbedit parameters");
> return -EINVAL;
> + }
>
> if (tb[TCA_SKBEDIT_PRIORITY] != NULL) {
> flags |= SKBEDIT_F_PRIORITY;
> @@ -161,8 +165,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
>
> if (tb[TCA_SKBEDIT_PTYPE] != NULL) {
> ptype = nla_data(tb[TCA_SKBEDIT_PTYPE]);
> - if (!skb_pkt_type_ok(*ptype))
> + if (!skb_pkt_type_ok(*ptype)) {
> + NL_SET_ERR_MSG_MOD(extack, "ptype is not a valid");
> return -EINVAL;
> + }
> flags |= SKBEDIT_F_PTYPE;
> }
>
> @@ -212,6 +218,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
> return ACT_P_BOUND;
>
> if (!flags) {
> + NL_SET_ERR_MSG_MOD(extack, "No skbedit action flag");
> if (exists)
> tcf_idr_release(*a, bind);
> else
> diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
> index bcb673ab0008..c80828cdeb69 100644
> --- a/net/sched/act_skbmod.c
> +++ b/net/sched/act_skbmod.c
> @@ -119,16 +119,20 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
> u16 eth_type = 0;
> int ret = 0, err;
>
> - if (!nla)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Skbmod requires attributes to be passed");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_SKBMOD_MAX, nla,
> skbmod_policy, NULL);
> if (err < 0)
> return err;
>
> - if (!tb[TCA_SKBMOD_PARMS])
> + if (!tb[TCA_SKBMOD_PARMS]) {
> + NL_SET_ERR_MSG_MOD(extack, "Missing required skbmod parameters");
> return -EINVAL;
> + }
>
> if (tb[TCA_SKBMOD_DMAC]) {
> daddr = nla_data(tb[TCA_SKBMOD_DMAC]);
> @@ -160,6 +164,7 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
> return ACT_P_BOUND;
>
> if (!lflags) {
> + NL_SET_ERR_MSG_MOD(extack, "No skbmod action flag");
> if (exists)
> tcf_idr_release(*a, bind);
> else
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index 836183011a7c..b468a4c8a904 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -134,16 +134,20 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
> int ret = 0, err;
> u32 index;
>
> - if (!nla)
> + if (!nla) {
> + NL_SET_ERR_MSG_MOD(extack, "Vlan requires attributes to be passed");
> return -EINVAL;
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_VLAN_MAX, nla, vlan_policy,
> NULL);
> if (err < 0)
> return err;
>
> - if (!tb[TCA_VLAN_PARMS])
> + if (!tb[TCA_VLAN_PARMS]) {
> + NL_SET_ERR_MSG_MOD(extack, "Missing required vlan action parameters");
> return -EINVAL;
> + }
> parm = nla_data(tb[TCA_VLAN_PARMS]);
> index = parm->index;
> err = tcf_idr_check_alloc(tn, &index, a, bind);
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-01-31 19:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-31 16:58 [PATCH net-next] net/sched: report errors with extack Stephen Hemminger
2024-01-31 19:53 ` Jamal Hadi Salim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).