Skip to content

Commit

Permalink
sched: add new attr TCA_EXT_WARN_MSG to report tc extact message
Browse files Browse the repository at this point in the history 
We will report extack message if there is an error via netlink_ack(). But
if the rule is not to be exclusively executed by the hardware, extack is not
passed along and offloading failures don't get logged.

In commit 81c7288 ("sched: cls: enable verbose logging") Marcelo
made cls could log verbose info for offloading failures, which helps
improving Open vSwitch debuggability when using flower offloading.

It would also be helpful if userspace monitor tools, like "tc monitor",
could log this kind of message, as it doesn't require vswitchd log level
adjusment. Let's add a new tc attributes to report the extack message so
the monitor program could receive the failures. e.g.

  # tc monitor
  added chain dev enp3s0f1np1 parent ffff: chain 0
  added filter dev enp3s0f1np1 ingress protocol all pref 49152 flower chain 0 handle 0x1
    ct_state +trk+new
    not_in_hw
          action order 1: gact action drop
           random type none pass val 0
           index 1 ref 1 bind 1

  Warning: mlx5_core: matching on ct_state +new isn't supported.

In this patch I only report the extack message on add/del operations.
It doesn't look like we need to report the extack message on get/dump
operations.

Note this message not only reporte to multicast groups, it could also
be reported unicast, which may affect the current usersapce tool's behaivor.

Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Link: https://lore.kernel.org/r/20230113034353.2766735-1-liuhangbin@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
Hangbin Liu authored and Paolo Abeni committed Jan 17, 2023
1 parent 86ce04f commit 0349b87
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 49 deletions.
1 change: 1 addition & 0 deletions include/uapi/linux/rtnetlink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,7 @@ enum {
TCA_INGRESS_BLOCK,
TCA_EGRESS_BLOCK,
TCA_DUMP_FLAGS,
TCA_EXT_WARN_MSG,
__TCA_MAX
};

Expand Down
15 changes: 10 additions & 5 deletions net/sched/act_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1582,7 +1582,7 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *p,

static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
u32 portid, u32 seq, u16 flags, int event, int bind,
int ref)
int ref, struct netlink_ext_ack *extack)
{
struct tcamsg *t;
struct nlmsghdr *nlh;
Expand All @@ -1606,7 +1606,12 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],

nla_nest_end(skb, nest);

if (extack && extack->_msg &&
nla_put_string(skb, TCA_EXT_WARN_MSG, extack->_msg))
goto out_nlmsg_trim;

nlh->nlmsg_len = skb_tail_pointer(skb) - b;

return skb->len;

out_nlmsg_trim:
Expand All @@ -1625,7 +1630,7 @@ tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
if (!skb)
return -ENOBUFS;
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
0, 1) <= 0) {
0, 1, NULL) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
kfree_skb(skb);
return -EINVAL;
Expand Down Expand Up @@ -1799,7 +1804,7 @@ tcf_reoffload_del_notify(struct net *net, struct tc_action *action)
if (!skb)
return -ENOBUFS;

if (tca_get_fill(skb, actions, 0, 0, 0, RTM_DELACTION, 0, 1) <= 0) {
if (tca_get_fill(skb, actions, 0, 0, 0, RTM_DELACTION, 0, 1, NULL) <= 0) {
kfree_skb(skb);
return -EINVAL;
}
Expand Down Expand Up @@ -1886,7 +1891,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
return -ENOBUFS;

if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
0, 2) <= 0) {
0, 2, extack) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action attributes");
kfree_skb(skb);
return -EINVAL;
Expand Down Expand Up @@ -1965,7 +1970,7 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
return -ENOBUFS;

if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
RTM_NEWACTION, 0, 0) <= 0) {
RTM_NEWACTION, 0, 0, extack) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
kfree_skb(skb);
return -EINVAL;
Expand Down
62 changes: 39 additions & 23 deletions net/sched/cls_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,8 @@ static struct tcf_chain *tcf_chain_lookup_rcu(const struct tcf_block *block,
#endif

static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,
u32 seq, u16 flags, int event, bool unicast);
u32 seq, u16 flags, int event, bool unicast,
struct netlink_ext_ack *extack);

static struct tcf_chain *__tcf_chain_get(struct tcf_block *block,
u32 chain_index, bool create,
Expand Down Expand Up @@ -521,7 +522,7 @@ static struct tcf_chain *__tcf_chain_get(struct tcf_block *block,
*/
if (is_first_reference && !by_act)
tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
RTM_NEWCHAIN, false);
RTM_NEWCHAIN, false, NULL);

return chain;

Expand Down Expand Up @@ -1817,7 +1818,8 @@ static int tcf_fill_node(struct net *net, struct sk_buff *skb,
struct tcf_proto *tp, struct tcf_block *block,
struct Qdisc *q, u32 parent, void *fh,
u32 portid, u32 seq, u16 flags, int event,
bool terse_dump, bool rtnl_held)
bool terse_dump, bool rtnl_held,
struct netlink_ext_ack *extack)
{
struct tcmsg *tcm;
struct nlmsghdr *nlh;
Expand Down Expand Up @@ -1857,7 +1859,13 @@ static int tcf_fill_node(struct net *net, struct sk_buff *skb,
tp->ops->dump(net, tp, fh, skb, tcm, rtnl_held) < 0)
goto nla_put_failure;
}

if (extack && extack->_msg &&
nla_put_string(skb, TCA_EXT_WARN_MSG, extack->_msg))
goto nla_put_failure;

nlh->nlmsg_len = skb_tail_pointer(skb) - b;

return skb->len;

out_nlmsg_trim:
Expand All @@ -1871,7 +1879,7 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
struct nlmsghdr *n, struct tcf_proto *tp,
struct tcf_block *block, struct Qdisc *q,
u32 parent, void *fh, int event, bool unicast,
bool rtnl_held)
bool rtnl_held, struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
Expand All @@ -1883,7 +1891,7 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,

if (tcf_fill_node(net, skb, tp, block, q, parent, fh, portid,
n->nlmsg_seq, n->nlmsg_flags, event,
false, rtnl_held) <= 0) {
false, rtnl_held, extack) <= 0) {
kfree_skb(skb);
return -EINVAL;
}
Expand Down Expand Up @@ -1912,7 +1920,7 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,

if (tcf_fill_node(net, skb, tp, block, q, parent, fh, portid,
n->nlmsg_seq, n->nlmsg_flags, RTM_DELTFILTER,
false, rtnl_held) <= 0) {
false, rtnl_held, extack) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to build del event notification");
kfree_skb(skb);
return -EINVAL;
Expand All @@ -1938,14 +1946,15 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
struct tcf_block *block, struct Qdisc *q,
u32 parent, struct nlmsghdr *n,
struct tcf_chain *chain, int event)
struct tcf_chain *chain, int event,
struct netlink_ext_ack *extack)
{
struct tcf_proto *tp;

for (tp = tcf_get_next_proto(chain, NULL);
tp; tp = tcf_get_next_proto(chain, tp))
tfilter_notify(net, oskb, n, tp, block,
q, parent, NULL, event, false, true);
tfilter_notify(net, oskb, n, tp, block, q, parent, NULL,
event, false, true, extack);
}

static void tfilter_put(struct tcf_proto *tp, void *fh)
Expand Down Expand Up @@ -2156,7 +2165,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
flags, extack);
if (err == 0) {
tfilter_notify(net, skb, n, tp, block, q, parent, fh,
RTM_NEWTFILTER, false, rtnl_held);
RTM_NEWTFILTER, false, rtnl_held, extack);
tfilter_put(tp, fh);
/* q pointer is NULL for shared blocks */
if (q)
Expand Down Expand Up @@ -2284,7 +2293,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,

if (prio == 0) {
tfilter_notify_chain(net, skb, block, q, parent, n,
chain, RTM_DELTFILTER);
chain, RTM_DELTFILTER, extack);
tcf_chain_flush(chain, rtnl_held);
err = 0;
goto errout;
Expand All @@ -2308,7 +2317,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,

tcf_proto_put(tp, rtnl_held, NULL);
tfilter_notify(net, skb, n, tp, block, q, parent, fh,
RTM_DELTFILTER, false, rtnl_held);
RTM_DELTFILTER, false, rtnl_held, extack);
err = 0;
goto errout;
}
Expand Down Expand Up @@ -2452,7 +2461,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
err = -ENOENT;
} else {
err = tfilter_notify(net, skb, n, tp, block, q, parent,
fh, RTM_NEWTFILTER, true, rtnl_held);
fh, RTM_NEWTFILTER, true, rtnl_held, NULL);
if (err < 0)
NL_SET_ERR_MSG(extack, "Failed to send filter notify message");
}
Expand Down Expand Up @@ -2490,7 +2499,7 @@ static int tcf_node_dump(struct tcf_proto *tp, void *n, struct tcf_walker *arg)
return tcf_fill_node(net, a->skb, tp, a->block, a->q, a->parent,
n, NETLINK_CB(a->cb->skb).portid,
a->cb->nlh->nlmsg_seq, NLM_F_MULTI,
RTM_NEWTFILTER, a->terse_dump, true);
RTM_NEWTFILTER, a->terse_dump, true, NULL);
}

static bool tcf_chain_dump(struct tcf_chain *chain, struct Qdisc *q, u32 parent,
Expand Down Expand Up @@ -2524,7 +2533,7 @@ static bool tcf_chain_dump(struct tcf_chain *chain, struct Qdisc *q, u32 parent,
if (tcf_fill_node(net, skb, tp, block, q, parent, NULL,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
RTM_NEWTFILTER, false, true) <= 0)
RTM_NEWTFILTER, false, true, NULL) <= 0)
goto errout;
cb->args[1] = 1;
}
Expand Down Expand Up @@ -2667,7 +2676,8 @@ static int tc_chain_fill_node(const struct tcf_proto_ops *tmplt_ops,
void *tmplt_priv, u32 chain_index,
struct net *net, struct sk_buff *skb,
struct tcf_block *block,
u32 portid, u32 seq, u16 flags, int event)
u32 portid, u32 seq, u16 flags, int event,
struct netlink_ext_ack *extack)
{
unsigned char *b = skb_tail_pointer(skb);
const struct tcf_proto_ops *ops;
Expand Down Expand Up @@ -2704,7 +2714,12 @@ static int tc_chain_fill_node(const struct tcf_proto_ops *tmplt_ops,
goto nla_put_failure;
}

if (extack && extack->_msg &&
nla_put_string(skb, TCA_EXT_WARN_MSG, extack->_msg))
goto out_nlmsg_trim;

nlh->nlmsg_len = skb_tail_pointer(skb) - b;

return skb->len;

out_nlmsg_trim:
Expand All @@ -2714,7 +2729,8 @@ static int tc_chain_fill_node(const struct tcf_proto_ops *tmplt_ops,
}

static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,
u32 seq, u16 flags, int event, bool unicast)
u32 seq, u16 flags, int event, bool unicast,
struct netlink_ext_ack *extack)
{
u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
struct tcf_block *block = chain->block;
Expand All @@ -2728,7 +2744,7 @@ static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,

if (tc_chain_fill_node(chain->tmplt_ops, chain->tmplt_priv,
chain->index, net, skb, block, portid,
seq, flags, event) <= 0) {
seq, flags, event, extack) <= 0) {
kfree_skb(skb);
return -EINVAL;
}
Expand Down Expand Up @@ -2756,7 +2772,7 @@ static int tc_chain_notify_delete(const struct tcf_proto_ops *tmplt_ops,
return -ENOBUFS;

if (tc_chain_fill_node(tmplt_ops, tmplt_priv, chain_index, net, skb,
block, portid, seq, flags, RTM_DELCHAIN) <= 0) {
block, portid, seq, flags, RTM_DELCHAIN, NULL) <= 0) {
kfree_skb(skb);
return -EINVAL;
}
Expand Down Expand Up @@ -2908,11 +2924,11 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
}

tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
RTM_NEWCHAIN, false);
RTM_NEWCHAIN, false, extack);
break;
case RTM_DELCHAIN:
tfilter_notify_chain(net, skb, block, q, parent, n,
chain, RTM_DELTFILTER);
chain, RTM_DELTFILTER, extack);
/* Flush the chain first as the user requested chain removal. */
tcf_chain_flush(chain, true);
/* In case the chain was successfully deleted, put a reference
Expand All @@ -2922,7 +2938,7 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
break;
case RTM_GETCHAIN:
err = tc_chain_notify(chain, skb, n->nlmsg_seq,
n->nlmsg_flags, n->nlmsg_type, true);
n->nlmsg_flags, n->nlmsg_type, true, extack);
if (err < 0)
NL_SET_ERR_MSG(extack, "Failed to send chain notify message");
break;
Expand Down Expand Up @@ -3022,7 +3038,7 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
chain->index, net, skb, block,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
RTM_NEWCHAIN);
RTM_NEWCHAIN, NULL);
if (err <= 0)
break;
index++;
Expand Down
Loading

0 comments on commit 0349b87

Please sign in to comment.