tidy and fix MUP Type2SessionTransformedRoute

The patch is large and does a lot:
- remove the need for useless teid_len
- add extra check on the teid and it size as per the draft
- reorder the json output of all MUP classes
- simply the __eq__  of all MUP classes
- fix the encoding test as it was not work
- fix the parsing so that generated routes are parsed back to the
original
- remove silly C style test where variable are after the constant (to
prevent unexpected set instead of comparaison)
- change tests to make the if path the error path
- give consistent naming to some fields
- use the newly create consume api from the tokeniser

etc/exabgp/conf-srv6-mup.conf

@@ -32,21 +32,16 @@ neighbor 127.0.0.1 {
 			next-hop 10.0.0.2 \
 			extended-community [ target:10:10 mup:10:10 ];
 
-			# The code generating those is in need of a code review
-			# for example it stores the IP len when not required and can found from the family
-			# the family itself is stored in a weird way ( the IP has family already )
-			# the code parsing this also does not check for invalid teid length
-			# not my code not fixing it, making the test pass /Thomas
-
-			# mup \
-			# mup-t2st 10.0.0.1 rd 100:100 teid 12345/23 \
-			# next-hop 10.0.0.2 \
-			# extended-community [ target:10:10 mup:10:10 ];
+			mup \
+			mup-t2st 10.0.0.1 rd 100:100 teid 12345/23 \
+			next-hop 10.0.0.2 \
+			extended-community [ target:10:10 mup:10:10 ];
 
-			# mup \
-			# mup-t2st 10.0.0.1 rd 100:100 teid 12345/0 \
-			# next-hop 10.0.0.2 \
-			# extended-community [ target:10:10 mup:10:10 ];
+			# was teid 12345/0 but we can not store the value 12345 using 0 bits
+			mup \
+			mup-t2st 10.0.0.1 rd 100:100 teid 0/0 \
+			next-hop 10.0.0.2 \
+			extended-community [ target:10:10 mup:10:10 ];
 		}
 		ipv6 {
 			mup \

qa/encoding/conf-srv6-mup.msg

@@ -4,8 +4,8 @@
 1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:0088:02:000000714001010040020040050400000064C010100002000A0000000A0C00000A0000000AC028250500220001001E0020010DB800010001000000000000000000001300010006401810000000800E250001551020010000000000000000000000000002000100020C00000064000000640A000001
 1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:005E:02:00000047400101004002004003040A00000240050400000064C010080002000A0000000A800E24000155040A0000020001000317000000640000006420C0A800010000303909200A000001
 1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:0060:02:00000049400101004002004003040A00000240050400000064C010100002000A0000000A0C00000A0000000A800E1E000155040A00000200010004110000006400000064400A00000100003039
-#1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:005F:02:00000048400101004002004003040A00000240050400000064C010100002000A0000000A0C00000A0000000A800E1D000155040A00000200010004100000006400000064370A000001000030
-#1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:005C:02:00000045400101004002004003040A00000240050400000064C010100002000A0000000A0C00000A0000000A800E1A000155040A000002000100040D0000006400000064200A000001
+1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:005F:02:00000048400101004002004003040A00000240050400000064C010100002000A0000000A0C00000A0000000A800E1D000155040A00000200010004100000006400000064370A000001003039
+1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:005C:02:00000045400101004002004003040A00000240050400000064C010100002000A0000000A0C00000A0000000A800E1A000155040A000002000100040D0000006400000064200A000001
 1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:0085:02:0000006E4001010040020040050400000064C010080002000A0000000AC028250500220001001E0020010DB800010001000000000000000000004700010006401810000000800E2A000255102001000000000000000000000000000200010001110000006400000064402001000000000000
 1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:0094:02:0000007D4001010040020040050400000064C010100002000A0000000A0C00000A0000000AC028250500220001001E0020010DB800020002000000000000000000001800010006401810000000800E3100025510200100000000000000000000000000020001000218000000640000006420010000000000000000000000000001
 1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:007B:02:000000644001010040020040050400000064C010080002000A0000000A800E480002551000000000000000000000FFFF0A000002000100032F00000064000000648020010DB800010001000000000000000100003039098020010000000000000000000000000001

src/exabgp/bgp/message/update/nlri/mup/dsd.py

@@ -40,8 +40,6 @@ def index(self):
     def __eq__(self, other):
         return (
             isinstance(other, DirectSegmentDiscoveryRoute)
-            and self.ARCHTYPE == other.ARCHTYPE
-            and self.CODE == other.CODE
             and self.rd == other.rd
             and self.ip == other.ip
         )
@@ -87,10 +85,10 @@ def unpack(cls, data, afi):
         return cls(rd, ip, afi)
 
     def json(self, compact=None):
-        content = '"arch": %d, ' % self.ARCHTYPE
+        content = '"name": "%s", ' % self.NAME
+        content += '"arch": %d, ' % self.ARCHTYPE
         content += '"code": %d, ' % self.CODE
-        content += '"raw": "%s", ' % self._raw()
-        content += '"name": "%s", ' % self.NAME
-        content += self.rd.json() + ', '
-        content += '"ip": "%s"' % str(self.ip)
+        content += '"ip": "%s", ' % str(self.ip)
+        content += self.rd.json()
+        content += ', "raw": "%s"' % self._raw()
         return '{ %s }' % content

src/exabgp/bgp/message/update/nlri/mup/isd.py

@@ -30,11 +30,11 @@ class InterworkSegmentDiscoveryRoute(MUP):
     NAME = "InterworkSegmentDiscoveryRoute"
     SHORT_NAME = "ISD"
 
-    def __init__(self, rd, ipprefix_len, ipprefix, afi, packed=None):
+    def __init__(self, rd, prefix_ip_len, prefix_ip, afi, packed=None):
         MUP.__init__(self, afi)
         self.rd = rd
-        self.ipprefix_len = ipprefix_len
-        self.ipprefix = ipprefix
+        self.prefix_ip_len = prefix_ip_len
+        self.prefix_ip = prefix_ip
         self._pack(packed)
 
     def index(self):
@@ -43,21 +43,19 @@ def index(self):
     def __eq__(self, other):
         return (
             isinstance(other, InterworkSegmentDiscoveryRoute)
-            and self.ARCHTYPE == other.ARCHTYPE
-            and self.CODE == other.CODE
             and self.rd == other.rd
-            and self.ipprefix_len == other.ipprefix_len
-            and self.ipprefix == other.ipprefix
+            and self.prefix_ip_len == other.prefix_ip_len
+            and self.prefix_ip == other.prefix_ip
         )
 
     def __ne__(self, other):
         return not self.__eq__(other)
 
     def __str__(self):
-        return "%s:%s:%s%s" % (self._prefix(), self.rd._str(), self.ipprefix, "/%d" % self.ipprefix_len)
+        return "%s:%s:%s%s" % (self._prefix(), self.rd._str(), self.prefix_ip, "/%d" % self.prefix_ip_len)
 
     def __hash__(self):
-        return hash((self.rd, self.ipprefix_len, self.ipprefix))
+        return hash((self.rd, self.prefix_ip_len, self.prefix_ip))
 
     def _pack(self, packed=None):
         if self._packed:
@@ -67,41 +65,40 @@ def _pack(self, packed=None):
             self._packed = packed
             return packed
 
-        offset = self.ipprefix_len // 8
-        remainder = self.ipprefix_len % 8
+        offset = self.prefix_ip_len // 8
+        remainder = self.prefix_ip_len % 8
         if remainder != 0:
             offset += 1
 
-        ipprefix_packed = self.ipprefix.pack()
+        prefix_ip_packed = self.prefix_ip.pack()
         # fmt: off
         self._packed = (
             self.rd.pack()
-            + pack('!B',self.ipprefix_len)
-            + ipprefix_packed[0: offset]
+            + pack('!B',self.prefix_ip_len)
+            + prefix_ip_packed[0: offset]
         )
         # fmt: on
         return self._packed
 
     @classmethod
     def unpack(cls, data, afi):
         rd = RouteDistinguisher.unpack(data[:8])
-        ipprefix_len = data[8]
+        prefix_ip_len = data[8]
         size = 4 if afi != AFI.ipv6 else 16
         ip = data[9:]
         padding = size - len(ip)
         if padding != 0 and 0 < padding:
             ip += bytes(padding)
+        prefix_ip = IP.unpack(ip)
 
-        ipprefix = IP.unpack(ip)
-
-        return cls(rd, ipprefix_len, ipprefix, afi)
+        return cls(rd, prefix_ip_len, prefix_ip, afi)
 
     def json(self, compact=None):
-        content = '"arch": %d, ' % self.ARCHTYPE
+        content = '"name": "%s", ' % self.NAME
+        content += '"arch": %d, ' % self.ARCHTYPE
         content += '"code": %d, ' % self.CODE
-        content += '"raw": "%s", ' % self._raw()
-        content += '"name": "%s", ' % self.NAME
-        content += self.rd.json() + ', '
-        content += '"ipprefix_len": %d, ' % self.ipprefix_len
-        content += '"ipprefix": "%s"' % str(self.ipprefix)
+        content += '"prefix_ip_len": %d, ' % self.prefix_ip_len
+        content += '"prefix_ip": "%s", ' % str(self.prefix_ip)
+        content += self.rd.json() 
+        content += ', "raw": "%s"' % self._raw()
         return '{ %s }' % content

src/exabgp/bgp/message/update/nlri/mup/nlri.py

@@ -31,8 +31,8 @@ class MUP(NLRI):
     registered = dict()
 
     # NEED to be defined in the subclasses
-    ARCHTYPE = -1
-    CODE = -1
+    ARCHTYPE = 0
+    CODE = 0
     NAME = 'Unknown'
     SHORT_NAME = 'unknown'
 

src/exabgp/bgp/message/update/nlri/mup/t1st.py

@@ -50,21 +50,21 @@ class Type1SessionTransformedRoute(MUP):
     def __init__(
         self,
         rd,
-        ipprefix_len,
-        ipprefix,
+        prefix_ip_len,
+        prefix_ip,
         teid,
         qfi,
         endpoint_ip_len,
         endpoint_ip,
-        afi,
         source_ip_len,
         source_ip,
+        afi,
         packed=None,
     ):
         MUP.__init__(self, afi)
         self.rd = rd
-        self.ipprefix_len = ipprefix_len
-        self.ipprefix = ipprefix
+        self.prefix_ip_len = prefix_ip_len
+        self.prefix_ip = prefix_ip
         self.teid = teid
         self.qfi = qfi
         self.endpoint_ip_len = endpoint_ip_len
@@ -76,11 +76,11 @@ def __init__(
     def __eq__(self, other):
         return (
             isinstance(other, Type1SessionTransformedRoute)
-            and self.ARCHTYPE == other.ARCHTYPE
-            and self.CODE == other.CODE
+            # and self.ARCHTYPE == other.ARCHTYPE
+            # and self.CODE == other.CODE
             and self.rd == other.rd
-            and self.ipprefix_len == other.ipprefix_len
-            and self.ipprefix == other.ipprefix
+            and self.prefix_ip_len == other.prefix_ip_len
+            and self.prefix_ip == other.prefix_ip
             and self.teid == other.teid
             and self.qfi == other.qfi
             and self.endpoint_ip_len == other.endpoint_ip_len
@@ -96,12 +96,12 @@ def __str__(self):
         s = "%s:%s:%s%s:%s:%s:%s%s" % (
             self._prefix(),
             self.rd._str(),
-            self.ipprefix,
-            "/%d" % self.ipprefix_len,
+            self.prefix_ip,
+            "/%d" % self.prefix_ip_len,
             self.teid,
             self.qfi,
             self.endpoint_ip,
-            "/%d" % self.ipprefix_len,
+            "/%d" % self.prefix_ip_len,
         )
 
         if self.source_ip_len != 0 and self.source_ip != b'':
@@ -111,7 +111,7 @@ def __str__(self):
 
     def pack_index(self):
         # removed teid, qfi, endpointip
-        packed = self.rd.pack() + pack('!B', self.ipprefix_len) + self.ipprefix.pack()
+        packed = self.rd.pack() + pack('!B', self.prefix_ip_len) + self.prefix_ip.pack()
         return pack('!BHB', self.ARCHTYPE, self.CODE, len(packed)) + packed
 
     def index(self):
@@ -121,8 +121,8 @@ def __hash__(self):
         return hash(
             (
                 self.rd,
-                self.ipprefix_len,
-                self.ipprefix,
+                self.prefix_ip_len,
+                self.prefix_ip,
                 self.teid,
                 self.qfi,
                 self.endpoint_ip_len,
@@ -140,18 +140,18 @@ def _pack(self, packed=None):
             self._packed = packed
             return packed
 
-        offset = self.ipprefix_len // 8
-        remainder = self.ipprefix_len % 8
+        offset = self.prefix_ip_len // 8
+        remainder = self.prefix_ip_len % 8
         if remainder != 0:
             offset += 1
 
-        ipprefix_packed = self.ipprefix.pack()
+        prefix_ip_packed = self.prefix_ip.pack()
 
         # fmt: off
         self._packed = (
             self.rd.pack()
-            + pack('!B',self.ipprefix_len)
-            + ipprefix_packed[0: offset]
+            + pack('!B',self.prefix_ip_len)
+            + prefix_ip_packed[0: offset]
             + pack('!IB',self.teid, self.qfi)
             + pack('!B',self.endpoint_ip_len)
             + self.endpoint_ip.pack()
@@ -167,9 +167,9 @@ def _pack(self, packed=None):
     def unpack(cls, data, afi):
         datasize = len(data)
         rd = RouteDistinguisher.unpack(data[:8])
-        ipprefix_len = data[8]
-        ip_offset = ipprefix_len // 8
-        ip_remainder = ipprefix_len % 8
+        prefix_ip_len = data[8]
+        ip_offset = prefix_ip_len // 8
+        ip_remainder = prefix_ip_len % 8
         if ip_remainder != 0:
             ip_offset += 1
 
@@ -180,7 +180,7 @@ def unpack(cls, data, afi):
             ip += bytes(ip_padding)
 
         size = ip_offset
-        ipprefix = IP.unpack(ip)
+        prefix_ip = IP.unpack(ip)
         size += 9
         teid = int.from_bytes(data[size : size + 4], "big")
         size += 4
@@ -189,42 +189,41 @@ def unpack(cls, data, afi):
         endpoint_ip_len = data[size]
         size += 1
 
-        if endpoint_ip_len in [32, 128]:
-            ep_len = endpoint_ip_len // 8
-            endpoint_ip = IP.unpack(data[size : size + ep_len])
-            size += ep_len
-        else:
+        if endpoint_ip_len not in [32, 128]:
             raise RuntimeError('mup t1st endpoint ip length is not 32bit or 128bit, unexpect len: %d' % endpoint_ip_len)
 
+        ep_len = endpoint_ip_len // 8
+        endpoint_ip = IP.unpack(data[size : size + ep_len])
+        size += ep_len
+
         source_ip_size = datasize - size
 
         source_ip_len = 0
         source_ip = b''
 
-        if 0 < source_ip_size:
+        if source_ip_size > 0:
             source_ip_len = data[size]
             size += 1
-            if source_ip_len in [32, 128]:
-                sip_len = source_ip_len // 8
-                source_ip = IP.unpack(data[size : size + sip_len])
-                size += sip_len
-            else:
+            if source_ip_len not in [32, 128]:
                 raise RuntimeError('mup t1st source ip length is not 32bit or 128bit, unexpect len: %d' % source_ip_len)
+            sip_len = source_ip_len // 8
+            source_ip = IP.unpack(data[size : size + sip_len])
+            size += sip_len
 
-        return cls(rd, ipprefix_len, ipprefix, teid, qfi, endpoint_ip_len, endpoint_ip, afi, source_ip_len, source_ip)
+        return cls(rd, prefix_ip_len, prefix_ip, teid, qfi, endpoint_ip_len, endpoint_ip, source_ip_len, source_ip, afi)
 
     def json(self, compact=None):
-        content = '"arch": %d, ' % self.ARCHTYPE
+        content = '"name": "%s", ' % self.NAME
+        content += '"arch": %d, ' % self.ARCHTYPE
         content += '"code": %d, ' % self.CODE
-        content += '"raw": "%s", ' % self._raw()
-        content += '"name": "%s", ' % self.NAME
-        content +=  self.rd.json() + ', '
-        content += '"ipprefix_len": %d, ' % self.ipprefix_len
-        content += '"ipprefix": "%s", ' % str(self.ipprefix)
+        content += '"prefix_ip_len": %d, ' % self.prefix_ip_len
+        content += '"prefix_ip": "%s", ' % str(self.prefix_ip)
         content += '"teid": "%s", ' % str(self.teid)
         content += '"qfi": "%s", ' % str(self.qfi)
+        content +=  self.rd.json() + ', '
         content += '"endpoint_ip_len": %d, ' % self.endpoint_ip_len
         content += '"endpoint_ip": "%s"' % str(self.endpoint_ip)
         content += '"source_ip_len": %d, ' % self.source_ip_len
-        content += '"source_ip": "%s"' % str(self.source_ip)
+        content += '"source_ip": "%s", ' % str(self.source_ip)
+        content += '"raw": "%s"' % self._raw()
         return '{ %s }' % content