[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080822.034454.107392192.davem@davemloft.net>
Date: Fri, 22 Aug 2008 03:44:54 -0700 (PDT)
From: David Miller <davem@...emloft.net>
To: alexander.duyck@...il.com
Cc: shemminger@...tta.com, jeffrey.t.kirsher@...el.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
akpm@...ux-foundation.org, alexander.h.duyck@...el.com,
tgraf@...g.ch
Subject: Re: [PATCH] IPROUTE: correct nla nested message generated by
netem_parse_opt
From: "Alexander Duyck" <alexander.duyck@...il.com>
Date: Thu, 21 Aug 2008 19:37:08 -0700
Thomas Graf added to CC:.
> On Thu, Aug 21, 2008 at 6:19 PM, Stephen Hemminger
> <shemminger@...tta.com> wrote:
> > On Thu, 21 Aug 2008 17:50:11 -0700
> > Jeff Kirsher <jeffrey.t.kirsher@...el.com> wrote:
> >
> >> From: Alexander Duyck <alexander.h.duyck@...el.com>
> >>
> >> netem_parse_opt was generating a malformed nested compat message. This patch
> >> corrects it so that the nested arguments are contained within a nested nla
> >> header.
> >>
> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>
> >> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> >> ---
> >>
> >> tc/q_netem.c | 9 +++------
> >> 1 files changed, 3 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tc/q_netem.c b/tc/q_netem.c
> >> index d06932e..a3365c1 100644
> >> --- a/tc/q_netem.c
> >> +++ b/tc/q_netem.c
> >> @@ -128,7 +128,7 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> >> struct nlmsghdr *n)
> >> {
> >> size_t dist_size = 0;
> >> - struct rtattr *tail;
> >> + struct rtattr *nest;
> >> struct tc_netem_qopt opt;
> >> struct tc_netem_corr cor;
> >> struct tc_netem_reorder reorder;
> >> @@ -257,8 +257,6 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> >> argc--; argv++;
> >> }
> >>
> >> - tail = NLMSG_TAIL(n);
> >> -
> >> if (reorder.probability) {
> >> if (opt.latency == 0) {
> >> fprintf(stderr, "reordering not possible without specifying some delay\n");
> >> @@ -277,8 +275,7 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> >> return -1;
> >> }
> >>
> >> - if (addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt)) < 0)
> >> - return -1;
> >> + nest = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
> >>
> >> if (present[TCA_NETEM_CORR] &&
> >> addattr_l(n, 1024, TCA_NETEM_CORR, &cor, sizeof(cor)) < 0)
> >> @@ -299,7 +296,7 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> >> return -1;
> >> free(dist_data);
> >> }
> >> - tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
> >> + addattr_nest_compat_end(n, nest);
> >> return 0;
> >> }
> >>
> >>
> >
> > The kernel ABI can not change? The nested attribute order should not change.
>
> The problem is the current kernel ABI was changed in commit
> b9a2f2e450b0f770bb4347ae8d48eb2dea701e24 "netlink: Fix
> nla_parse_nested_compat() to call nla_parse() directly" to support a
> format that I have only seen generated in netem_parse_opt. The kernel
> and prio_parse_opt qdisc both generate the other format which includes
> the extra attribute header to contain the nested attributes. This
> patch fixes tc so that netem will use the correct nested attribute
> order for kernels prior to this commit and once this commit has been
> reverted.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists