Re: [PATCH] IPROUTE: correct nla nested message generated by netem_parse_opt

[David Miller <davem@davemloft.net>]

From: "Alexander Duyck" <alexander.duyck@gmail.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@vyatta.com> wrote:
> > On Thu, 21 Aug 2008 17:50:11 -0700
> > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> >
> >> From: Alexander Duyck <alexander.h.duyck@intel.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@intel.com>
> >> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.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.