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

["Alexander Duyck" <alexander.duyck@gmail.com>]

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.