[PATCH] act: fix the logic of parsing actions

[Thomas Haller]

On Tue, 2014-04-15 at 13:27 -0700, Cong Wang wrote:
> On Tue, Apr 15, 2014 at 4:01 AM, Thomas Haller <thaller at redhat.com> wrote:
> > Hi Cong,
> >
> 
> Hi, Thomas
> 
> >
> > The parsed rtnl_act instances are put in the linked list @head. Up to
> > now, the index of the items in the list correspond to the netlink
> > attribute (@i) as they were provided by the netlink message.
> >
> > With this patch (by skipping over wholes), the index does no longer
> > correspond to the attribute. See also rtnl_act_fill() which seems to
> > care about the index too.
> >
> > Is this really correct? Maybe we would have to insert a dummy-object
> > instead of skipping over with continue?
> >
> 
> I am not sure either, but this is at least same with what iproute2 does:
> 
> In tc_print_action():
> 
>         for (i = 0; i < TCA_ACT_MAX_PRIO; i++) {
>                 if (tb[i]) {
>                         fprintf(f, "\n\taction order %d: ", i);
>                         if (0 > tc_print_one_action(f, tb[i])) {
>                                 fprintf(f, "Error printing action\n");
>                         }
>                 }
> 
>         }
> 
> And in tc_action_gd():
> 
>                 addattr_l(&req.n, MAX_MSG, ++prio, NULL, 0);
> 
> 
> Also, without this patch, I can't get any action from cache even when
> I am sure there is one.

Hi Cong, Thomas,


It seems wrong, that we loose the correlation between netlink
attribute/order and the index.

Especially, rtnl_act_msg_build() will now no longer produce what
rtnl_act_msg_parse() parses.

Anyway. I don't know :)

@Thomas, could you please (N)ACK this patch?


Thomas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20140430/297efcbe/attachment.sig>