* [PATCHv2 net-next 0/8] net: sched: act: add extack support
@ 2018-02-14 22:13 Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 1/8] net: sched: act: fix code style Alexander Aring
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
Hi,
this patch series adds extack support for the TC action subsystem.
As example I for the extack support in a TC action I choosed mirred
action.
- Alex
Cc: David Ahern <dsahern@gmail.com>
changes since v2:
- remove newline in extack of generic walker handling
Thanks to Davide Caratti
- add kernel@mojatatu.com in cc
Alexander Aring (8):
net: sched: act: fix code style
net: sched: act: add extack to init
net: sched: act: handle generic action errors
net: sched: act: add extack to init callback
net: sched: act: add extack for lookup callback
net: sched: act: add extack for walk callback
net: sched: act: handle extack in tcf_generic_walker
net: sched: act: mirred: add extack support
include/net/act_api.h | 17 ++++--
net/sched/act_api.c | 135 +++++++++++++++++++++++++++++----------------
net/sched/act_bpf.c | 10 ++--
net/sched/act_connmark.c | 11 ++--
net/sched/act_csum.c | 10 ++--
net/sched/act_gact.c | 10 ++--
net/sched/act_ife.c | 10 ++--
net/sched/act_ipt.c | 20 ++++---
net/sched/act_mirred.c | 25 ++++++---
net/sched/act_nat.c | 11 ++--
net/sched/act_pedit.c | 10 ++--
net/sched/act_police.c | 11 ++--
net/sched/act_sample.c | 10 ++--
net/sched/act_simple.c | 10 ++--
net/sched/act_skbedit.c | 10 ++--
net/sched/act_skbmod.c | 10 ++--
net/sched/act_tunnel_key.c | 10 ++--
net/sched/act_vlan.c | 10 ++--
net/sched/cls_api.c | 4 +-
19 files changed, 215 insertions(+), 129 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 1/8] net: sched: act: fix code style
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 2/8] net: sched: act: add extack to init Alexander Aring
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem; +Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring
This patch is used by subsequent patches. It fixes code style issues
caught by checkpatch.
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
include/net/act_api.h | 5 +++--
net/sched/act_api.c | 12 ++++++------
net/sched/act_mirred.c | 6 +++---
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 6ed9692f20bd..32ef544f4ddc 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -87,12 +87,13 @@ struct tc_action_ops {
struct tcf_result *);
int (*dump)(struct sk_buff *, struct tc_action *, int, int);
void (*cleanup)(struct tc_action *);
- int (*lookup)(struct net *, struct tc_action **, u32);
+ int (*lookup)(struct net *net, struct tc_action **a, u32 index);
int (*init)(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **act, int ovr,
int bind);
int (*walk)(struct net *, struct sk_buff *,
- struct netlink_callback *, int, const struct tc_action_ops *);
+ struct netlink_callback *, int,
+ const struct tc_action_ops *);
void (*stats_update)(struct tc_action *, u64, u32, u64);
struct net_device *(*get_dev)(const struct tc_action *a);
};
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 4886ea4a7d6e..becc63689fae 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -621,7 +621,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
goto err_out;
err = -EINVAL;
kind = tb[TCA_ACT_KIND];
- if (kind == NULL)
+ if (!kind)
goto err_out;
if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
goto err_out;
@@ -822,7 +822,7 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
t->tca__pad2 = 0;
nest = nla_nest_start(skb, TCA_ACT_TAB);
- if (nest == NULL)
+ if (!nest)
goto out_nlmsg_trim;
if (tcf_action_dump(skb, actions, bind, ref) < 0)
@@ -934,7 +934,7 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
t->tca__pad2 = 0;
nest = nla_nest_start(skb, TCA_ACT_TAB);
- if (nest == NULL)
+ if (!nest)
goto out_module_put;
err = ops->walk(net, skb, &dcb, RTM_DELACTION, ops);
@@ -1005,10 +1005,10 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
return ret;
if (event == RTM_DELACTION && n->nlmsg_flags & NLM_F_ROOT) {
- if (tb[1] != NULL)
+ if (tb[1])
return tca_action_flush(net, tb[1], n, portid);
- else
- return -EINVAL;
+
+ return -EINVAL;
}
for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index e6ff88f72900..abcd5f12b913 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -80,12 +80,12 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
bool exists = false;
int ret;
- if (nla == NULL)
+ if (!nla)
return -EINVAL;
ret = nla_parse_nested(tb, TCA_MIRRED_MAX, nla, mirred_policy, NULL);
if (ret < 0)
return ret;
- if (tb[TCA_MIRRED_PARMS] == NULL)
+ if (!tb[TCA_MIRRED_PARMS])
return -EINVAL;
parm = nla_data(tb[TCA_MIRRED_PARMS]);
@@ -117,7 +117,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
}
if (!exists) {
- if (dev == NULL)
+ if (!dev)
return -EINVAL;
ret = tcf_idr_create(tn, parm->index, est, a,
&act_mirred_ops, bind, true);
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 2/8] net: sched: act: add extack to init
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 1/8] net: sched: act: fix code style Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors Alexander Aring
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
This patch adds extack to tcf_action_init and tcf_action_init_1
functions. These are necessary to make individual extack handling in
each act implementation.
Based on work by David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
include/net/act_api.h | 5 +++--
net/sched/act_api.c | 17 +++++++++++------
net/sched/cls_api.c | 4 ++--
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 32ef544f4ddc..41d95930ffbc 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -163,10 +163,11 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
int nr_actions, struct tcf_result *res);
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
- struct list_head *actions);
+ struct list_head *actions, struct netlink_ext_ack *extack);
struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
- char *name, int ovr, int bind);
+ char *name, int ovr, int bind,
+ struct netlink_ext_ack *extack);
int tcf_action_dump(struct sk_buff *skb, struct list_head *, int, int);
int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index becc63689fae..8d89b026414f 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -605,7 +605,8 @@ static struct tc_cookie *nla_memdup_cookie(struct nlattr **tb)
struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
- char *name, int ovr, int bind)
+ char *name, int ovr, int bind,
+ struct netlink_ext_ack *extack)
{
struct tc_action *a;
struct tc_action_ops *a_o;
@@ -726,7 +727,7 @@ static void cleanup_a(struct list_head *actions, int ovr)
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
- struct list_head *actions)
+ struct list_head *actions, struct netlink_ext_ack *extack)
{
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
@@ -738,7 +739,8 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
return err;
for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {
- act = tcf_action_init_1(net, tp, tb[i], est, name, ovr, bind);
+ act = tcf_action_init_1(net, tp, tb[i], est, name, ovr, bind,
+ extack);
if (IS_ERR(act)) {
err = PTR_ERR(act);
goto err;
@@ -1060,12 +1062,14 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
}
static int tcf_action_add(struct net *net, struct nlattr *nla,
- struct nlmsghdr *n, u32 portid, int ovr)
+ struct nlmsghdr *n, u32 portid, int ovr,
+ struct netlink_ext_ack *extack)
{
int ret = 0;
LIST_HEAD(actions);
- ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, &actions);
+ ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, &actions,
+ extack);
if (ret)
return ret;
@@ -1113,7 +1117,8 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
if (n->nlmsg_flags & NLM_F_REPLACE)
ovr = 1;
replay:
- ret = tcf_action_add(net, tca[TCA_ACT_TAB], n, portid, ovr);
+ ret = tcf_action_add(net, tca[TCA_ACT_TAB], n, portid, ovr,
+ extack);
if (ret == -EAGAIN)
goto replay;
break;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2bc1bc23d42e..f21610c5da1a 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1434,7 +1434,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
if (exts->police && tb[exts->police]) {
act = tcf_action_init_1(net, tp, tb[exts->police],
rate_tlv, "police", ovr,
- TCA_ACT_BIND);
+ TCA_ACT_BIND, extack);
if (IS_ERR(act))
return PTR_ERR(act);
@@ -1447,7 +1447,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
err = tcf_action_init(net, tp, tb[exts->action],
rate_tlv, NULL, ovr, TCA_ACT_BIND,
- &actions);
+ &actions, extack);
if (err)
return err;
list_for_each_entry(act, &actions, list)
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 1/8] net: sched: act: fix code style Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 2/8] net: sched: act: add extack to init Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
2018-02-15 11:14 ` Davide Caratti
2018-02-14 22:13 ` [PATCHv2 net-next 4/8] net: sched: act: add extack to init callback Alexander Aring
` (4 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
This patch adds extack support for generic act handling. The extack
will be set deeper to each called function which is not part of netdev
core api.
Based on work by David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
net/sched/act_api.c | 93 +++++++++++++++++++++++++++++++++++------------------
1 file changed, 61 insertions(+), 32 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 8d89b026414f..a5138ae026a1 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -617,31 +617,40 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
int err;
if (name == NULL) {
- err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, NULL);
+ err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, extack);
if (err < 0)
goto err_out;
err = -EINVAL;
kind = tb[TCA_ACT_KIND];
- if (!kind)
+ if (!kind) {
+ NL_SET_ERR_MSG(extack, "TC action kind must be specified");
goto err_out;
- if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
+ }
+ if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ) {
+ NL_SET_ERR_MSG(extack, "TC action name too long");
goto err_out;
+ }
if (tb[TCA_ACT_COOKIE]) {
int cklen = nla_len(tb[TCA_ACT_COOKIE]);
- if (cklen > TC_COOKIE_MAX_SIZE)
+ if (cklen > TC_COOKIE_MAX_SIZE) {
+ NL_SET_ERR_MSG(extack, "TC cookie size above the maximum");
goto err_out;
+ }
cookie = nla_memdup_cookie(tb);
if (!cookie) {
+ NL_SET_ERR_MSG(extack, "No memory to generate TC cookie");
err = -ENOMEM;
goto err_out;
}
}
} else {
- err = -EINVAL;
- if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ)
+ if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ) {
+ NL_SET_ERR_MSG(extack, "TC action name too long");
+ err = -EINVAL;
goto err_out;
+ }
}
a_o = tc_lookup_action_n(act_name);
@@ -664,6 +673,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
goto err_mod;
}
#endif
+ NL_SET_ERR_MSG(extack, "Failed to load TC action module");
err = -ENOENT;
goto err_out;
}
@@ -698,6 +708,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
list_add_tail(&a->list, &actions);
tcf_action_destroy(&actions, bind);
+ NL_SET_ERR_MSG(extack, "Failed to init action chain");
return ERR_PTR(err);
}
}
@@ -734,7 +745,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
int err;
int i;
- err = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, NULL);
+ err = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
if (err < 0)
return err;
@@ -842,7 +853,8 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
static int
tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
- struct list_head *actions, int event)
+ struct list_head *actions, int event,
+ struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
@@ -851,6 +863,7 @@ tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
return -ENOBUFS;
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
0, 0) <= 0) {
+ NL_SET_ERR_MSG(extack, "Failed to fill netlink tc action attributes");
kfree_skb(skb);
return -EINVAL;
}
@@ -859,7 +872,8 @@ tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
}
static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
- struct nlmsghdr *n, u32 portid)
+ struct nlmsghdr *n, u32 portid,
+ struct netlink_ext_ack *extack)
{
struct nlattr *tb[TCA_ACT_MAX + 1];
const struct tc_action_ops *ops;
@@ -867,20 +881,24 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
int index;
int err;
- err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, NULL);
+ err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, extack);
if (err < 0)
goto err_out;
err = -EINVAL;
if (tb[TCA_ACT_INDEX] == NULL ||
- nla_len(tb[TCA_ACT_INDEX]) < sizeof(index))
+ nla_len(tb[TCA_ACT_INDEX]) < sizeof(index)) {
+ NL_SET_ERR_MSG(extack, "Invalid TC action index value");
goto err_out;
+ }
index = nla_get_u32(tb[TCA_ACT_INDEX]);
err = -EINVAL;
ops = tc_lookup_action(tb[TCA_ACT_KIND]);
- if (!ops) /* could happen in batch of actions */
+ if (!ops) { /* could happen in batch of actions */
+ NL_SET_ERR_MSG(extack, "Specified TC action not found");
goto err_out;
+ }
err = -ENOENT;
if (ops->lookup(net, &a, index) == 0)
goto err_mod;
@@ -895,7 +913,8 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
}
static int tca_action_flush(struct net *net, struct nlattr *nla,
- struct nlmsghdr *n, u32 portid)
+ struct nlmsghdr *n, u32 portid,
+ struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
unsigned char *b;
@@ -909,35 +928,39 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
int err = -ENOMEM;
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
- if (!skb) {
- pr_debug("tca_action_flush: failed skb alloc\n");
+ if (!skb)
return err;
- }
b = skb_tail_pointer(skb);
- err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, NULL);
+ err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, extack);
if (err < 0)
goto err_out;
err = -EINVAL;
kind = tb[TCA_ACT_KIND];
ops = tc_lookup_action(kind);
- if (!ops) /*some idjot trying to flush unknown action */
+ if (!ops) { /*some idjot trying to flush unknown action */
+ NL_SET_ERR_MSG(extack, "Aborted Flush. Cannot flush unknown TC action");
goto err_out;
+ }
nlh = nlmsg_put(skb, portid, n->nlmsg_seq, RTM_DELACTION,
sizeof(*t), 0);
- if (!nlh)
+ if (!nlh) {
+ NL_SET_ERR_MSG(extack, "Aborted Flush. Failed to create flush netlink notification");
goto out_module_put;
+ }
t = nlmsg_data(nlh);
t->tca_family = AF_UNSPEC;
t->tca__pad1 = 0;
t->tca__pad2 = 0;
nest = nla_nest_start(skb, TCA_ACT_TAB);
- if (!nest)
+ if (!nest) {
+ NL_SET_ERR_MSG(extack, "Failed to add new netlink message");
goto out_module_put;
+ }
err = ops->walk(net, skb, &dcb, RTM_DELACTION, ops);
if (err <= 0)
@@ -952,6 +975,8 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
n->nlmsg_flags & NLM_F_ECHO);
if (err > 0)
return 0;
+ if (err < 0)
+ NL_SET_ERR_MSG(extack, "Failed to send flush notification");
return err;
@@ -964,7 +989,7 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
static int
tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
- u32 portid)
+ u32 portid, struct netlink_ext_ack *extack)
{
int ret;
struct sk_buff *skb;
@@ -975,6 +1000,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
0, 1) <= 0) {
+ NL_SET_ERR_MSG(extack, "Failed to fill netlink tc action attributes");
kfree_skb(skb);
return -EINVAL;
}
@@ -982,6 +1008,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
/* now do the delete */
ret = tcf_action_destroy(actions, 0);
if (ret < 0) {
+ NL_SET_ERR_MSG(extack, "Failed to delete TC action");
kfree_skb(skb);
return ret;
}
@@ -995,26 +1022,27 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
static int
tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
- u32 portid, int event)
+ u32 portid, int event, struct netlink_ext_ack *extack)
{
int i, ret;
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
LIST_HEAD(actions);
- ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, NULL);
+ ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
if (ret < 0)
return ret;
if (event == RTM_DELACTION && n->nlmsg_flags & NLM_F_ROOT) {
if (tb[1])
- return tca_action_flush(net, tb[1], n, portid);
+ return tca_action_flush(net, tb[1], n, portid, extack);
+ NL_SET_ERR_MSG(extack, "Invalid netlink attributes to flush actions");
return -EINVAL;
}
for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {
- act = tcf_action_get_1(net, tb[i], n, portid);
+ act = tcf_action_get_1(net, tb[i], n, portid, extack);
if (IS_ERR(act)) {
ret = PTR_ERR(act);
goto err;
@@ -1024,9 +1052,9 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
}
if (event == RTM_GETACTION)
- ret = tcf_get_notify(net, portid, n, &actions, event);
+ ret = tcf_get_notify(net, portid, n, &actions, event, extack);
else { /* delete */
- ret = tcf_del_notify(net, n, &actions, portid);
+ ret = tcf_del_notify(net, n, &actions, portid, extack);
if (ret)
goto err;
return ret;
@@ -1039,7 +1067,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
static int
tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
- u32 portid)
+ u32 portid, struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
int err = 0;
@@ -1050,6 +1078,7 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
RTM_NEWACTION, 0, 0) <= 0) {
+ NL_SET_ERR_MSG(extack, "Failed to fill netlink tc action attributes");
kfree_skb(skb);
return -EINVAL;
}
@@ -1073,7 +1102,7 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
if (ret)
return ret;
- return tcf_add_notify(net, n, &actions, portid);
+ return tcf_add_notify(net, n, &actions, portid, extack);
}
static u32 tcaa_root_flags_allowed = TCA_FLAG_LARGE_DUMP_ON;
@@ -1101,7 +1130,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
return ret;
if (tca[TCA_ACT_TAB] == NULL) {
- pr_notice("tc_ctl_action: received NO action attribs\n");
+ NL_SET_ERR_MSG(extack, "Netlink Action attributes missing");
return -EINVAL;
}
@@ -1124,11 +1153,11 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
break;
case RTM_DELACTION:
ret = tca_action_gd(net, tca[TCA_ACT_TAB], n,
- portid, RTM_DELACTION);
+ portid, RTM_DELACTION, extack);
break;
case RTM_GETACTION:
ret = tca_action_gd(net, tca[TCA_ACT_TAB], n,
- portid, RTM_GETACTION);
+ portid, RTM_GETACTION, extack);
break;
default:
BUG();
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 4/8] net: sched: act: add extack to init callback
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
` (2 preceding siblings ...)
2018-02-14 22:13 ` [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 5/8] net: sched: act: add extack for lookup callback Alexander Aring
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
This patch adds extack support for act init callback api. This
prepares to handle extack support inside each specific act
implementation.
Based on work by David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
include/net/act_api.h | 2 +-
net/sched/act_api.c | 5 +++--
net/sched/act_bpf.c | 2 +-
net/sched/act_connmark.c | 3 ++-
net/sched/act_csum.c | 2 +-
net/sched/act_gact.c | 2 +-
net/sched/act_ife.c | 2 +-
net/sched/act_ipt.c | 4 ++--
net/sched/act_mirred.c | 2 +-
net/sched/act_nat.c | 3 ++-
net/sched/act_pedit.c | 2 +-
net/sched/act_police.c | 3 ++-
net/sched/act_sample.c | 2 +-
net/sched/act_simple.c | 2 +-
net/sched/act_skbedit.c | 2 +-
net/sched/act_skbmod.c | 2 +-
net/sched/act_tunnel_key.c | 2 +-
net/sched/act_vlan.c | 2 +-
18 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 41d95930ffbc..3717e0f2bb1b 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -90,7 +90,7 @@ struct tc_action_ops {
int (*lookup)(struct net *net, struct tc_action **a, u32 index);
int (*init)(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **act, int ovr,
- int bind);
+ int bind, struct netlink_ext_ack *extack);
int (*walk)(struct net *, struct sk_buff *,
struct netlink_callback *, int,
const struct tc_action_ops *);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a5138ae026a1..00c5a1d9a21e 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -680,9 +680,10 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
/* backward compatibility for policer */
if (name == NULL)
- err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, &a, ovr, bind);
+ err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, &a, ovr, bind,
+ extack);
else
- err = a_o->init(net, nla, est, &a, ovr, bind);
+ err = a_o->init(net, nla, est, &a, ovr, bind, extack);
if (err < 0)
goto err_mod;
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index b3f2c15affa7..b3ebfa9598e2 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -272,7 +272,7 @@ static void tcf_bpf_prog_fill_cfg(const struct tcf_bpf *prog,
static int tcf_bpf_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **act,
- int replace, int bind)
+ int replace, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, bpf_net_id);
struct nlattr *tb[TCA_ACT_BPF_MAX + 1];
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 2b15ba84e0c8..20e0215360b5 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -96,7 +96,8 @@ static const struct nla_policy connmark_policy[TCA_CONNMARK_MAX + 1] = {
static int tcf_connmark_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, connmark_net_id);
struct nlattr *tb[TCA_CONNMARK_MAX + 1];
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index b7ba9b06b147..3b8c48bb2683 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -46,7 +46,7 @@ static struct tc_action_ops act_csum_ops;
static int tcf_csum_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind)
+ int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, csum_net_id);
struct tcf_csum_params *params_old, *params_new;
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index b56986d41c87..912f3398f1c1 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -56,7 +56,7 @@ static const struct nla_policy gact_policy[TCA_GACT_MAX + 1] = {
static int tcf_gact_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, gact_net_id);
struct nlattr *tb[TCA_GACT_MAX + 1];
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 5954e992685a..e5127d400737 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -447,7 +447,7 @@ static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
static int tcf_ife_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, ife_net_id);
struct nlattr *tb[TCA_IFE_MAX + 1];
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 06e380ae0928..6894cfa83863 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -193,7 +193,7 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
static int tcf_ipt_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind)
+ int bind, struct netlink_ext_ack *extack)
{
return __tcf_ipt_init(net, ipt_net_id, nla, est, a, &act_ipt_ops, ovr,
bind);
@@ -201,7 +201,7 @@ static int tcf_ipt_init(struct net *net, struct nlattr *nla,
static int tcf_xt_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind)
+ int bind, struct netlink_ext_ack *extack)
{
return __tcf_ipt_init(net, xt_net_id, nla, est, a, &act_xt_ops, ovr,
bind);
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index abcd5f12b913..7ccd4c71179f 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -69,7 +69,7 @@ static struct tc_action_ops act_mirred_ops;
static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind)
+ int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
struct nlattr *tb[TCA_MIRRED_MAX + 1];
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 98c6a4b2f523..7e5ebd7f52a6 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -37,7 +37,8 @@ static const struct nla_policy nat_policy[TCA_NAT_MAX + 1] = {
};
static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
- struct tc_action **a, int ovr, int bind)
+ struct tc_action **a, int ovr, int bind,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, nat_net_id);
struct nlattr *tb[TCA_NAT_MAX + 1];
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 349beaffb29e..bb2c35ed6f10 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -132,7 +132,7 @@ static int tcf_pedit_key_ex_dump(struct sk_buff *skb,
static int tcf_pedit_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, pedit_net_id);
struct nlattr *tb[TCA_PEDIT_MAX + 1];
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 95d3c9097b25..6b6facbe251b 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -74,7 +74,8 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = {
static int tcf_act_police_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind,
+ struct netlink_ext_ack *extack)
{
int ret = 0, err;
struct nlattr *tb[TCA_POLICE_MAX + 1];
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 1ba0df238756..f4579ceba1f6 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -37,7 +37,7 @@ static const struct nla_policy sample_policy[TCA_SAMPLE_MAX + 1] = {
static int tcf_sample_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
- int bind)
+ int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, sample_net_id);
struct nlattr *tb[TCA_SAMPLE_MAX + 1];
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 425eac11f6da..b0346347c5f0 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -79,7 +79,7 @@ static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = {
static int tcf_simp_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, simp_net_id);
struct nlattr *tb[TCA_DEF_MAX + 1];
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 5a3f691bb545..7651c9d2182d 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -66,7 +66,7 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbedit_net_id);
struct nlattr *tb[TCA_SKBEDIT_MAX + 1];
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index fa975262dbac..1266449aa8ea 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -84,7 +84,7 @@ static const struct nla_policy skbmod_policy[TCA_SKBMOD_MAX + 1] = {
static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbmod_net_id);
struct nlattr *tb[TCA_SKBMOD_MAX + 1];
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 0e23aac09ad6..dde01eca7ed0 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -70,7 +70,7 @@ static const struct nla_policy tunnel_key_policy[TCA_TUNNEL_KEY_MAX + 1] = {
static int tunnel_key_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
struct nlattr *tb[TCA_TUNNEL_KEY_MAX + 1];
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index e1a1b3f3983a..6c387310b1b6 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -109,7 +109,7 @@ static const struct nla_policy vlan_policy[TCA_VLAN_MAX + 1] = {
static int tcf_vlan_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a,
- int ovr, int bind)
+ int ovr, int bind, struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
struct nlattr *tb[TCA_VLAN_MAX + 1];
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 5/8] net: sched: act: add extack for lookup callback
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
` (3 preceding siblings ...)
2018-02-14 22:13 ` [PATCHv2 net-next 4/8] net: sched: act: add extack to init callback Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 6/8] net: sched: act: add extack for walk callback Alexander Aring
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
This patch adds extack support for act lookup callback api. This
prepares to handle extack support inside each specific act
implementation.
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
include/net/act_api.h | 3 ++-
net/sched/act_api.c | 2 +-
net/sched/act_bpf.c | 3 ++-
net/sched/act_connmark.c | 3 ++-
net/sched/act_csum.c | 3 ++-
net/sched/act_gact.c | 3 ++-
net/sched/act_ife.c | 3 ++-
net/sched/act_ipt.c | 6 ++++--
net/sched/act_mirred.c | 3 ++-
net/sched/act_nat.c | 3 ++-
net/sched/act_pedit.c | 3 ++-
net/sched/act_police.c | 3 ++-
net/sched/act_sample.c | 3 ++-
net/sched/act_simple.c | 3 ++-
net/sched/act_skbedit.c | 3 ++-
net/sched/act_skbmod.c | 3 ++-
net/sched/act_tunnel_key.c | 3 ++-
net/sched/act_vlan.c | 3 ++-
18 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 3717e0f2bb1b..0bd65db506ba 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -87,7 +87,8 @@ struct tc_action_ops {
struct tcf_result *);
int (*dump)(struct sk_buff *, struct tc_action *, int, int);
void (*cleanup)(struct tc_action *);
- int (*lookup)(struct net *net, struct tc_action **a, u32 index);
+ int (*lookup)(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack);
int (*init)(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **act, int ovr,
int bind, struct netlink_ext_ack *extack);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 00c5a1d9a21e..6d2a035f1177 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -901,7 +901,7 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
goto err_out;
}
err = -ENOENT;
- if (ops->lookup(net, &a, index) == 0)
+ if (ops->lookup(net, &a, index, extack) == 0)
goto err_mod;
module_put(ops->owner);
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index b3ebfa9598e2..d9654b863347 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -374,7 +374,8 @@ static int tcf_bpf_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, bpf_net_id);
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 20e0215360b5..0504b7600fb6 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -184,7 +184,8 @@ static int tcf_connmark_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, connmark_net_id);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 3b8c48bb2683..bdd17b9ef034 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -638,7 +638,8 @@ static int tcf_csum_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_csum_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_csum_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, csum_net_id);
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 912f3398f1c1..e1e69e38f4b0 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -208,7 +208,8 @@ static int tcf_gact_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, gact_net_id);
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index e5127d400737..0b70fb0cc609 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -831,7 +831,8 @@ static int tcf_ife_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, ife_net_id);
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 6894cfa83863..f29af79a2d1f 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -310,7 +310,8 @@ static int tcf_ipt_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, ipt_net_id);
@@ -358,7 +359,8 @@ static int tcf_xt_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, xt_net_id);
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 7ccd4c71179f..9980c6affb5e 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -272,7 +272,8 @@ static int tcf_mirred_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_mirred_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_mirred_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 7e5ebd7f52a6..6f6d7667ef9a 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -285,7 +285,8 @@ static int tcf_nat_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, nat_net_id);
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index bb2c35ed6f10..308b2680a6d9 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -426,7 +426,8 @@ static int tcf_pedit_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, pedit_net_id);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 6b6facbe251b..1292f880eab0 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -305,7 +305,8 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
return -1;
}
-static int tcf_police_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_police_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, police_net_id);
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index f4579ceba1f6..22379b2cfd1a 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -209,7 +209,8 @@ static int tcf_sample_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, sample_net_id);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index b0346347c5f0..3ebf71470977 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -177,7 +177,8 @@ static int tcf_simp_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, simp_net_id);
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 7651c9d2182d..ff1970e14016 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -215,7 +215,8 @@ static int tcf_skbedit_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbedit_net_id);
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 1266449aa8ea..110d7c1f823d 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -239,7 +239,8 @@ static int tcf_skbmod_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbmod_net_id);
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index dde01eca7ed0..65a19928f1a9 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -298,7 +298,8 @@ static int tunnel_key_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index)
+static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 6c387310b1b6..03dcbbc5ffd2 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -274,7 +274,8 @@ static int tcf_vlan_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops);
}
-static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index)
+static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 6/8] net: sched: act: add extack for walk callback
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
` (4 preceding siblings ...)
2018-02-14 22:13 ` [PATCHv2 net-next 5/8] net: sched: act: add extack for lookup callback Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 7/8] net: sched: act: handle extack in tcf_generic_walker Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 8/8] net: sched: act: mirred: add extack support Alexander Aring
7 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
This patch adds extack support for act walker callback api. This
prepares to handle extack support inside each specific act
implementation.
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
include/net/act_api.h | 3 ++-
net/sched/act_api.c | 4 ++--
net/sched/act_bpf.c | 3 ++-
net/sched/act_connmark.c | 3 ++-
net/sched/act_csum.c | 3 ++-
net/sched/act_gact.c | 3 ++-
net/sched/act_ife.c | 3 ++-
net/sched/act_ipt.c | 6 ++++--
net/sched/act_mirred.c | 3 ++-
net/sched/act_nat.c | 3 ++-
net/sched/act_pedit.c | 3 ++-
net/sched/act_police.c | 3 ++-
net/sched/act_sample.c | 3 ++-
net/sched/act_simple.c | 3 ++-
net/sched/act_skbedit.c | 3 ++-
net/sched/act_skbmod.c | 3 ++-
net/sched/act_tunnel_key.c | 3 ++-
net/sched/act_vlan.c | 3 ++-
18 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 0bd65db506ba..ab3529255377 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -94,7 +94,8 @@ struct tc_action_ops {
int bind, struct netlink_ext_ack *extack);
int (*walk)(struct net *, struct sk_buff *,
struct netlink_callback *, int,
- const struct tc_action_ops *);
+ const struct tc_action_ops *,
+ struct netlink_ext_ack *);
void (*stats_update)(struct tc_action *, u64, u32, u64);
struct net_device *(*get_dev)(const struct tc_action *a);
};
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6d2a035f1177..cb284a0437eb 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -963,7 +963,7 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
goto out_module_put;
}
- err = ops->walk(net, skb, &dcb, RTM_DELACTION, ops);
+ err = ops->walk(net, skb, &dcb, RTM_DELACTION, ops, extack);
if (err <= 0)
goto out_module_put;
@@ -1253,7 +1253,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
if (nest == NULL)
goto out_module_put;
- ret = a_o->walk(net, skb, cb, RTM_GETACTION, a_o);
+ ret = a_o->walk(net, skb, cb, RTM_GETACTION, a_o, NULL);
if (ret < 0)
goto out_module_put;
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index d9654b863347..7e01e2c710c4 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -367,7 +367,8 @@ static void tcf_bpf_cleanup(struct tc_action *act)
static int tcf_bpf_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, bpf_net_id);
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 0504b7600fb6..cb722da0bb15 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -177,7 +177,8 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_connmark_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, connmark_net_id);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index bdd17b9ef034..3e8efadb750f 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -631,7 +631,8 @@ static void tcf_csum_cleanup(struct tc_action *a)
static int tcf_csum_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, csum_net_id);
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index e1e69e38f4b0..d96ebe4bb65a 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -201,7 +201,8 @@ static int tcf_gact_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_gact_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, gact_net_id);
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 0b70fb0cc609..b777e381e0dd 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -824,7 +824,8 @@ static int tcf_ife_act(struct sk_buff *skb, const struct tc_action *a,
static int tcf_ife_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, ife_net_id);
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index f29af79a2d1f..f33a8cc5dee6 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -303,7 +303,8 @@ static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
static int tcf_ipt_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, ipt_net_id);
@@ -352,7 +353,8 @@ static struct pernet_operations ipt_net_ops = {
static int tcf_xt_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, xt_net_id);
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 9980c6affb5e..3dcd295ea6a7 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -265,7 +265,8 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
static int tcf_mirred_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 6f6d7667ef9a..67243cdc0588 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -278,7 +278,8 @@ static int tcf_nat_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_nat_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, nat_net_id);
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 308b2680a6d9..6d6481f6bffa 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -419,7 +419,8 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_pedit_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, pedit_net_id);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 1292f880eab0..ff803414a736 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -58,7 +58,8 @@ static struct tc_action_ops act_police_ops;
static int tcf_act_police_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, police_net_id);
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 22379b2cfd1a..7a2b6a33f239 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -202,7 +202,8 @@ static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_sample_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, sample_net_id);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 3ebf71470977..3f5474d20702 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -170,7 +170,8 @@ static int tcf_simp_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_simp_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, simp_net_id);
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index ff1970e14016..d99b6f1f5181 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -208,7 +208,8 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_skbedit_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbedit_net_id);
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 110d7c1f823d..369ea85d0f02 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -232,7 +232,8 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_skbmod_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, skbmod_net_id);
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 65a19928f1a9..bced6fd00d43 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -291,7 +291,8 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
static int tunnel_key_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 03dcbbc5ffd2..7cf409443d02 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -267,7 +267,8 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
static int tcf_vlan_walker(struct net *net, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 7/8] net: sched: act: handle extack in tcf_generic_walker
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
` (5 preceding siblings ...)
2018-02-14 22:13 ` [PATCHv2 net-next 6/8] net: sched: act: add extack for walk callback Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 8/8] net: sched: act: mirred: add extack support Alexander Aring
7 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
This patch adds extack handling for a common used TC act function
"tcf_generic_walker()" to add an extack message on failures.
The tcf_generic_walker() function can fail if get a invalid command
different than DEL and GET. The naming "action" here is wrong, the
correct naming would be command.
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
include/net/act_api.h | 3 ++-
net/sched/act_api.c | 6 ++++--
net/sched/act_bpf.c | 2 +-
net/sched/act_connmark.c | 2 +-
net/sched/act_csum.c | 2 +-
net/sched/act_gact.c | 2 +-
net/sched/act_ife.c | 2 +-
net/sched/act_ipt.c | 4 ++--
net/sched/act_mirred.c | 2 +-
net/sched/act_nat.c | 2 +-
net/sched/act_pedit.c | 2 +-
net/sched/act_police.c | 2 +-
net/sched/act_sample.c | 2 +-
net/sched/act_simple.c | 2 +-
net/sched/act_skbedit.c | 2 +-
net/sched/act_skbmod.c | 2 +-
net/sched/act_tunnel_key.c | 2 +-
net/sched/act_vlan.c | 2 +-
18 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index ab3529255377..9c2f22695025 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -140,7 +140,8 @@ static inline void tc_action_net_exit(struct list_head *net_list,
int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops);
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack);
int tcf_idr_search(struct tc_action_net *tn, struct tc_action **a, u32 index);
bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
int bind);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index cb284a0437eb..08f326560d5e 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -202,7 +202,8 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb,
struct netlink_callback *cb, int type,
- const struct tc_action_ops *ops)
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
{
struct tcf_idrinfo *idrinfo = tn->idrinfo;
@@ -211,7 +212,8 @@ int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb,
} else if (type == RTM_GETACTION) {
return tcf_dump_walker(idrinfo, skb, cb);
} else {
- WARN(1, "tcf_generic_walker: unknown action %d\n", type);
+ WARN(1, "tcf_generic_walker: unknown command %d\n", type);
+ NL_SET_ERR_MSG(extack, "tcf_generic_walker: unknown command");
return -EINVAL;
}
}
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 7e01e2c710c4..cb3c5d403c88 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -372,7 +372,7 @@ static int tcf_bpf_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, bpf_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index cb722da0bb15..e4b880fa51fe 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -182,7 +182,7 @@ static int tcf_connmark_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, connmark_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 3e8efadb750f..d5c2e528d150 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -636,7 +636,7 @@ static int tcf_csum_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, csum_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_csum_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index d96ebe4bb65a..f072bcf33760 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -206,7 +206,7 @@ static int tcf_gact_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, gact_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index b777e381e0dd..a5994cf0512b 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -829,7 +829,7 @@ static int tcf_ife_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, ife_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index f33a8cc5dee6..9784629090ad 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -308,7 +308,7 @@ static int tcf_ipt_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, ipt_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index,
@@ -358,7 +358,7 @@ static int tcf_xt_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, xt_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 3dcd295ea6a7..05c2ebe92eca 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -270,7 +270,7 @@ static int tcf_mirred_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_mirred_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 67243cdc0588..4b5848b6c252 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -283,7 +283,7 @@ static int tcf_nat_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, nat_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 6d6481f6bffa..094303c27c5e 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -424,7 +424,7 @@ static int tcf_pedit_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, pedit_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index ff803414a736..ff55bd6c7db0 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -63,7 +63,7 @@ static int tcf_act_police_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, police_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = {
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 7a2b6a33f239..9765145aaf40 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -207,7 +207,7 @@ static int tcf_sample_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, sample_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 3f5474d20702..8244e221fe4f 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -175,7 +175,7 @@ static int tcf_simp_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, simp_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index d99b6f1f5181..ddf69fc01bdf 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -213,7 +213,7 @@ static int tcf_skbedit_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, skbedit_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 369ea85d0f02..a406f191cb84 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -237,7 +237,7 @@ static int tcf_skbmod_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, skbmod_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index bced6fd00d43..41ff9d0e5c62 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -296,7 +296,7 @@ static int tunnel_key_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index,
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 7cf409443d02..71411a255f04 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -272,7 +272,7 @@ static int tcf_vlan_walker(struct net *net, struct sk_buff *skb,
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
- return tcf_generic_walker(tn, skb, cb, type, ops);
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index,
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 net-next 8/8] net: sched: act: mirred: add extack support
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
` (6 preceding siblings ...)
2018-02-14 22:13 ` [PATCHv2 net-next 7/8] net: sched: act: handle extack in tcf_generic_walker Alexander Aring
@ 2018-02-14 22:13 ` Alexander Aring
7 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2018-02-14 22:13 UTC (permalink / raw)
To: davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, Alexander Aring, David Ahern
This patch adds extack support for TC mirred action.
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
net/sched/act_mirred.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 05c2ebe92eca..fd34015331ab 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -80,13 +80,17 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
bool exists = false;
int ret;
- if (!nla)
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Mirred requires attributes to be passed");
return -EINVAL;
- ret = nla_parse_nested(tb, TCA_MIRRED_MAX, nla, mirred_policy, NULL);
+ }
+ ret = nla_parse_nested(tb, TCA_MIRRED_MAX, nla, mirred_policy, extack);
if (ret < 0)
return ret;
- if (!tb[TCA_MIRRED_PARMS])
+ if (!tb[TCA_MIRRED_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required mirred parameters");
return -EINVAL;
+ }
parm = nla_data(tb[TCA_MIRRED_PARMS]);
exists = tcf_idr_check(tn, parm->index, a, bind);
@@ -102,6 +106,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
default:
if (exists)
tcf_idr_release(*a, bind);
+ NL_SET_ERR_MSG_MOD(extack, "Unknown mirred option");
return -EINVAL;
}
if (parm->ifindex) {
@@ -117,8 +122,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
}
if (!exists) {
- if (!dev)
+ if (!dev) {
+ NL_SET_ERR_MSG_MOD(extack, "Specified device does not exist");
return -EINVAL;
+ }
ret = tcf_idr_create(tn, parm->index, est, a,
&act_mirred_ops, bind, true);
if (ret)
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors
2018-02-14 22:13 ` [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors Alexander Aring
@ 2018-02-15 11:14 ` Davide Caratti
2018-02-15 15:43 ` Alexander Aring
0 siblings, 1 reply; 12+ messages in thread
From: Davide Caratti @ 2018-02-15 11:14 UTC (permalink / raw)
To: Alexander Aring, davem
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel, David Ahern
On Wed, 2018-02-14 at 17:13 -0500, Alexander Aring wrote:
> This patch adds extack support for generic act handling. The extack
> will be set deeper to each called function which is not part of netdev
> core api.
>
> Based on work by David Ahern <dsahern@gmail.com>
hello Alexander,
after looking at the code more closely, I think there is margin for some
more improvement here (i.e make the message contents easier to grep from a
log). Please let me know if the comments below make sense for you.
thank you in advance!
--
davide
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
> net/sched/act_api.c | 93 +++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 61 insertions(+), 32 deletions(-)
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 8d89b026414f..a5138ae026a1 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -617,31 +617,40 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> int err;
>
> if (name == NULL) {
> - err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, NULL);
> + err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, extack);
> if (err < 0)
> goto err_out;
> err = -EINVAL;
> kind = tb[TCA_ACT_KIND];
> - if (!kind)
> + if (!kind) {
> + NL_SET_ERR_MSG(extack, "TC action kind must be specified");
> goto err_out;
> - if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
> + }
> + if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ) {
> + NL_SET_ERR_MSG(extack, "TC action name too long");
> goto err_out;
> + }
> if (tb[TCA_ACT_COOKIE]) {
> int cklen = nla_len(tb[TCA_ACT_COOKIE]);
>
> - if (cklen > TC_COOKIE_MAX_SIZE)
> + if (cklen > TC_COOKIE_MAX_SIZE) {
> + NL_SET_ERR_MSG(extack, "TC cookie size above the maximum");
> goto err_out;
> + }
>
> cookie = nla_memdup_cookie(tb);
> if (!cookie) {
> + NL_SET_ERR_MSG(extack, "No memory to generate TC cookie");
> err = -ENOMEM;
> goto err_out;
> }
> }
> } else {
> - err = -EINVAL;
> - if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ)
> + if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ) {
> + NL_SET_ERR_MSG(extack, "TC action name too long");
> + err = -EINVAL;
> goto err_out;
> + }
> }
>
> a_o = tc_lookup_action_n(act_name);
> @@ -664,6 +673,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> goto err_mod;
> }
> #endif
> + NL_SET_ERR_MSG(extack, "Failed to load TC action module");
> err = -ENOENT;
> goto err_out;
> }
> @@ -698,6 +708,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
>
> list_add_tail(&a->list, &actions);
> tcf_action_destroy(&actions, bind);
> + NL_SET_ERR_MSG(extack, "Failed to init action chain");
most of the times, the word 'action' is prepended by the word 'TC'.
Proposal:
"Failed to init TC action chain"
> return ERR_PTR(err);
> }
> }
> @@ -734,7 +745,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
> int err;
> int i;
>
> - err = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, NULL);
> + err = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
> if (err < 0)
> return err;
>
> @@ -842,7 +853,8 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
>
> static int
> tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
> - struct list_head *actions, int event)
> + struct list_head *actions, int event,
> + struct netlink_ext_ack *extack)
> {
> struct sk_buff *skb;
>
> @@ -851,6 +863,7 @@ tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
> return -ENOBUFS;
> if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
> 0, 0) <= 0) {
> + NL_SET_ERR_MSG(extack, "Failed to fill netlink tc action attributes");
see also @@ -975,6 +1000,7 @@ and @@ -975,6 +1000,7 @@:
this is the same message you get in case tcf_add_notify() fails, and it's
very similar to what you get when tcf_del_notify() fails: the only
difference is uppercase/lowercase 'tc' word.
Proposal:
"Failed to fill netlink attributes while getting TC action"
> kfree_skb(skb);
> return -EINVAL;
> }
> @@ -859,7 +872,8 @@ tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
> }
>
> static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
> - struct nlmsghdr *n, u32 portid)
> + struct nlmsghdr *n, u32 portid,
> + struct netlink_ext_ack *extack)
> {
> struct nlattr *tb[TCA_ACT_MAX + 1];
> const struct tc_action_ops *ops;
> @@ -867,20 +881,24 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
> int index;
> int err;
>
> - err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, NULL);
> + err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, extack);
> if (err < 0)
> goto err_out;
>
> err = -EINVAL;
> if (tb[TCA_ACT_INDEX] == NULL ||
> - nla_len(tb[TCA_ACT_INDEX]) < sizeof(index))
> + nla_len(tb[TCA_ACT_INDEX]) < sizeof(index)) {
> + NL_SET_ERR_MSG(extack, "Invalid TC action index value");
> goto err_out;
> + }
> index = nla_get_u32(tb[TCA_ACT_INDEX]);
>
> err = -EINVAL;
> ops = tc_lookup_action(tb[TCA_ACT_KIND]);
> - if (!ops) /* could happen in batch of actions */
> + if (!ops) { /* could happen in batch of actions */
> + NL_SET_ERR_MSG(extack, "Specified TC action not found");
> goto err_out;
> + }
> err = -ENOENT;
> if (ops->lookup(net, &a, index) == 0)
> goto err_mod;
> @@ -895,7 +913,8 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
> }
>
> static int tca_action_flush(struct net *net, struct nlattr *nla,
> - struct nlmsghdr *n, u32 portid)
> + struct nlmsghdr *n, u32 portid,
> + struct netlink_ext_ack *extack)
> {
> struct sk_buff *skb;
> unsigned char *b;
> @@ -909,35 +928,39 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
> int err = -ENOMEM;
>
> skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> - if (!skb) {
> - pr_debug("tca_action_flush: failed skb alloc\n");
> + if (!skb)
> return err;
> - }
>
> b = skb_tail_pointer(skb);
>
> - err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, NULL);
> + err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL, extack);
> if (err < 0)
> goto err_out;
>
> err = -EINVAL;
> kind = tb[TCA_ACT_KIND];
> ops = tc_lookup_action(kind);
> - if (!ops) /*some idjot trying to flush unknown action */
> + if (!ops) { /*some idjot trying to flush unknown action */
the above comment was already there before your patch. I think that it can
be removed, now that we have an error message for this situation; it's
also funny (and harmless) to leave it - but then maybe it becomes more
clear if the word 'idiot' is spelled correctly :)
> + NL_SET_ERR_MSG(extack, "Aborted Flush. Cannot flush unknown TC action");
Also the tests below result in an aborted flush when they fail. So,
'Aborted Flush' can probably be omitted. Proposal:
"Cannot flush unknown TC action"
> goto err_out;
> + }
>
> nlh = nlmsg_put(skb, portid, n->nlmsg_seq, RTM_DELACTION,
> sizeof(*t), 0);
> - if (!nlh)
> + if (!nlh) {
> + NL_SET_ERR_MSG(extack, "Aborted Flush. Failed to create flush netlink notification");
Proposal:
"Failed to create netlink notification while flushing TC action". But I'm
not sure that these kind of errors deserve an extended ACK (see below).
> goto out_module_put;
> + }
> t = nlmsg_data(nlh);
> t->tca_family = AF_UNSPEC;
> t->tca__pad1 = 0;
> t->tca__pad2 = 0;
>
> nest = nla_nest_start(skb, TCA_ACT_TAB);
> - if (!nest)
> + if (!nest) {
> + NL_SET_ERR_MSG(extack, "Failed to add new netlink message");
"Failed to set nested attribute while flushing TC action". But I'm not
sure that this kind of errors deserve an extended ACK. Probaly a good
compromise is to do a single message in the error path of the function
(i.e. below the "out_module_put" label).
> goto out_module_put;
> + }
>
> err = ops->walk(net, skb, &dcb, RTM_DELACTION, ops);
> if (err <= 0)
here the action flush is aborted because walk() failed. Is it worth adding
a message here? (I'm also ok if there is a single error below
"out_module_put" label).
(and, for the record, I think we miss a nla_nest_cancel() here. I will
post a patch targeting 'net' today.)
> @@ -952,6 +975,8 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
> n->nlmsg_flags & NLM_F_ECHO);
> if (err > 0)
> return 0;
> + if (err < 0)
> + NL_SET_ERR_MSG(extack, "Failed to send flush notification");
Proposal:
"Failed to send TC action flush notification"
>
> return err;
>
> @@ -964,7 +989,7 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
>
> static int
> tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
> - u32 portid)
> + u32 portid, struct netlink_ext_ack *extack)
> {
> int ret;
> struct sk_buff *skb;
> @@ -975,6 +1000,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
>
> if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
> 0, 1) <= 0) {
> + NL_SET_ERR_MSG(extack, "Failed to fill netlink tc action attributes");
see also @@ -1039,7 +1067,7 @@:
this error message is almost the same as the one we get in case of failing
tca_get_fill(..., RTM_NEWACTION, ...) in tcf_add_notify(). The only
difference is the word 'tc' lowercase here, and uppercase word 'TC' in
tcf_add_notify(). So, if these message must be different, it's better to
specify better what went wrong _ something like:
"Failed to fill netlink attributes while deleting TC action"
> kfree_skb(skb);
> return -EINVAL;
> }
> @@ -982,6 +1008,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
> /* now do the delete */
> ret = tcf_action_destroy(actions, 0);
> if (ret < 0) {
> + NL_SET_ERR_MSG(extack, "Failed to delete TC action");
> kfree_skb(skb);
> return ret;
> }
> @@ -995,26 +1022,27 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
>
> static int
> tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
> - u32 portid, int event)
> + u32 portid, int event, struct netlink_ext_ack *extack)
> {
> int i, ret;
> struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
> struct tc_action *act;
> LIST_HEAD(actions);
>
> - ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, NULL);
> + ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
> if (ret < 0)
> return ret;
>
> if (event == RTM_DELACTION && n->nlmsg_flags & NLM_F_ROOT) {
> if (tb[1])
> - return tca_action_flush(net, tb[1], n, portid);
> + return tca_action_flush(net, tb[1], n, portid, extack);
>
> + NL_SET_ERR_MSG(extack, "Invalid netlink attributes to flush actions");
"Invalid netlink attributes while flushing TC action"
> return -EINVAL;
> }
>
> for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {
> - act = tcf_action_get_1(net, tb[i], n, portid);
> + act = tcf_action_get_1(net, tb[i], n, portid, extack);
> if (IS_ERR(act)) {
> ret = PTR_ERR(act);
> goto err;
> @@ -1024,9 +1052,9 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
> }
>
> if (event == RTM_GETACTION)
> - ret = tcf_get_notify(net, portid, n, &actions, event);
> + ret = tcf_get_notify(net, portid, n, &actions, event, extack);
> else { /* delete */
> - ret = tcf_del_notify(net, n, &actions, portid);
> + ret = tcf_del_notify(net, n, &actions, portid, extack);
> if (ret)
> goto err;
> return ret;
> @@ -1039,7 +1067,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
>
> static int
> tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
> - u32 portid)
> + u32 portid, struct netlink_ext_ack *extack)
> {
> struct sk_buff *skb;
> int err = 0;
> @@ -1050,6 +1078,7 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
>
> if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
> RTM_NEWACTION, 0, 0) <= 0) {
> + NL_SET_ERR_MSG(extack, "Failed to fill netlink tc action attributes");
see comment at @@ -975,6 +1000,7 @@
"Failed to fill netlink attributes while adding TC action"
> kfree_skb(skb);
> return -EINVAL;
> }
> @@ -1073,7 +1102,7 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
> if (ret)
> return ret;
>
> - return tcf_add_notify(net, n, &actions, portid);
> + return tcf_add_notify(net, n, &actions, portid, extack);
> }
>
> static u32 tcaa_root_flags_allowed = TCA_FLAG_LARGE_DUMP_ON;
> @@ -1101,7 +1130,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
> return ret;
>
> if (tca[TCA_ACT_TAB] == NULL) {
> - pr_notice("tc_ctl_action: received NO action attribs\n");
> + NL_SET_ERR_MSG(extack, "Netlink Action attributes missing");
"action" is lowercase in almost all messages, and tipically prepended by
'TC' keyword:
"Missing netlink attributes for TC action"
> return -EINVAL;
> }
>
> @@ -1124,11 +1153,11 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
> break;
> case RTM_DELACTION:
> ret = tca_action_gd(net, tca[TCA_ACT_TAB], n,
> - portid, RTM_DELACTION);
> + portid, RTM_DELACTION, extack);
> break;
> case RTM_GETACTION:
> ret = tca_action_gd(net, tca[TCA_ACT_TAB], n,
> - portid, RTM_GETACTION);
> + portid, RTM_GETACTION, extack);
> break;
> default:
> BUG();
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors
2018-02-15 11:14 ` Davide Caratti
@ 2018-02-15 15:43 ` Alexander Aring
2018-02-15 15:45 ` Davide Caratti
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Aring @ 2018-02-15 15:43 UTC (permalink / raw)
To: Davide Caratti
Cc: David Miller, Jamal Hadi Salim, Cong Wang,
Jiří Pírko, netdev, kernel, David Ahern
Hi,
On Thu, Feb 15, 2018 at 6:14 AM, Davide Caratti <dcaratti@redhat.com> wrote:
> On Wed, 2018-02-14 at 17:13 -0500, Alexander Aring wrote:
>> This patch adds extack support for generic act handling. The extack
>> will be set deeper to each called function which is not part of netdev
>> core api.
>>
>> Based on work by David Ahern <dsahern@gmail.com>
>
> hello Alexander,
>
> after looking at the code more closely, I think there is margin for some
> more improvement here (i.e make the message contents easier to grep from a
> log). Please let me know if the comments below make sense for you.
>
I will send v3 with your changes. But not with one error handling in
label, the most whole point is to get some messages when somebody
throw a -EINVAL to the userspace and TC subsystem has a lot of places
with that.
- Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors
2018-02-15 15:43 ` Alexander Aring
@ 2018-02-15 15:45 ` Davide Caratti
0 siblings, 0 replies; 12+ messages in thread
From: Davide Caratti @ 2018-02-15 15:45 UTC (permalink / raw)
To: Alexander Aring
Cc: David Miller, Jamal Hadi Salim, Cong Wang,
Jiří Pírko, netdev, kernel, David Ahern
On Thu, 2018-02-15 at 10:43 -0500, Alexander Aring wrote:
> I will send v3 with your changes. But not with one error handling in
> label, the most whole point is to get some messages when somebody
> throw a -EINVAL to the userspace and TC subsystem has a lot of places
> with that.
that's ok for me.
thanks!
--
davide
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-02-15 15:45 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-14 22:13 [PATCHv2 net-next 0/8] net: sched: act: add extack support Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 1/8] net: sched: act: fix code style Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 2/8] net: sched: act: add extack to init Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 3/8] net: sched: act: handle generic action errors Alexander Aring
2018-02-15 11:14 ` Davide Caratti
2018-02-15 15:43 ` Alexander Aring
2018-02-15 15:45 ` Davide Caratti
2018-02-14 22:13 ` [PATCHv2 net-next 4/8] net: sched: act: add extack to init callback Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 5/8] net: sched: act: add extack for lookup callback Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 6/8] net: sched: act: add extack for walk callback Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 7/8] net: sched: act: handle extack in tcf_generic_walker Alexander Aring
2018-02-14 22:13 ` [PATCHv2 net-next 8/8] net: sched: act: mirred: add extack support Alexander Aring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.