Skip to content

Commit

Permalink
[openconfig_acl] Allow setting ICMP type/code to 0 (sonic-net#6932)
Browse files Browse the repository at this point in the history 
There is a bug in how pyangbind translates yang models into python bindings. The model always sets integer values to 0 by default, so there is no way to check if a user has provided a value that is equal to 0. This is problematic for ICMP and VLAN (among others) because 0 is a valid input value.

This change converts ICMP and VLAN fields to union types so that acl-loader will treat them as null values unless a user explicitly adds an integer value.

Signed-off-by: Danny Allen <daall@microsoft.com>
  • Loading branch information
@daall @yxieca
daall authored and yxieca committed Mar 3, 2021
1 parent bc0cf44 commit 96edb96
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 18 deletions.
24 changes: 12 additions & 12 deletions src/sonic-config-engine/openconfig_acl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ def __init__(self, *args, **kwargs):
self.__destination_mac_mask = YANGDynClass(base=RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}'}), is_leaf=True, yang_name="destination-mac-mask", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='http://openconfig.net/yang/acl', defining_module='openconfig-acl', yang_type='yang:mac-address', is_config=True)
self.__source_mac = YANGDynClass(base=RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}'}), is_leaf=True, yang_name="source-mac", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='http://openconfig.net/yang/acl', defining_module='openconfig-acl', yang_type='yang:mac-address', is_config=True)
self.__source_mac_mask = YANGDynClass(base=RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}'}), is_leaf=True, yang_name="source-mac-mask", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='http://openconfig.net/yang/acl', defining_module='openconfig-acl', yang_type='yang:mac-address', is_config=True)
self.__vlan_id = YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'0..4095']}), is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)
self.__vlan_id = YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'1..4095']}),], is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)
self.__destination_mac = YANGDynClass(base=RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}'}), is_leaf=True, yang_name="destination-mac", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='http://openconfig.net/yang/acl', defining_module='openconfig-acl', yang_type='yang:mac-address', is_config=True)

load = kwargs.pop("load", None)
Expand Down Expand Up @@ -1068,20 +1068,20 @@ def _set_vlan_id(self, v, load=False):
if hasattr(v, "_utype"):
v = v._utype(v)
try:
t = YANGDynClass(v,base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'0..4095']}), is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)
t = YANGDynClass(v,base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'1..4095']}),], is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)
except (TypeError, ValueError):
raise ValueError({
'error-string': """vlan_id must be of a type compatible with vlan-id-type""",
'defined-type': "sonic-acl-extension:vlan-id-type",
'generated-type': """YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'0..4095']}), is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)""",
'generated-type': """YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'1..4095']}),], is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)""",
})

self.__vlan_id = t
if hasattr(self, '_set'):
self._set()

def _unset_vlan_id(self):
self.__vlan_id = YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'0..4095']}), is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)
self.__vlan_id = YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..65535']},int_size=16), restriction_dict={u'range': [u'1..4095']}),], is_leaf=True, yang_name="vlan-id", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='vlan-id-type', is_config=True)

source_mac = __builtin__.property(_get_source_mac, _set_source_mac)
source_mac_mask = __builtin__.property(_get_source_mac_mask, _set_source_mac_mask)
Expand Down Expand Up @@ -3723,8 +3723,8 @@ def __init__(self, *args, **kwargs):
self._path_helper = False

self._extmethods = False
self.__code = YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)
self.__type = YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)
self.__code = YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)
self.__type = YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)

load = kwargs.pop("load", None)
if args:
Expand Down Expand Up @@ -3774,20 +3774,20 @@ def _set_type(self, v, load=False):
if hasattr(v, "_utype"):
v = v._utype(v)
try:
t = YANGDynClass(v,base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)
t = YANGDynClass(v,base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)
except (TypeError, ValueError):
raise ValueError({
'error-string': """type must be of a type compatible with icmp-type-type""",
'defined-type': "sonic-acl-extension:icmp-type-type",
'generated-type': """YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)""",
'generated-type': """YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)""",
})

self.__type = t
if hasattr(self, '_set'):
self._set()

def _unset_type(self):
self.__type = YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)
self.__type = YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="type", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-type-type', is_config=True)


def _get_code(self):
Expand All @@ -3811,20 +3811,20 @@ def _set_code(self, v, load=False):
if hasattr(v, "_utype"):
v = v._utype(v)
try:
t = YANGDynClass(v,base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)
t = YANGDynClass(v,base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)
except (TypeError, ValueError):
raise ValueError({
'error-string': """code must be of a type compatible with icmp-code-type""",
'defined-type': "sonic-acl-extension:icmp-code-type",
'generated-type': """YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)""",
'generated-type': """YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)""",
})

self.__code = t
if hasattr(self, '_set'):
self._set()

def _unset_code(self):
self.__code = YANGDynClass(base=RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}), is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)
self.__code = YANGDynClass(base=[RestrictedClassType(base_type=six.text_type, restriction_dict={u'pattern': u'null'}),RestrictedClassType(base_type=RestrictedClassType(base_type=int, restriction_dict={'range': ['0..255']}, int_size=8), restriction_dict={u'range': [u'0..255']}),], is_leaf=True, yang_name="code", parent=self, path_helper=self._path_helper, extmethods=self._extmethods, register_paths=True, namespace='https://github.com/Azure/sonic-buildimage', defining_module='sonic-acl-extension', yang_type='icmp-code-type', is_config=True)

type = __builtin__.property(_get_type, _set_type)
code = __builtin__.property(_get_code, _set_code)
Expand Down
27 changes: 21 additions & 6 deletions src/sonic-config-engine/sonic-acl-extension.yang
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,39 @@ module sonic-acl-extension {
import openconfig-acl { prefix oc-acl; }

typedef vlan-id-type {
type uint16 {
range 0..4095;
type union {
type string {
pattern "null";
}
type uint16 {
range 1..4095;
}
}
description
"The VLAN ID value may be expressed as a 12-bit number in decimal notation";
}

typedef icmp-type-type {
type uint8 {
range 0..255;
type union {
type string {
pattern "null";
}
type uint8 {
range 0..255;
}
}
description
"The ICMP type value may be expressed as an 8-bit number in decimal notation";
}

typedef icmp-code-type {
type uint8 {
range 0..255;
type union {
type string {
pattern "null";
}
type uint8 {
range 0..255;
}
}
description
"The ICMP code value may be expressed as an 8-bit number in decimal notation";
Expand Down

0 comments on commit 96edb96

Please sign in to comment.