snabb icon indicating copy to clipboard operation
snabb copied to clipboard

Invalid parser for YANG enumeration type

Open alexandergall opened this issue 6 years ago • 4 comments

Consider the schema

module test {
  namespace test;
  prefix test;
  
  leaf test {
    type enumeration {
      enum foo { value 1; }
      enum bar { value 2; }
    }
  }
}

In my understanding, the following should be a valid configuration

test foo;

and the parser should produce the Lua table

{ test = 1 }

Instead, I get an error

test foo;
         ^
lib/yang/parser.lua:49: <unknown>:1:9: error: enumeration foo is not a valid value

The error originates from lib.yang.data

local function enum_validator(enums, f)
   if not enums then return f end
   return function (val, P)
      if not enums[val] then
         P:error('enumeration '..val..' is not a valid value')
      end
      return f(val, P)
   end
end

The enums table is keyed by the enumeration values, rather than the names of the enum objects. This seems backwards to me unless I completely misunderstand the semantics of enumeration.

alexandergall avatar Feb 04 '19 08:02 alexandergall

Looking a bit at the code it seems to me that the semantics of enumeration have just not been implemented yet. When enum_validator() is called, the substatements have been parsed into the array

{ {
    if_features = {},
    kind = "enum",
    name = "foo",
    value = 1
  }, {
    if_features = {},
    kind = "enum",
    name = "bar",
    value = 2
  } }

The value from the configuration is then used as a key into this array, which is completely bogus. What should happen is that the missing values are assigned according to 9.6.4.2 of RFC6020 (in this example they are set explicitely but that's optional) and the value from the configuration needs to be compared with the name attribute of the enums. The first part probably needs to be done somewhere else.

alexandergall avatar Feb 04 '19 16:02 alexandergall

Good investigation. Definitely this is a bug, enum_validator should validate on key, not value. If you need to continue with your work, simply disable the enum_validator. I'd try to look into this issue if I have some spare time.

dpino avatar Feb 05 '19 13:02 dpino

I have implemented this here https://github.com/alexandergall/snabbswitch/tree/yang-enumeration. I tried to guess where the code should go :) Can you please have a look (I'm also unsure about the style of error messages this should generate)?

I'm not sure whether it's actually useful to set the numeric value in the final Lua table or the name of the selected enum. My code currently does the latter, i.e. in the example above, the result is

{ test = "foo" }

This seems more useful since otherwise the Lua code processing this table needs to define the same name-to-value mapping as the schema.

alexandergall avatar Feb 05 '19 13:02 alexandergall

Proposed fix: https://github.com/snabbco/snabb/pull/1416

alexandergall avatar Feb 05 '19 15:02 alexandergall