diff --git a/cmd/gobgp/global.go b/cmd/gobgp/global.go index 68d510c3..b9c11c6c 100644 --- a/cmd/gobgp/global.go +++ b/cmd/gobgp/global.go @@ -952,16 +952,16 @@ func parseEvpnArgs(args []string) (bgp.AddrPrefixInterface, []string, error) { return nil, nil, fmt.Errorf("invalid subtype. expect [macadv|multicast|prefix] but %s", subtype) } -func parseMUPInterworkSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixInterface, []string, error) { +func parseMUPInterworkSegmentDiscoveryRouteArgs(args []string, nexthop string) (bgp.AddrPrefixInterface, []string, error) { // Format: - // rd [mup ] + // rd [rt ...] req := 5 if len(args) < req { return nil, nil, fmt.Errorf("%d args required at least, but got %d", req, len(args)) } m, err := extractReserved(args, map[string]int{ - "rd": paramSingle, - "mup": paramSingle, + "rd": paramSingle, + "rt": paramSingle, }) if err != nil { return nil, nil, err @@ -969,7 +969,7 @@ func parseMUPInterworkSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixIn if len(m[""]) < 1 { return nil, nil, fmt.Errorf("specify prefix") } - for _, f := range []string{"rd"} { + for _, f := range []string{"rd", "rt"} { for len(m[f]) == 0 { return nil, nil, fmt.Errorf("specify %s", f) } @@ -982,11 +982,18 @@ func parseMUPInterworkSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixIn if err != nil { return nil, nil, err } + nh, err := netip.ParseAddr(nexthop) + if err != nil { + return nil, nil, err + } + if nh.Is4() { + return nil, nil, fmt.Errorf("nexthop should be IPv6 address: %s", nexthop) + } + extcomms := make([]string, 0) - if len(m["mup"]) > 0 { - extcomms = append(extcomms, "mup", m["mup"][0]) - } else { - extcomms = append(extcomms, "mup", "0:0") + if len(m["rt"]) > 0 { + extcomms = append(extcomms, "rt") + extcomms = append(extcomms, m["rt"]...) } r := &bgp.MUPInterworkSegmentDiscoveryRoute{ @@ -997,15 +1004,16 @@ func parseMUPInterworkSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixIn return bgp.NewMUPNLRI(bgp.MUP_ARCH_TYPE_UNDEFINED, bgp.MUP_ROUTE_TYPE_INTERWORK_SEGMENT_DISCOVERY, r), extcomms, nil } -func parseMUPDirectSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixInterface, []string, error) { +func parseMUPDirectSegmentDiscoveryRouteArgs(args []string, nexthop string) (bgp.AddrPrefixInterface, []string, error) { // Format: - // rd [mup ] + // rd [rt ...] [mup ] req := 5 if len(args) < req { return nil, nil, fmt.Errorf("%d args required at least, but got %d", req, len(args)) } m, err := extractReserved(args, map[string]int{ "rd": paramSingle, + "rt": paramSingle, "mup": paramSingle, }) if err != nil { @@ -1014,7 +1022,7 @@ func parseMUPDirectSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixInter if len(m[""]) < 1 { return nil, nil, fmt.Errorf("specify address") } - for _, f := range []string{"rd"} { + for _, f := range []string{"rd", "rt", "mup"} { for len(m[f]) == 0 { return nil, nil, fmt.Errorf("specify %s", f) } @@ -1027,11 +1035,21 @@ func parseMUPDirectSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixInter if err != nil { return nil, nil, err } + nh, err := netip.ParseAddr(nexthop) + if err != nil { + return nil, nil, err + } + if nh.Is4() { + return nil, nil, fmt.Errorf("nexthop should be IPv6 address: %s", nexthop) + } + extcomms := make([]string, 0) + if len(m["rt"]) > 0 { + extcomms = append(extcomms, "rt") + extcomms = append(extcomms, m["rt"]...) + } if len(m["mup"]) > 0 { extcomms = append(extcomms, "mup", m["mup"][0]) - } else { - extcomms = append(extcomms, "mup", "0:0") } r := &bgp.MUPDirectSegmentDiscoveryRoute{ @@ -1043,17 +1061,17 @@ func parseMUPDirectSegmentDiscoveryRouteArgs(args []string) (bgp.AddrPrefixInter func parseMUPType1SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInterface, []string, error) { // Format: - // rd teid qfi endpoint [mup ] + // rd [rt ...] teid qfi endpoint req := 5 if len(args) < req { return nil, nil, fmt.Errorf("%d args required at least, but got %d", req, len(args)) } m, err := extractReserved(args, map[string]int{ "rd": paramSingle, + "rt": paramSingle, "teid": paramSingle, "qfi": paramSingle, "endpoint": paramSingle, - "mup": paramSingle, }) if err != nil { return nil, nil, err @@ -1061,7 +1079,7 @@ func parseMUPType1SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInte if len(m[""]) < 1 { return nil, nil, fmt.Errorf("specify prefix") } - for _, f := range []string{"rd", "teid", "qfi", "endpoint"} { + for _, f := range []string{"rd", "rt", "teid", "qfi", "endpoint"} { for len(m[f]) == 0 { return nil, nil, fmt.Errorf("specify %s", f) } @@ -1078,6 +1096,9 @@ func parseMUPType1SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInte if err != nil { return nil, nil, err } + if teid == 0 { + return nil, nil, fmt.Errorf("teid should not be 0") + } qfi, err := strconv.ParseUint(m["qfi"][0], 10, 8) if err != nil { return nil, nil, err @@ -1087,10 +1108,9 @@ func parseMUPType1SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInte return nil, nil, err } extcomms := make([]string, 0) - if len(m["mup"]) > 0 { - extcomms = append(extcomms, "mup", m["mup"][0]) - } else { - extcomms = append(extcomms, "mup", "0:0") + if len(m["rt"]) > 0 { + extcomms = append(extcomms, "rt") + extcomms = append(extcomms, m["rt"]...) } r := &bgp.MUPType1SessionTransformedRoute{ @@ -1107,15 +1127,15 @@ func parseMUPType1SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInte func parseMUPType2SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInterface, []string, error) { // Format: - // rd teid [mup ] + // rd [rt ...] teid req := 5 if len(args) < req { return nil, nil, fmt.Errorf("%d args required at least, but got %d", req, len(args)) } m, err := extractReserved(args, map[string]int{ "rd": paramSingle, + "rt": paramSingle, "teid": paramSingle, - "mup": paramSingle, }) if err != nil { return nil, nil, err @@ -1123,7 +1143,7 @@ func parseMUPType2SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInte if len(m[""]) < 1 { return nil, nil, fmt.Errorf("specify endpoint") } - for _, f := range []string{"rd", "teid"} { + for _, f := range []string{"rd", "rt", "teid"} { for len(m[f]) == 0 { return nil, nil, fmt.Errorf("specify %s", f) } @@ -1140,11 +1160,13 @@ func parseMUPType2SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInte if err != nil { return nil, nil, err } + if teid == 0 { + return nil, nil, fmt.Errorf("teid should not be 0") + } extcomms := make([]string, 0) - if len(m["mup"]) > 0 { - extcomms = append(extcomms, "mup", m["mup"][0]) - } else { - extcomms = append(extcomms, "mup", "0:0") + if len(m["rt"]) > 0 { + extcomms = append(extcomms, "rt") + extcomms = append(extcomms, m["rt"]...) } r := &bgp.MUPType2SessionTransformedRoute{ @@ -1156,7 +1178,7 @@ func parseMUPType2SessionTransformedRouteArgs(args []string) (bgp.AddrPrefixInte return bgp.NewMUPNLRI(bgp.MUP_ARCH_TYPE_UNDEFINED, bgp.MUP_ROUTE_TYPE_TYPE_2_SESSION_TRANSFORMED, r), extcomms, nil } -func parseMUPArgs(args []string) (bgp.AddrPrefixInterface, []string, error) { +func parseMUPArgs(args []string, nexthop string) (bgp.AddrPrefixInterface, []string, error) { if len(args) < 1 { return nil, nil, fmt.Errorf("lack of args. need 1 but %d", len(args)) } @@ -1164,9 +1186,9 @@ func parseMUPArgs(args []string) (bgp.AddrPrefixInterface, []string, error) { args = args[1:] switch subtype { case "isd": - return parseMUPInterworkSegmentDiscoveryRouteArgs(args) + return parseMUPInterworkSegmentDiscoveryRouteArgs(args, nexthop) case "dsd": - return parseMUPDirectSegmentDiscoveryRouteArgs(args) + return parseMUPDirectSegmentDiscoveryRouteArgs(args, nexthop) case "t1st": return parseMUPType1SessionTransformedRouteArgs(args) case "t2st": @@ -1562,7 +1584,7 @@ func parsePath(rf bgp.RouteFamily, args []string) (*api.Path, error) { nlri = bgp.NewOpaqueNLRI([]byte(m["key"][0]), nil) } case bgp.RF_MUP_IPv4, bgp.RF_MUP_IPv6: - nlri, extcomms, err = parseMUPArgs(args) + nlri, extcomms, err = parseMUPArgs(args, nexthop) default: return nil, fmt.Errorf("unsupported route family: %s", rf) } @@ -1774,10 +1796,20 @@ usage: %s rib %s { a-d | macadv | multicast | esi | dsd | t1st | t2st } -a mup-ipv4 - : rd [mup ] - : rd [mup ] - : rd teid qfi endpoint [mup ] - : rd teid [mup ]`, + : rd [rt ...] + : rd [rt ...] [mup ] + : rd [rt ...] teid qfi endpoint + : rd [rt ...] teid `, + err, + cmdstr, + modtype, + ) + helpErrMap[bgp.RF_MUP_IPv6] = fmt.Errorf(`error: %s +usage: %s rib %s { isd | dsd | t1st | t2st } -a mup-ipv6 + : rd [rt ...] + : rd [rt ...] [mup ] + : rd [rt ...] teid qfi endpoint + : rd [rt ...] teid `, err, cmdstr, modtype, diff --git a/pkg/packet/bgp/mup.go b/pkg/packet/bgp/mup.go index 410f8e15..d1424046 100644 --- a/pkg/packet/bgp/mup.go +++ b/pkg/packet/bgp/mup.go @@ -416,6 +416,9 @@ func (r *MUPType1SessionTransformedRoute) DecodeFromBytes(data []byte) error { } p += int(r.PrefixLength / 8) r.TEID = binary.BigEndian.Uint32(data[p : p+4]) + if r.TEID == 0 { + return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, fmt.Sprintf("Invalid TEID: %d", r.TEID)) + } p += 4 r.QFI = data[p] p += 1 @@ -539,7 +542,13 @@ func (r *MUPType2SessionTransformedRoute) DecodeFromBytes(data []byte) error { return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, fmt.Sprintf("Invalid Endpoint Address length: %d", r.EndpointAddressLength)) } r.EndpointAddress = ea - r.TEID = binary.BigEndian.Uint32(data[p : p+teidLen]) + if teidLen > 0 { + teidData := append(make([]byte, 4-teidLen), data[p:p+teidLen]...) + r.TEID = binary.BigEndian.Uint32(teidData) + if r.TEID == 0 { + return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, fmt.Sprintf("Invalid TEID: %d", r.TEID)) + } + } return nil } diff --git a/test/scenario_test/mup_test.py b/test/scenario_test/mup_test.py index e1ee8ace..25846278 100644 --- a/test/scenario_test/mup_test.py +++ b/test/scenario_test/mup_test.py @@ -18,6 +18,8 @@ from itertools import combinations import sys import time import unittest +import logging +log = logging.getLogger(__name__) import nose @@ -66,20 +68,21 @@ class GoBGPTestBase(unittest.TestCase): def test_02_add_del_mup_route(self): tests = [ - ('mup_isd_route_ipv4', 'ipv4-mup', 'isd 10.0.0.0/24 rd 100:100 mup 10:10', '0.0.0.0'), - ('mup_dsd_route_ipv4', 'ipv4-mup', 'dsd 10.0.0.1 rd 100:100 mup 10:10', '0.0.0.0'), - ('mup_t1st_route_ipv4', 'ipv4-mup', 't1st 192.168.0.1 rd 100:100 teid 12345 qfi 9 endpoint 10.0.0.1 mup 10:10', '0.0.0.0'), - ('mup_t2st_route_ipv4', 'ipv4-mup', 't2st 10.0.0.1 rd 100:100 teid 12345 mup 10:10', '0.0.0.0'), - ('mup_isd_route_ipv6', 'ipv6-mup', 'isd 2001::/64 rd 100:100 mup 10:10', '::'), - ('mup_dsd_route_ipv6', 'ipv6-mup', 'dsd 2001::1 rd 100:100 mup 10:10', '::'), - ('mup_t1st_route_ipv6', 'ipv6-mup', 't1st 2001:db8:1:1::1 rd 100:100 teid 12345 qfi 9 endpoint 2001::1 mup 10:10', '::'), - ('mup_t2st_route_ipv6', 'ipv6-mup', 't2st 2001::1 rd 100:100 teid 12345 mup 10:10', '::'), + ('mup_isd_route_ipv4', 'ipv4-mup', 'isd 10.0.0.0/24 rd 100:100 rt 10:10 nexthop 2001::2', '2001::2'), + ('mup_dsd_route_ipv4', 'ipv4-mup', 'dsd 10.0.0.1 rd 100:100 rt 10:10 mup 10:10 nexthop 2001::2', '2001::2'), + ('mup_t1st_route_ipv4', 'ipv4-mup', 't1st 192.168.0.1 rd 100:100 rt 10:10 teid 12345 qfi 9 endpoint 10.0.0.1 nexthop 10.0.0.2', '10.0.0.2'), + ('mup_t2st_route_ipv4', 'ipv4-mup', 't2st 10.0.0.1 rd 100:100 rt 10:10 teid 12345 nexthop 10.0.0.2', '10.0.0.2'), + ('mup_isd_route_ipv6', 'ipv6-mup', 'isd 2001::/64 rd 100:100 rt 10:10 nexthop 2001::2', '2001::2'), + ('mup_dsd_route_ipv6', 'ipv6-mup', 'dsd 2001::1 rd 100:100 rt 10:10 mup 10:10 nexthop 2001::2', '2001::2'), + ('mup_t1st_route_ipv6', 'ipv6-mup', 't1st 2001:db8:1:1::1 rd 100:100 rt 10:10 teid 12345 qfi 9 endpoint 2001::1 nexthop 10.0.0.2', '10.0.0.2'), + ('mup_t2st_route_ipv6', 'ipv6-mup', 't2st 2001::1 rd 100:100 rt 10:10 teid 12345 nexthop 10.0.0.2', '10.0.0.2'), ] for msg, rf, route, nh in tests: with self.subTest(msg): self.g1.local('gobgp global rib add ' '-a {} {}'.format(rf, route)) grib = self.g1.get_global_rib(rf=rf) + log.debug('grib: {}'.format(grib)) self.assertEqual(len(grib), 1) dst = grib[0] self.assertEqual(len(dst['paths']), 1) @@ -103,7 +106,7 @@ class GoBGPTestBase(unittest.TestCase): self.assertEqual(len(dst['paths']), 1) path = dst['paths'][0] n_addrs = [i[1].split('/')[0] for i in self.g1.ip_addrs] - self.assertTrue(path['nexthop'] in n_addrs) + self.assertEqual(path['nexthop'], nh) done = True self.g1.local('gobgp global rib del '