From: Stephen Hemminger <stephen@networkplumber.org>
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: netdev@vger.kernel.org, Alexander Aring <aring@mojatatu.com>,
Jiri Pirko <jiri@resnulli.us>, Jakub Kicinski <kubakici@wp.pl>
Subject: Re: [PATCH RFC iproute2] libnetlink: allow reading more than one message from extack
Date: Mon, 19 Mar 2018 09:09:50 -0700 [thread overview]
Message-ID: <20180319090950.3241e397@xeon-e3> (raw)
In-Reply-To: <0e68f6e4a4fc68fd3f5f164b8d50a95fde6a1f50.1521227930.git.mleitner@redhat.com>
On Fri, 16 Mar 2018 16:23:09 -0300
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote:
> This patch introduces support for reading more than one message from
> extack's and to adjust their level (warning/error) accordingly.
>
> Yes, there is a FIXME tag in the callback call for now.
>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Make sense, can hold off until kernel supports warnings.
> ---
> lib/libnetlink.c | 55 ++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 40 insertions(+), 15 deletions(-)
>
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index 928de1dd16d84b7802c06d0a659f5d73e1bbcb4b..7c4c81e11e02ea857888190eb5e7a9e99d159bb3 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -43,9 +43,16 @@ static const enum mnl_attr_data_type extack_policy[NLMSGERR_ATTR_MAX + 1] = {
> [NLMSGERR_ATTR_OFFS] = MNL_TYPE_U32,
> };
>
> +#define NETLINK_MAX_EXTACK_MSGS 8
Would rather not have fixed maximums
> +struct extack_args {
> + const struct nlattr *msg[NETLINK_MAX_EXTACK_MSGS];
> + const struct nlattr *offs;
> + int msg_count;
> +};
If you put msg[] last in structure, it could be variable length.
> static int err_attr_cb(const struct nlattr *attr, void *data)
> {
> - const struct nlattr **tb = data;
> + struct extack_args *tb = data;
> uint16_t type;
>
> if (mnl_attr_type_valid(attr, NLMSGERR_ATTR_MAX) < 0) {
> @@ -60,19 +67,23 @@ static int err_attr_cb(const struct nlattr *attr, void *data)
> return MNL_CB_ERROR;
> }
>
> - tb[type] = attr;
> + if (type == NLMSGERR_ATTR_OFFS)
> + tb->offs = attr;
> + else if (tb->msg_count < NETLINK_MAX_EXTACK_MSGS)
> + tb->msg[tb->msg_count++] = attr;
> return MNL_CB_OK;
> }
>
> /* dump netlink extended ack error message */
> int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
> {
> - struct nlattr *tb[NLMSGERR_ATTR_MAX + 1] = {};
> + struct extack_args tb = {};
> const struct nlmsgerr *err = mnl_nlmsg_get_payload(nlh);
> const struct nlmsghdr *err_nlh = NULL;
> unsigned int hlen = sizeof(*err);
> - const char *msg = NULL;
> + const char *msg[NETLINK_MAX_EXTACK_MSGS] = {};
> uint32_t off = 0;
> + int ret, i;
>
> /* no TLVs, nothing to do here */
> if (!(nlh->nlmsg_flags & NLM_F_ACK_TLVS))
> @@ -82,14 +93,14 @@ int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
> if (!(nlh->nlmsg_flags & NLM_F_CAPPED))
> hlen += mnl_nlmsg_get_payload_len(&err->msg);
>
> - if (mnl_attr_parse(nlh, hlen, err_attr_cb, tb) != MNL_CB_OK)
> + if (mnl_attr_parse(nlh, hlen, err_attr_cb, &tb) != MNL_CB_OK)
> return 0;
>
> - if (tb[NLMSGERR_ATTR_MSG])
> - msg = mnl_attr_get_str(tb[NLMSGERR_ATTR_MSG]);
> + for (i = 0; i < NETLINK_MAX_EXTACK_MSGS && tb.msg[i]; i++)
> + msg[i] = mnl_attr_get_str(tb.msg[i]);
>
> - if (tb[NLMSGERR_ATTR_OFFS]) {
> - off = mnl_attr_get_u32(tb[NLMSGERR_ATTR_OFFS]);
> + if (tb.offs) {
> + off = mnl_attr_get_u32(tb.offs);
>
> if (off > nlh->nlmsg_len) {
> fprintf(stderr,
> @@ -100,21 +111,35 @@ int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
> }
>
> if (errfn)
> - return errfn(msg, off, err_nlh);
> + return errfn(*msg, off, err_nlh); /* FIXME */
>
> - if (msg && *msg != '\0') {
> + ret = 0;
> + for (i = 0; i < NETLINK_MAX_EXTACK_MSGS && msg[i]; i++) {
> bool is_err = !!err->error;
> + const char *_msg = msg[i];
> +
> + /* Message tagging has precedence.
> + * KERN_WARNING = ASCII Start Of Header ('\001') + '4'
> + */
> + if (!strncmp(_msg, "\0014", 2)) {
> + is_err = false;
> + _msg += 2;
> + }
If you are going to have an API that has levels, it must be the same
as existing syslog kernel log format and maybe even get some code reuse.
> + /* But we can't have Error if it didn't fail. */
> + if (is_err && !err->error)
> + is_err = 0;
>
> fprintf(stderr, "%s: %s",
> - is_err ? "Error" : "Warning", msg);
> - if (msg[strlen(msg) - 1] != '.')
> + is_err ? "Error" : "Warning", _msg);
> + if (_msg[strlen(_msg) - 1] != '.')
> fprintf(stderr, ".");
> fprintf(stderr, "\n");
>
> - return is_err ? 1 : 0;
> + if (is_err)
> + ret = 1;
> }
>
> - return 0;
> + return ret;
> }
> #else
> #warning "libmnl required for error support"
next prev parent reply other threads:[~2018-03-19 16:09 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-16 19:23 [PATCH RFC 0/2] Add support for warnings to extack Marcelo Ricardo Leitner
2018-03-16 19:23 ` [PATCH RFC iproute2] libnetlink: allow reading more than one message from extack Marcelo Ricardo Leitner
2018-03-19 16:09 ` Stephen Hemminger [this message]
2018-03-19 17:14 ` Marcelo Ricardo Leitner
2018-03-16 19:23 ` [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message Marcelo Ricardo Leitner
2018-03-18 16:11 ` David Ahern
2018-03-18 18:19 ` Marcelo Ricardo Leitner
2018-03-19 4:27 ` David Ahern
2018-03-19 15:34 ` Marcelo Ricardo Leitner
2018-03-16 19:23 ` [PATCH RFC 2/2] sched: pass extack through even if skip_sw is not set Marcelo Ricardo Leitner
2018-03-16 19:27 ` [PATCH RFC 0/2] Add support for warnings to extack Marcelo Ricardo Leitner
2018-03-16 22:05 ` Jakub Kicinski
2018-03-18 17:38 ` Marcelo Ricardo Leitner
2018-03-18 16:11 ` David Ahern
2018-03-18 18:20 ` Marcelo Ricardo Leitner
2018-03-18 18:36 ` Marcelo Ricardo Leitner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180319090950.3241e397@xeon-e3 \
--to=stephen@networkplumber.org \
--cc=aring@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=kubakici@wp.pl \
--cc=marcelo.leitner@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).