[lkml]   [2008]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] IPROUTE: correct nla nested message generated by netem_parse_opt
    From: Thomas Graf <>
    Date: Wed, 27 Aug 2008 16:41:22 +0200

    > There is two ways of sending a fixed struct + attributes inside an
    > attribute:
    > a) (old and outdated method)
    > attr foo
    > fixed struct
    > [nested attr 1]
    > [nested attr 2]
    > This format can be parsed with nla_parse_nested_compat(attr foo)
    > b) (new method)
    > attr foo
    > fixed struct
    > attr container
    > [nested attr 1]
    > [nested attr 2]
    > This format is parsed with nla_parse_nested(attr container)

    Correct, but here is the sequence of events I discovered after
    researching this fully:

    1) Patrick adds nla_next_compat*() and NLA_NESTED_COMPAT, in this

    commit 1092cb219774a82b1f16781aec7b8d4ec727c981
    Author: Patrick McHardy <>
    Date: Mon Jun 25 13:49:35 2007 -0700

    [NETLINK]: attr: add nested compat attribute type

    2) The multiqueue packet scheduler bits got added by Peter W. in
    this changeset:

    commit d62733c8e437fdb58325617c4b3331769ba82d70
    Author: Peter P Waskiewicz Jr <>
    Date: Thu Jun 28 21:04:31 2007 -0700

    [SCHED]: Qdisc changes and sch_rr added for multiqueue

    The thing to note is that at this point this code is using
    rtattr_parse_nested_compat(), RTA_NEST_COMPAT*(), etc.

    This went into 2.6.23

    3) iproute2 gets sch_rr support from Peter W. on August 14th, from
    this iproute2 commit:

    commit 292ce96bca64dee087fe00d38743f5e2d1895c5d
    Author: PJ Waskiewicz <>
    Date: Tue Aug 14 11:21:24 2007 -0700

    iproute2: sch_rr support in tc

    4) On January 22nd, 2008, Patrick converted all of the packet schedulers
    to nla_*(), nlattr, etc.

    commit 1e90474c377e92db7262a8968a45c1dd980ca9e5
    Author: Patrick McHardy <>
    Date: Tue Jan 22 22:11:17 2008 -0800

    [NET_SCHED]: Convert packet schedulers from rtnetlink to new netlink API

    At this point, even though PRIO and RR were converted as well, they
    were still working properly with iproute2 as-is.

    This went into 2.6.25

    5) One day later Patrick converted netem to nla_parse_nested_compat:

    commit b03f4672007e533c8dbf0965f995182586216bf1
    Author: Patrick McHardy <>
    Date: Wed Jan 23 20:32:21 2008 -0800

    [NET_SCHED]: sch_netem: use nla_parse_nested_compat

    This broke netem.

    This also went into 2.6.25

    6) And then on May 22nd, 2008 we get the changeset that has obtained
    a lot of discussion:

    commit b9a2f2e450b0f770bb4347ae8d48eb2dea701e24
    Author: Thomas Graf <>
    Date: Thu May 22 10:48:59 2008 -0700

    netlink: Fix nla_parse_nested_compat() to call nla_parse() directly

    This fixed netem but broke prio and rr qdiscs.

    This went into 2.6.26 and also

    And as a result 2.6.26 was released with broken netlink parsing
    for prio and rr qdiscs, as was and later 2.6.25 releases.

    Base 2.6.25 was fine, because it was after Patrick's conversion of
    prio and rr to nla_parse_nested_compat(), but before Thomas's netem
    fix in #6

    So all the trouble started with Patrick's netem conversion in
    commit #5 above. It was not an equivalent transformation. But
    in fixing netem in #6 we broke prio and rr, which both expected
    the previous behavior of nla_parse_nested_compat().

    Looking at this situation, I have to say that Alexander has been
    correct from the beginning, and I think we should revert both
    #5 and #6 to fix this bug in all cases where they appear, and
    that would be: mainline, 2.6.26-stable 2.6.25-stable

    It doesn't matter what "correct" compat nested attribute parsing
    is or is not. The fact is that RR and PRIO were using a certain
    format, consistently, prior to commit #6. This is what was
    codified in userspace in the iproute2 sources and worked
    correctly until #6.

     \ /
      Last update: 2008-08-28 08:51    [W:0.025 / U:6.804 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site