[ovs-dev] [bugfixes 2/3] datapath: Fix behavior of NLA_NESTED for pre-2.6.29 kernels.

Ben Pfaff blp at nicira.com
Tue Feb 1 04:23:30 UTC 2011


On Mon, Jan 31, 2011 at 05:40:21PM -0800, Jesse Gross wrote:
> On Mon, Jan 31, 2011 at 4:48 PM, Ben Pfaff <blp at nicira.com> wrote:
> > Before v2.6.29, a NLA_NESTED attribute, if it was present, was not allowed
> > to be empty. ?However, OVS depends on the ability to accept empty
> > attributes. ?For example, a present but empty ODP_FLOW_ATTR_ACTIONS on
> > ODP_FLOW_CMD_SET replaces the existing set of actions by an empty "drop"
> > action, whereas a missing ODP_FLOW_ATTR_ACTIONS leaves the existing
> > actions, if any, unchanged.
> >
> > The datapath code currently doesn't otherwise depend on the NLA_NESTED
> > semantics (we only pass such attributes to nla_parse_nested() and
> > nla_for_each_nested(), which don't care if the attribute's length is
> > valid), so we might as well just use NLA_UNSPEC here.
> 
> I don't understand this comment.  nla_parse_nested() absolutely cares
> that the attribute's length is valid: it uses it to know how much data
> is there to parse.  However, the previous round of parsing checks that
> attributes are well formed and that the length of the attribute does
> not exceed the available size.
> 
> That being said, I only see two differences between NLA_NESTED and NLA_UNSPEC:
> * If the size of the nested attributes is zero, no further size checks
> are performed.
> * If the size of the nested attributes is not zero and no length
> parameter is specified the minimum size of nested attributes is
> NLA_HDRLEN.
> 
> nla_parse_nested() validates that there is at least enough space for
> NLA_HDRLEN, so neither of these conditions are important.  So I think
> that this change is fine but the description is very misleading.

OK, here's an updated change log:

    datapath: Fix behavior of NLA_NESTED for pre-2.6.29 kernels.
    
    Before v2.6.29, a NLA_NESTED attribute, if it was present, was not allowed
    to be empty.  However, OVS depends on the ability to accept empty
    attributes.  For example, a present but empty ODP_FLOW_ATTR_ACTIONS on
    ODP_FLOW_CMD_SET replaces the existing set of actions by an empty "drop"
    action, whereas a missing ODP_FLOW_ATTR_ACTIONS leaves the existing
    actions, if any, unchanged.
    
    NLA_NESTED is different from NLA_UNSPEC in only two ways:
    
    * If the size of the nested attributes is zero, no further size checks
      are performed.
    
    * If the size of the nested attributes is not zero and no length
      parameter is specified the minimum size of nested attributes is
      NLA_HDRLEN.
    
    nla_parse_nested() validates that there is at least enough space for
    NLA_HDRLEN, so neither of these conditions are important, and we might
    as well use NLA_UNSPEC with old kernels.
    
    Signed-off-by: Ben Pfaff <blp at nicira.com>




More information about the dev mailing list