diff options
35 files changed, 394 insertions, 634 deletions
diff --git a/api/grpc_server.go b/api/grpc_server.go index 43402d22..3fa65ee5 100644 --- a/api/grpc_server.go +++ b/api/grpc_server.go @@ -687,31 +687,30 @@ func (s *Server) MonitorRib(arg *MonitorRibRequest, stream GobgpApi_MonitorRibSe } return nil } - for { - select { - case ev := <-w.Event(): - switch msg := ev.(type) { - case *server.WatchEventBestPath: - if err := sendPath(func() []*table.Path { - if len(msg.MultiPathList) > 0 { - l := make([]*table.Path, 0) - for _, p := range msg.MultiPathList { - l = append(l, p...) - } - return l - } else { - return msg.PathList + + for ev := range w.Event() { + switch msg := ev.(type) { + case *server.WatchEventBestPath: + if err := sendPath(func() []*table.Path { + if len(msg.MultiPathList) > 0 { + l := make([]*table.Path, 0) + for _, p := range msg.MultiPathList { + l = append(l, p...) } - }()); err != nil { - return err - } - case *server.WatchEventUpdate: - if err := sendPath(msg.PathList); err != nil { - return err + return l + } else { + return msg.PathList } + }()); err != nil { + return err + } + case *server.WatchEventUpdate: + if err := sendPath(msg.PathList); err != nil { + return err } } } + return nil }() } @@ -723,40 +722,38 @@ func (s *Server) MonitorPeerState(arg *Arguments, stream GobgpApi_MonitorPeerSta w := s.bgpServer.Watch(server.WatchPeerState(arg.Current)) defer func() { w.Stop() }() - for { - select { - case ev := <-w.Event(): - switch msg := ev.(type) { - case *server.WatchEventPeerState: - if len(arg.Name) > 0 && arg.Name != msg.PeerAddress.String() && arg.Name != msg.PeerInterface { - continue - } - if err := stream.Send(&Peer{ - Conf: &PeerConf{ - PeerAs: msg.PeerAS, - LocalAs: msg.LocalAS, - NeighborAddress: msg.PeerAddress.String(), - Id: msg.PeerID.String(), - NeighborInterface: msg.PeerInterface, - }, - Info: &PeerState{ - PeerAs: msg.PeerAS, - LocalAs: msg.LocalAS, - NeighborAddress: msg.PeerAddress.String(), - BgpState: msg.State.String(), - AdminState: PeerState_AdminState(msg.AdminState), - }, - Transport: &Transport{ - LocalAddress: msg.LocalAddress.String(), - LocalPort: uint32(msg.LocalPort), - RemotePort: uint32(msg.PeerPort), - }, - }); err != nil { - return err - } + for ev := range w.Event() { + switch msg := ev.(type) { + case *server.WatchEventPeerState: + if len(arg.Name) > 0 && arg.Name != msg.PeerAddress.String() && arg.Name != msg.PeerInterface { + continue + } + if err := stream.Send(&Peer{ + Conf: &PeerConf{ + PeerAs: msg.PeerAS, + LocalAs: msg.LocalAS, + NeighborAddress: msg.PeerAddress.String(), + Id: msg.PeerID.String(), + NeighborInterface: msg.PeerInterface, + }, + Info: &PeerState{ + PeerAs: msg.PeerAS, + LocalAs: msg.LocalAS, + NeighborAddress: msg.PeerAddress.String(), + BgpState: msg.State.String(), + AdminState: PeerState_AdminState(msg.AdminState), + }, + Transport: &Transport{ + LocalAddress: msg.LocalAddress.String(), + LocalPort: uint32(msg.LocalPort), + RemotePort: uint32(msg.PeerPort), + }, + }); err != nil { + return err } } } + return nil }() } @@ -899,7 +896,7 @@ func (s *Server) api2PathList(resource Resource, ApiPathList []*Path) ([]*table. } newPath := table.NewPath(pi, nlri, path.IsWithdraw, pattrs, time.Now(), path.NoImplicitWithdraw) - if path.IsWithdraw == false { + if !path.IsWithdraw { total := bytes.NewBuffer(make([]byte, 0)) for _, a := range newPath.GetPathAttrs() { if a.GetType() == bgp.BGP_ATTR_TYPE_MP_REACH_NLRI { @@ -1660,33 +1657,23 @@ func NewAPIDefinedSetFromTableStruct(t table.DefinedSet) (*DefinedSet, error) { case table.DEFINED_TYPE_NEIGHBOR: s := t.(*table.NeighborSet) c := s.ToConfig() - for _, n := range c.NeighborInfoList { - a.List = append(a.List, n) - } + a.List = append(a.List, c.NeighborInfoList...) case table.DEFINED_TYPE_AS_PATH: s := t.(*table.AsPathSet) c := s.ToConfig() - for _, n := range c.AsPathList { - a.List = append(a.List, n) - } + a.List = append(a.List, c.AsPathList...) case table.DEFINED_TYPE_COMMUNITY: s := t.(*table.CommunitySet) c := s.ToConfig() - for _, n := range c.CommunityList { - a.List = append(a.List, n) - } + a.List = append(a.List, c.CommunityList...) case table.DEFINED_TYPE_EXT_COMMUNITY: s := t.(*table.ExtCommunitySet) c := s.ToConfig() - for _, n := range c.ExtCommunityList { - a.List = append(a.List, n) - } + a.List = append(a.List, c.ExtCommunityList...) case table.DEFINED_TYPE_LARGE_COMMUNITY: s := t.(*table.LargeCommunitySet) c := s.ToConfig() - for _, n := range c.LargeCommunityList { - a.List = append(a.List, n) - } + a.List = append(a.List, c.LargeCommunityList...) default: return nil, fmt.Errorf("invalid defined type") } @@ -1874,6 +1861,8 @@ func NewDefinedSetFromApiStruct(a *DefinedSet) (table.DefinedSet, error) { } } +var _regexpPrefixMaskLengthRange = regexp.MustCompile(`(\d+)\.\.(\d+)`) + func (s *Server) GetDefinedSet(ctx context.Context, arg *GetDefinedSetRequest) (*GetDefinedSetResponse, error) { cd, err := s.bgpServer.GetDefinedSet(table.DefinedType(arg.Type), arg.Name) if err != nil { @@ -1887,8 +1876,7 @@ func (s *Server) GetDefinedSet(ctx context.Context, arg *GetDefinedSetRequest) ( Prefixes: func() []*Prefix { l := make([]*Prefix, 0, len(cs.PrefixList)) for _, p := range cs.PrefixList { - exp := regexp.MustCompile("(\\d+)\\.\\.(\\d+)") - elems := exp.FindStringSubmatch(p.MasklengthRange) + elems := _regexpPrefixMaskLengthRange.FindStringSubmatch(p.MasklengthRange) min, _ := strconv.ParseUint(elems[1], 10, 32) max, _ := strconv.ParseUint(elems[2], 10, 32) @@ -1981,6 +1969,8 @@ func NewAPIStatementFromTableStruct(t *table.Statement) *Statement { return toStatementApi(t.ToConfig()) } +var _regexpMedActionType = regexp.MustCompile(`([+-]?)(\d+)`) + func toStatementApi(s *config.Statement) *Statement { cs := &Conditions{} if s.Conditions.MatchPrefixSet.PrefixSet != "" { @@ -2066,8 +2056,7 @@ func toStatementApi(s *config.Statement) *Statement { if len(medStr) == 0 { return nil } - re := regexp.MustCompile("([+-]?)(\\d+)") - matches := re.FindStringSubmatch(medStr) + matches := _regexpMedActionType.FindStringSubmatch(medStr) if len(matches) == 0 { return nil } diff --git a/config/default.go b/config/default.go index 1576bb33..cbb11167 100644 --- a/config/default.go +++ b/config/default.go @@ -30,7 +30,7 @@ var configuredFields map[string]interface{} func RegisterConfiguredFields(addr string, n interface{}) { if configuredFields == nil { - configuredFields = make(map[string]interface{}, 0) + configuredFields = make(map[string]interface{}) } configuredFields[addr] = n } diff --git a/config/serve.go b/config/serve.go index 8b2f1cfc..6b09af6e 100644 --- a/config/serve.go +++ b/config/serve.go @@ -68,12 +68,10 @@ func ReadConfigfileServe(path, format string, configCh chan *BgpConfigSet) { }).Warningf("Can't read config file %s", path) } NEXT: - select { - case <-sigCh: - log.WithFields(log.Fields{ - "Topic": "Config", - }).Info("Reload the config file") - } + <-sigCh + log.WithFields(log.Fields{ + "Topic": "Config", + }).Info("Reload the config file") } } diff --git a/config/util.go b/config/util.go index 7cd8c01e..e857b98b 100644 --- a/config/util.go +++ b/config/util.go @@ -81,23 +81,6 @@ func getIPv6LinkLocalAddress(ifname string) (string, error) { return "", fmt.Errorf("no ipv6 link local address for %s", ifname) } -func isLocalLinkLocalAddress(ifindex int, addr net.IP) (bool, error) { - ifi, err := net.InterfaceByIndex(ifindex) - if err != nil { - return false, err - } - addrs, err := ifi.Addrs() - if err != nil { - return false, err - } - for _, a := range addrs { - if ip, _, _ := net.ParseCIDR(a.String()); addr.Equal(ip) { - return true, nil - } - } - return false, nil -} - func (b *BgpConfigSet) getPeerGroup(n string) (*PeerGroup, error) { if n == "" { return nil, nil @@ -240,6 +223,9 @@ func (n *Neighbor) NeedsResendOpenMessage(new *Neighbor) bool { isAfiSafiChanged(n.AfiSafis, new.AfiSafis) } +// TODO: these regexp are duplicated in api +var _regexpPrefixMaskLengthRange = regexp.MustCompile(`(\d+)\.\.(\d+)`) + func ParseMaskLength(prefix, mask string) (int, int, error) { _, ipNet, err := net.ParseCIDR(prefix) if err != nil { @@ -249,8 +235,7 @@ func ParseMaskLength(prefix, mask string) (int, int, error) { l, _ := ipNet.Mask.Size() return l, l, nil } - exp := regexp.MustCompile("(\\d+)\\.\\.(\\d+)") - elems := exp.FindStringSubmatch(mask) + elems := _regexpPrefixMaskLengthRange.FindStringSubmatch(mask) if len(elems) != 3 { return 0, 0, fmt.Errorf("invalid mask length range: %s", mask) } diff --git a/gobgp/cmd/common.go b/gobgp/cmd/common.go index c2cfeaa7..7add79a1 100644 --- a/gobgp/cmd/common.go +++ b/gobgp/cmd/common.go @@ -97,23 +97,6 @@ var neighborsOpts struct { Transport string `short:"t" long:"transport" description:"specifying a transport protocol"` } -var conditionOpts struct { - Prefix string `long:"prefix" description:"specifying a prefix set name of policy"` - Neighbor string `long:"neighbor" description:"specifying a neighbor set name of policy"` - AsPath string `long:"aspath" description:"specifying an as set name of policy"` - Community string `long:"community" description:"specifying a community set name of policy"` - ExtCommunity string `long:"extcommunity" description:"specifying a extended community set name of policy"` - AsPathLength string `long:"aspath-len" description:"specifying an as path length of policy (<operator>,<numeric>)"` -} - -var actionOpts struct { - RouteAction string `long:"route-action" description:"specifying a route action of policy (accept | reject)"` - CommunityAction string `long:"community" description:"specifying a community action of policy"` - MedAction string `long:"med" description:"specifying a med action of policy"` - AsPathPrependAction string `long:"as-prepend" description:"specifying a as-prepend action of policy"` - NexthopAction string `long:"next-hop" description:"specifying a next-hop action of policy"` -} - var mrtOpts struct { OutputDir string FileFormat string @@ -216,10 +199,7 @@ func (n neighbors) Less(i, j int) bool { p1Isv4 := !strings.Contains(p1, ":") p2Isv4 := !strings.Contains(p2, ":") if p1Isv4 != p2Isv4 { - if p1Isv4 { - return true - } - return false + return p1Isv4 } addrlen := 128 if p1Isv4 { diff --git a/gobgp/cmd/global.go b/gobgp/cmd/global.go index 26bd3d27..91c71ff5 100644 --- a/gobgp/cmd/global.go +++ b/gobgp/cmd/global.go @@ -877,18 +877,21 @@ func toAs4Value(s string) (uint32, error) { return uint32(i), nil } +var ( + _regexpASPathGroups = regexp.MustCompile("[{}]") + _regexpASPathSegment = regexp.MustCompile(`,|\s+`) +) + func newAsPath(aspath string) (bgp.PathAttributeInterface, error) { // For the first step, parses "aspath" into a list of uint32 list. // e.g.) "10 20 {30,40} 50" -> [][]uint32{{10, 20}, {30, 40}, {50}} - exp := regexp.MustCompile("[{}]") - segments := exp.Split(aspath, -1) + segments := _regexpASPathGroups.Split(aspath, -1) asPathPrams := make([]bgp.AsPathParamInterface, 0, len(segments)) - exp = regexp.MustCompile(",|\\s+") for idx, segment := range segments { if segment == "" { continue } - nums := exp.Split(segment, -1) + nums := _regexpASPathSegment.Split(segment, -1) asNums := make([]uint32, 0, len(nums)) for _, n := range nums { if n == "" { @@ -1071,22 +1074,6 @@ func extractAggregator(args []string) ([]string, bgp.PathAttributeInterface, err return args, nil, nil } -func extractRouteDistinguisher(args []string) ([]string, bgp.RouteDistinguisherInterface, error) { - for idx, arg := range args { - if arg == "rd" { - if len(args) < (idx + 1) { - return nil, nil, fmt.Errorf("invalid rd format") - } - rd, err := bgp.ParseRouteDistinguisher(args[idx+1]) - if err != nil { - return nil, nil, err - } - return append(args[:idx], args[idx+2:]...), rd, nil - } - } - return args, nil, nil -} - func ParsePath(rf bgp.RouteFamily, args []string) (*table.Path, error) { var nlri bgp.AddrPrefixInterface var extcomms []string @@ -1250,7 +1237,7 @@ func ParsePath(rf bgp.RouteFamily, args []string) (*table.Path, error) { attrs = append(attrs, mpreach) } - if extcomms != nil && len(extcomms) > 0 { + if extcomms != nil { extcomms, err := ParseExtendedCommunities(extcomms) if err != nil { return nil, err diff --git a/gobgp/cmd/neighbor.go b/gobgp/cmd/neighbor.go index 3769ca05..2e30fc8f 100644 --- a/gobgp/cmd/neighbor.go +++ b/gobgp/cmd/neighbor.go @@ -129,11 +129,11 @@ func showNeighbors(vrf string) error { } timedelta = append(timedelta, timeStr) } - var format string - format = "%-" + fmt.Sprint(maxaddrlen) + "s" + " %" + fmt.Sprint(maxaslen) + "s" + " %" + fmt.Sprint(maxtimelen) + "s" + + format := "%-" + fmt.Sprint(maxaddrlen) + "s" + " %" + fmt.Sprint(maxaslen) + "s" + " %" + fmt.Sprint(maxtimelen) + "s" format += " %-11s |%9s %9s\n" fmt.Printf(format, "Peer", "AS", "Up/Down", "State", "#Received", "Accepted") - format_fsm := func(admin config.AdminState, fsm config.SessionState) string { + formatFsm := func(admin config.AdminState, fsm config.SessionState) string { switch admin { case config.ADMIN_STATE_DOWN: return "Idle(Admin)" @@ -164,7 +164,7 @@ func showNeighbors(vrf string) error { if n.Config.NeighborInterface != "" { neigh = n.Config.NeighborInterface } - fmt.Printf(format, neigh, getASN(n), timedelta[i], format_fsm(n.State.AdminState, n.State.SessionState), fmt.Sprint(n.State.AdjTable.Received), fmt.Sprint(n.State.AdjTable.Accepted)) + fmt.Printf(format, neigh, getASN(n), timedelta[i], formatFsm(n.State.AdminState, n.State.SessionState), fmt.Sprint(n.State.AdjTable.Received), fmt.Sprint(n.State.AdjTable.Accepted)) } return nil @@ -422,11 +422,7 @@ func showNeighbor(args []string) error { return nil } -type AsPathFormat struct { - start string - end string - separator string -} +type AsPathFormat struct{} func getPathSymbolString(p *table.Path, idx int, showBest bool) string { symbols := "" @@ -825,9 +821,7 @@ func showNeighborRib(r string, name string, args []string) error { var ps []*table.Path switch r { case CMD_ACCEPTED: - for _, p := range d.GetAllKnownPathList() { - ps = append(ps, p) - } + ps = append(ps, d.GetAllKnownPathList()...) case CMD_REJECTED: // always nothing default: diff --git a/gobgp/cmd/policy.go b/gobgp/cmd/policy.go index 7c8a5669..7e9c08b2 100644 --- a/gobgp/cmd/policy.go +++ b/gobgp/cmd/policy.go @@ -31,6 +31,8 @@ import ( "github.com/osrg/gobgp/table" ) +var _regexpCommunity = regexp.MustCompile(`\^\^(\S+)\$\$`) + func formatDefinedSet(head bool, typ string, indent int, list []table.DefinedSet) string { if len(list) == 0 { return "Nothing defined yet\n" @@ -59,8 +61,7 @@ func formatDefinedSet(head bool, typ string, indent int, list []table.DefinedSet } for i, x := range l { if typ == "COMMUNITY" || typ == "EXT-COMMUNITY" || typ == "LARGE-COMMUNITY" { - exp := regexp.MustCompile("\\^\\^(\\S+)\\$\\$") - x = exp.ReplaceAllString(x, "$1") + x = _regexpCommunity.ReplaceAllString(x, "$1") } if i == 0 { buff.WriteString(fmt.Sprintf(format, s.Name(), x)) @@ -778,6 +779,9 @@ func modAction(name, op string, args []string) error { stmt.Actions.BgpActions.SetNextHop = config.BgpNextHopType(args[0]) } t, err := table.NewStatement(stmt) + if err != nil { + return err + } switch op { case CMD_ADD: err = client.AddStatement(t) diff --git a/gobgp/cmd/root.go b/gobgp/cmd/root.go index c9281b8c..cfa84661 100644 --- a/gobgp/cmd/root.go +++ b/gobgp/cmd/root.go @@ -37,7 +37,6 @@ var globalOpts struct { CaFile string } -var cmds []string var client *cli.Client func NewRootCmd() *cobra.Command { diff --git a/gobgp/cmd/rpki.go b/gobgp/cmd/rpki.go index 567c4b1b..a0423f58 100644 --- a/gobgp/cmd/rpki.go +++ b/gobgp/cmd/rpki.go @@ -36,36 +36,37 @@ func showRPKIServer(args []string) error { for _, r := range servers { s := "Down" uptime := "never" - if r.State.Up == true { + if r.State.Up { s = "Up" - uptime = fmt.Sprint(formatTimedelta(int64(time.Now().Sub(time.Unix(r.State.Uptime, 0)).Seconds()))) + uptime = fmt.Sprint(formatTimedelta(int64(time.Since(time.Unix(r.State.Uptime, 0)).Seconds()))) } fmt.Printf(format, net.JoinHostPort(r.Config.Address, fmt.Sprintf("%d", r.Config.Port)), s, uptime, fmt.Sprintf("%d/%d", r.State.RecordsV4, r.State.RecordsV6)) } - } else { - for _, r := range servers { - if r.Config.Address == args[0] { - up := "Down" - if r.State.Up == true { - up = "Up" - } - fmt.Printf("Session: %s, State: %s\n", r.Config.Address, up) - fmt.Println(" Port:", r.Config.Port) - fmt.Println(" Serial:", r.State.SerialNumber) - fmt.Printf(" Prefix: %d/%d\n", r.State.PrefixesV4, r.State.PrefixesV6) - fmt.Printf(" Record: %d/%d\n", r.State.RecordsV4, r.State.RecordsV6) - fmt.Println(" Message statistics:") - fmt.Printf(" Receivedv4: %10d\n", r.State.RpkiMessages.RpkiReceived.Ipv4Prefix) - fmt.Printf(" Receivedv6: %10d\n", r.State.RpkiMessages.RpkiReceived.Ipv4Prefix) - fmt.Printf(" SerialNotify: %10d\n", r.State.RpkiMessages.RpkiReceived.SerialNotify) - fmt.Printf(" CacheReset: %10d\n", r.State.RpkiMessages.RpkiReceived.CacheReset) - fmt.Printf(" CacheResponse: %10d\n", r.State.RpkiMessages.RpkiReceived.CacheResponse) - fmt.Printf(" EndOfData: %10d\n", r.State.RpkiMessages.RpkiReceived.EndOfData) - fmt.Printf(" Error: %10d\n", r.State.RpkiMessages.RpkiReceived.Error) - fmt.Printf(" SerialQuery: %10d\n", r.State.RpkiMessages.RpkiSent.SerialQuery) - fmt.Printf(" ResetQuery: %10d\n", r.State.RpkiMessages.RpkiSent.ResetQuery) + return nil + } + + for _, r := range servers { + if r.Config.Address == args[0] { + up := "Down" + if r.State.Up { + up = "Up" } + fmt.Printf("Session: %s, State: %s\n", r.Config.Address, up) + fmt.Println(" Port:", r.Config.Port) + fmt.Println(" Serial:", r.State.SerialNumber) + fmt.Printf(" Prefix: %d/%d\n", r.State.PrefixesV4, r.State.PrefixesV6) + fmt.Printf(" Record: %d/%d\n", r.State.RecordsV4, r.State.RecordsV6) + fmt.Println(" Message statistics:") + fmt.Printf(" Receivedv4: %10d\n", r.State.RpkiMessages.RpkiReceived.Ipv4Prefix) + fmt.Printf(" Receivedv6: %10d\n", r.State.RpkiMessages.RpkiReceived.Ipv4Prefix) + fmt.Printf(" SerialNotify: %10d\n", r.State.RpkiMessages.RpkiReceived.SerialNotify) + fmt.Printf(" CacheReset: %10d\n", r.State.RpkiMessages.RpkiReceived.CacheReset) + fmt.Printf(" CacheResponse: %10d\n", r.State.RpkiMessages.RpkiReceived.CacheResponse) + fmt.Printf(" EndOfData: %10d\n", r.State.RpkiMessages.RpkiReceived.EndOfData) + fmt.Printf(" Error: %10d\n", r.State.RpkiMessages.RpkiReceived.Error) + fmt.Printf(" SerialQuery: %10d\n", r.State.RpkiMessages.RpkiSent.SerialQuery) + fmt.Printf(" ResetQuery: %10d\n", r.State.RpkiMessages.RpkiSent.ResetQuery) } } return nil diff --git a/gobgp/cmd/vrf.go b/gobgp/cmd/vrf.go index 188cec44..4fe64802 100644 --- a/gobgp/cmd/vrf.go +++ b/gobgp/cmd/vrf.go @@ -154,7 +154,9 @@ func modVrf(typ string, args []string) error { return err } } - err = client.AddVRF(name, int(id), rd, importRt, exportRt) + if err := client.AddVRF(name, int(id), rd, importRt, exportRt); err != nil { + return err + } case CMD_DEL: if len(args) != 1 { return fmt.Errorf("Usage: gobgp vrf del <vrf name>") diff --git a/gobgpd/main.go b/gobgpd/main.go index 2fc89424..13daa9d7 100644 --- a/gobgpd/main.go +++ b/gobgpd/main.go @@ -113,7 +113,7 @@ func main() { log.SetLevel(log.InfoLevel) } - if opts.DisableStdlog == true { + if opts.DisableStdlog { log.SetOutput(ioutil.Discard) } else { log.SetOutput(os.Stdout) diff --git a/packet/bgp/bgp.go b/packet/bgp/bgp.go index 361da392..b882f9a6 100644 --- a/packet/bgp/bgp.go +++ b/packet/bgp/bgp.go @@ -289,13 +289,13 @@ func (c BGPCapabilityCode) String() string { var ( // Used parsing RouteDistinguisher - _regexpRouteDistinguisher = regexp.MustCompile("^((\\d+)\\.(\\d+)\\.(\\d+)\\.(\\d+)|((\\d+)\\.)?(\\d+)|([\\w]+:[\\w:]*:[\\w]+)):(\\d+)$") + _regexpRouteDistinguisher = regexp.MustCompile(`^((\d+)\.(\d+)\.(\d+)\.(\d+)|((\d+)\.)?(\d+)|([\w]+:[\w:]*:[\w]+)):(\d+)$`) // Used for operator and value for the FlowSpec numeric type // Example: // re.FindStringSubmatch("&==80") // >>> ["&==80" "&" "==" "80"] - _regexpFlowSpecNumericType = regexp.MustCompile("(&?)(==|=|>|>=|<|<=|!|!=|=!)?(\\d+|-\\d|true|false)") + _regexpFlowSpecNumericType = regexp.MustCompile(`(&?)(==|=|>|>=|<|<=|!|!=|=!)?(\d+|-\d|true|false)`) // - "=!" is used in the old style format of "tcp-flags" and "fragment". // - The value field should be one of the followings: @@ -303,8 +303,8 @@ var ( // * Combination of the small letters, decimals, "-" and "+" // (e.g., tcp, ipv4, is-fragment+first-fragment) // * Capital letters (e.g., SA) - _regexpFlowSpecOperator = regexp.MustCompile("&|=|>|<|!|[\\w\\-+]+") - _regexpFlowSpecOperatorValue = regexp.MustCompile("[\\w\\-+]+") + _regexpFlowSpecOperator = regexp.MustCompile(`&|=|>|<|!|[\w\-+]+`) + _regexpFlowSpecOperatorValue = regexp.MustCompile(`[\w\-+]+`) // Note: "(-*)" and "(.*)" catch the invalid flags // Example: In this case, "Z" is unsupported flag type. @@ -315,13 +315,13 @@ var ( // Note: "(.*)" catches the invalid flags // re.FindStringSubmatch("&!=+first-fragment+last-fragment+invalid-fragment") // >>> ["&!=+first-fragment+last-fragment+invalid-fragment" "&" "!=" "+first-fragment+last-fragment" "+last-fragment" "+" "last" "+invalid-fragment"] - _regexpFlowSpecFragment = regexp.MustCompile("(&?)(==|=|!|!=|=!)?(((\\+)?(dont|is|first|last|not-a)-fragment)+)(.*)") + _regexpFlowSpecFragment = regexp.MustCompile(`(&?)(==|=|!|!=|=!)?(((\+)?(dont|is|first|last|not-a)-fragment)+)(.*)`) // re.FindStringSubmatch("192.168.0.0/24") // >>> ["192.168.0.0/24" "192.168.0.0" "/24" "24"] // re.FindStringSubmatch("192.168.0.1") // >>> ["192.168.0.1" "192.168.0.1" "" ""] - _regexpFindIPv4Prefix = regexp.MustCompile("^([\\d.]+)(/(\\d{1,2}))?") + _regexpFindIPv4Prefix = regexp.MustCompile(`^([\d.]+)(/(\d{1,2}))?`) // re.FindStringSubmatch("2001:dB8::/64") // >>> ["2001:dB8::/64" "2001:dB8::" "/64" "64" "" ""] @@ -329,7 +329,7 @@ var ( // >>> ["2001:dB8::/64/8" "2001:dB8::" "/64" "64" "/8" "8"] // re.FindStringSubmatch("2001:dB8::1") // >>> ["2001:dB8::1" "2001:dB8::1" "" "" "" ""] - _regexpFindIPv6Prefix = regexp.MustCompile("^([a-fA-F\\d:.]+)(/(\\d{1,3}))?(/(\\d{1,3}))?") + _regexpFindIPv6Prefix = regexp.MustCompile(`^([a-fA-F\d:.]+)(/(\d{1,3}))?(/(\d{1,3}))?`) ) type ParameterCapabilityInterface interface { @@ -1572,7 +1572,8 @@ func (l *MPLSLabelStack) DecodeFromBytes(data []byte) error { break } } - if foundBottom == false { + + if foundBottom { l.Labels = []uint32{} return nil } @@ -2161,7 +2162,7 @@ func ParseEthernetSegmentIdentifier(args []string) (EthernetSegmentIdentifier, e } invalidEsiValuesError := fmt.Errorf("invalid esi values for type %s: %s", esi.Type.String(), args[1:]) - esi.Value = make([]byte, 9, 9) + esi.Value = make([]byte, 9) switch esi.Type { case ESI_LACP: fallthrough @@ -2196,7 +2197,7 @@ func ParseEthernetSegmentIdentifier(args []string) (EthernetSegmentIdentifier, e if err != nil { return esi, invalidEsiValuesError } - iBuf := make([]byte, 4, 4) + iBuf := make([]byte, 4) binary.BigEndian.PutUint32(iBuf, uint32(i)) copy(esi.Value[6:9], iBuf[1:4]) case ESI_ROUTERID: @@ -2278,7 +2279,7 @@ func labelSerialize(label uint32) ([]byte, error) { if label > 0xffffff { return nil, NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, fmt.Sprintf("Out of range Label: %d", label)) } - buf := make([]byte, 3, 3) + buf := make([]byte, 3) buf[0] = byte((label >> 16) & 0xff) buf[1] = byte((label >> 8) & 0xff) buf[2] = byte(label & 0xff) @@ -3951,13 +3952,10 @@ func (v *FlowSpecComponentItem) Len() int { } func (v *FlowSpecComponentItem) Serialize() ([]byte, error) { - if v.Value < 0 { - return nil, fmt.Errorf("invalid value size(too small): %d", v.Value) - } - if v.Op < 0 || v.Op > math.MaxUint8 { + if v.Op > math.MaxUint8 { return nil, fmt.Errorf("invalid op size: %d", v.Op) - } + order := uint32(math.Log2(float64(v.Len()))) buf := make([]byte, 1+(1<<order)) buf[0] = byte(uint32(v.Op) | order<<4) @@ -5488,7 +5486,7 @@ func (p *PathAttributeAsPath) DecodeFromBytes(data []byte, options ...*Marshalli } for len(value) > 0 { var tuple AsPathParamInterface - if isAs4 == true { + if isAs4 { tuple = &As4PathParam{} } else { tuple = &AsPathParam{} @@ -5498,9 +5496,6 @@ func (p *PathAttributeAsPath) DecodeFromBytes(data []byte, options ...*Marshalli return err } p.Value = append(p.Value, tuple) - if tuple.Len() > len(value) { - - } value = value[tuple.Len():] } return nil @@ -5877,19 +5872,19 @@ type WellKnownCommunity uint32 const ( COMMUNITY_INTERNET WellKnownCommunity = 0x00000000 - COMMUNITY_PLANNED_SHUT = 0xffff0000 - COMMUNITY_ACCEPT_OWN = 0xffff0001 - COMMUNITY_ROUTE_FILTER_TRANSLATED_v4 = 0xffff0002 - COMMUNITY_ROUTE_FILTER_v4 = 0xffff0003 - COMMUNITY_ROUTE_FILTER_TRANSLATED_v6 = 0xffff0004 - COMMUNITY_ROUTE_FILTER_v6 = 0xffff0005 - COMMUNITY_LLGR_STALE = 0xffff0006 - COMMUNITY_NO_LLGR = 0xffff0007 - COMMUNITY_BLACKHOLE = 0xffff029a - COMMUNITY_NO_EXPORT = 0xffffff01 - COMMUNITY_NO_ADVERTISE = 0xffffff02 - COMMUNITY_NO_EXPORT_SUBCONFED = 0xffffff03 - COMMUNITY_NO_PEER = 0xffffff04 + COMMUNITY_PLANNED_SHUT WellKnownCommunity = 0xffff0000 + COMMUNITY_ACCEPT_OWN WellKnownCommunity = 0xffff0001 + COMMUNITY_ROUTE_FILTER_TRANSLATED_v4 WellKnownCommunity = 0xffff0002 + COMMUNITY_ROUTE_FILTER_v4 WellKnownCommunity = 0xffff0003 + COMMUNITY_ROUTE_FILTER_TRANSLATED_v6 WellKnownCommunity = 0xffff0004 + COMMUNITY_ROUTE_FILTER_v6 WellKnownCommunity = 0xffff0005 + COMMUNITY_LLGR_STALE WellKnownCommunity = 0xffff0006 + COMMUNITY_NO_LLGR WellKnownCommunity = 0xffff0007 + COMMUNITY_BLACKHOLE WellKnownCommunity = 0xffff029a + COMMUNITY_NO_EXPORT WellKnownCommunity = 0xffffff01 + COMMUNITY_NO_ADVERTISE WellKnownCommunity = 0xffffff02 + COMMUNITY_NO_EXPORT_SUBCONFED WellKnownCommunity = 0xffffff03 + COMMUNITY_NO_PEER WellKnownCommunity = 0xffffff04 ) var WellKnownCommunityNameMap = map[WellKnownCommunity]string{ @@ -5996,7 +5991,7 @@ func (p *PathAttributeOriginatorId) MarshalJSON() ([]byte, error) { } func (p *PathAttributeOriginatorId) Serialize(options ...*MarshallingOption) ([]byte, error) { - buf := make([]byte, 4, 4) + buf := make([]byte, 4) copy(buf, p.Value) return p.PathAttribute.Serialize(buf, options...) } @@ -6710,7 +6705,7 @@ type ValidationExtended struct { } func (e *ValidationExtended) Serialize() ([]byte, error) { - buf := make([]byte, 8, 8) + buf := make([]byte, 8) typ, subType := e.GetTypes() buf[0] = byte(typ) buf[1] = byte(subType) @@ -6750,7 +6745,7 @@ type ColorExtended struct { } func (e *ColorExtended) Serialize() ([]byte, error) { - buf := make([]byte, 8, 8) + buf := make([]byte, 8) typ, subType := e.GetTypes() buf[0] = byte(typ) buf[1] = byte(subType) @@ -6790,7 +6785,7 @@ type EncapExtended struct { } func (e *EncapExtended) Serialize() ([]byte, error) { - buf := make([]byte, 8, 8) + buf := make([]byte, 8) typ, subType := e.GetTypes() buf[0] = byte(typ) buf[1] = byte(subType) @@ -6850,7 +6845,7 @@ type DefaultGatewayExtended struct { } func (e *DefaultGatewayExtended) Serialize() ([]byte, error) { - buf := make([]byte, 8, 8) + buf := make([]byte, 8) typ, subType := e.GetTypes() buf[0] = byte(typ) buf[1] = byte(subType) @@ -6889,7 +6884,7 @@ func (e *OpaqueExtended) Serialize() ([]byte, error) { if len(e.Value) != 7 { return nil, fmt.Errorf("invalid value length for opaque extended community: %d", len(e.Value)) } - buf := make([]byte, 8, 8) + buf := make([]byte, 8) if e.IsTransitive { buf[0] = byte(EC_TYPE_TRANSITIVE_OPAQUE) } else { @@ -6900,7 +6895,7 @@ func (e *OpaqueExtended) Serialize() ([]byte, error) { } func (e *OpaqueExtended) String() string { - buf := make([]byte, 8, 8) + buf := make([]byte, 8) copy(buf[1:], e.Value) return fmt.Sprintf("%d", binary.BigEndian.Uint64(buf)) } @@ -6931,7 +6926,7 @@ func (e *OpaqueExtended) MarshalJSON() ([]byte, error) { } func NewOpaqueExtended(isTransitive bool, value []byte) *OpaqueExtended { - v := make([]byte, 7, 7) + v := make([]byte, 7) copy(v, value) return &OpaqueExtended{ IsTransitive: isTransitive, @@ -7552,7 +7547,7 @@ func (e *UnknownExtended) Serialize() ([]byte, error) { if len(e.Value) != 7 { return nil, fmt.Errorf("invalid value length for unknown extended community: %d", len(e.Value)) } - buf := make([]byte, 8, 8) + buf := make([]byte, 8) buf[0] = uint8(e.Type) copy(buf[1:], e.Value) return buf, nil @@ -7587,7 +7582,7 @@ func (e *UnknownExtended) GetTypes() (ExtendedCommunityAttrType, ExtendedCommuni } func NewUnknownExtended(typ ExtendedCommunityAttrType, value []byte) *UnknownExtended { - v := make([]byte, 7, 7) + v := make([]byte, 7) copy(v, value) return &UnknownExtended{ Type: typ, @@ -7731,9 +7726,11 @@ func (p *PathAttributeAs4Path) DecodeFromBytes(data []byte, options ...*Marshall if err != nil { return err } - if isAs4 == false { + + if !isAs4 { return NewMessageError(eCode, eSubCode, nil, "AS4 PATH param is malformed") } + for len(value) > 0 { tuple := &As4PathParam{} tuple.DecodeFromBytes(value) @@ -8149,7 +8146,7 @@ func (p *TunnelEncapTLV) Serialize() ([]byte, error) { } func (p *TunnelEncapTLV) String() string { - tlvList := make([]string, len(p.Value), len(p.Value)) + tlvList := make([]string, len(p.Value)) for i, v := range p.Value { tlvList[i] = v.String() } @@ -8208,7 +8205,7 @@ func (p *PathAttributeTunnelEncap) Serialize(options ...*MarshallingOption) ([]b } func (p *PathAttributeTunnelEncap) String() string { - tlvList := make([]string, len(p.Value), len(p.Value)) + tlvList := make([]string, len(p.Value)) for i, v := range p.Value { tlvList[i] = v.String() } diff --git a/packet/bgp/bgp_test.go b/packet/bgp/bgp_test.go index 1d6f386f..555369db 100644 --- a/packet/bgp/bgp_test.go +++ b/packet/bgp/bgp_test.go @@ -19,11 +19,11 @@ import ( "bytes" "encoding/binary" "net" - "reflect" "strconv" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func keepalive() *BGPMessage { @@ -50,23 +50,18 @@ func BenchmarkNormalizeFlowSpecOpValues(b *testing.B) { func Test_Message(t *testing.T) { l := []*BGPMessage{keepalive(), notification(), refresh(), NewTestBGPOpenMessage(), NewTestBGPUpdateMessage()} + for _, m1 := range l { - buf1, _ := m1.Serialize() + buf1, err := m1.Serialize() + require.NoError(t, err) + t.Log("LEN =", len(buf1)) m2, err := ParseBGPMessage(buf1) - if err != nil { - t.Error(err) - } + require.NoError(t, err) + // FIXME: shouldn't but workaround for some structs. - buf2, _ := m2.Serialize() - - if reflect.DeepEqual(m1, m2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), m1, buf1) - t.Error(len(buf2), m2, buf2) - } + + assert.Equal(t, m1, m2) } } @@ -395,26 +390,24 @@ func Test_FlowSpecNlri(t *testing.T) { lastFragment := uint64(0x08) item5 := NewFlowSpecComponentItem(BITMASK_FLAG_OP_MATCH, isFragment) item6 := NewFlowSpecComponentItem(BITMASK_FLAG_OP_AND, lastFragment) + cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_FRAGMENT, []*FlowSpecComponentItem{item5, item6})) item7 := NewFlowSpecComponentItem(0, TCP_FLAG_ACK) item8 := NewFlowSpecComponentItem(BITMASK_FLAG_OP_AND|BITMASK_FLAG_OP_NOT, TCP_FLAG_URGENT) + cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_TCP_FLAG, []*FlowSpecComponentItem{item7, item8})) n1 := NewFlowSpecIPv4Unicast(cmp) + buf1, err := n1.Serialize() assert.Nil(err) + n2, err := NewPrefixFromRouteFamily(RouteFamilyToAfiSafi(RF_FS_IPv4_UC)) assert.Nil(err) + err = n2.DecodeFromBytes(buf1) assert.Nil(err) - buf2, _ := n2.Serialize() - if reflect.DeepEqual(n1, n2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n1, buf1) - t.Error(len(buf2), n2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + // should be equal + assert.Equal(n1, n2) } func Test_FlowSpecExtended(t *testing.T) { @@ -428,42 +421,36 @@ func Test_FlowSpecExtended(t *testing.T) { exts = append(exts, NewTrafficRemarkExtended(10)) m1 := NewPathAttributeExtendedCommunities(exts) buf1, err := m1.Serialize() - assert.Nil(err) + require.NoError(t, err) + m2 := NewPathAttributeExtendedCommunities(nil) err = m2.DecodeFromBytes(buf1) - assert.Nil(err) - buf2, _ := m2.Serialize() - if reflect.DeepEqual(m1, m2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), m1, buf1) - t.Error(len(buf2), m2, buf2) - } + require.NoError(t, err) + + _, err = m2.Serialize() + require.NoError(t, err) + + assert.Equal(m1, m2) } func Test_IP6FlowSpecExtended(t *testing.T) { - assert := assert.New(t) exts := make([]ExtendedCommunityInterface, 0) exts = append(exts, NewRedirectIPv6AddressSpecificExtended("2001:db8::68", 1000)) m1 := NewPathAttributeIP6ExtendedCommunities(exts) buf1, err := m1.Serialize() - assert.Nil(err) + require.NoError(t, err) + m2 := NewPathAttributeIP6ExtendedCommunities(nil) err = m2.DecodeFromBytes(buf1) - assert.Nil(err) - buf2, _ := m2.Serialize() - if reflect.DeepEqual(m1, m2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), m1, buf1) - t.Error(len(buf2), m2, buf2) - } + require.NoError(t, err) + + _, err = m2.Serialize() + require.NoError(t, err) + + assert.Equal(t, m1, m2) } func Test_FlowSpecNlriv6(t *testing.T) { - assert := assert.New(t) cmp := make([]FlowSpecComponentInterface, 0) cmp = append(cmp, NewFlowSpecDestinationPrefix6(NewIPv6AddrPrefix(64, "2001::"), 12)) cmp = append(cmp, NewFlowSpecSourcePrefix6(NewIPv6AddrPrefix(64, "2001::"), 12)) @@ -488,20 +475,18 @@ func Test_FlowSpecNlriv6(t *testing.T) { cmp = append(cmp, NewFlowSpecComponent(FLOW_SPEC_TYPE_TCP_FLAG, []*FlowSpecComponentItem{item6, item7})) n1 := NewFlowSpecIPv6Unicast(cmp) buf1, err := n1.Serialize() - assert.Nil(err) + require.NoError(t, err) + n2, err := NewPrefixFromRouteFamily(RouteFamilyToAfiSafi(RF_FS_IPv6_UC)) - assert.Nil(err) + require.NoError(t, err) + err = n2.DecodeFromBytes(buf1) - assert.Nil(err) - buf2, _ := n2.Serialize() - if reflect.DeepEqual(n1, n2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n1, buf1) - t.Error(len(buf2), n2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + require.NoError(t, err) + + _, err = n2.Serialize() + require.NoError(t, err) + + assert.Equal(t, n1, n2) } func Test_Aigp(t *testing.T) { @@ -509,19 +494,13 @@ func Test_Aigp(t *testing.T) { m := NewAigpTLVIgpMetric(1000) a1 := NewPathAttributeAigp([]AigpTLVInterface{m}) buf1, err := a1.Serialize() - assert.Nil(err) + require.NoError(t, err) + a2 := NewPathAttributeAigp(nil) err = a2.DecodeFromBytes(buf1) - assert.Nil(err) - buf2, _ := a2.Serialize() - if reflect.DeepEqual(a1, a2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), a1, buf1) - t.Error(len(buf2), a2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + require.NoError(t, err) + + assert.Equal(a1, a2) } func Test_FlowSpecNlriL2(t *testing.T) { @@ -540,15 +519,8 @@ func Test_FlowSpecNlriL2(t *testing.T) { assert.Nil(err) err = n2.DecodeFromBytes(buf1) assert.Nil(err) - buf2, _ := n2.Serialize() - if reflect.DeepEqual(n1, n2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n1, buf1) - t.Error(len(buf2), n2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + + assert.Equal(n1, n2) } func Test_NotificationErrorCode(t *testing.T) { @@ -572,16 +544,9 @@ func Test_FlowSpecNlriVPN(t *testing.T) { n2, err := NewPrefixFromRouteFamily(RouteFamilyToAfiSafi(RF_FS_IPv4_VPN)) assert.Nil(err) err = n2.DecodeFromBytes(buf1) - assert.Nil(err) - buf2, _ := n2.Serialize() - if reflect.DeepEqual(n1, n2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n1, buf1) - t.Error(len(buf2), n2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + require.NoError(t, err) + + assert.Equal(n1, n2) } func Test_EVPNIPPrefixRoute(t *testing.T) { @@ -606,17 +571,8 @@ func Test_EVPNIPPrefixRoute(t *testing.T) { assert.Nil(err) err = n2.DecodeFromBytes(buf1) assert.Nil(err) - buf2, _ := n2.Serialize() - t.Log(n1.RouteTypeData.(*EVPNIPPrefixRoute).ESI.Value, n2.(*EVPNNLRI).RouteTypeData.(*EVPNIPPrefixRoute).ESI.Value) - t.Log(reflect.DeepEqual(n1.RouteTypeData.(*EVPNIPPrefixRoute).ESI.Value, n2.(*EVPNNLRI).RouteTypeData.(*EVPNIPPrefixRoute).ESI.Value)) - if reflect.DeepEqual(n1, n2) { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n1, buf1) - t.Error(len(buf2), n2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + + assert.Equal(n1, n2) } func Test_CapExtendedNexthop(t *testing.T) { @@ -627,15 +583,8 @@ func Test_CapExtendedNexthop(t *testing.T) { assert.Nil(err) n2, err := DecodeCapability(buf1) assert.Nil(err) - buf2, _ := n2.Serialize() - if reflect.DeepEqual(n1, n2) { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n1, buf1) - t.Error(len(buf2), n2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + + assert.Equal(n1, n2) } func Test_AddPath(t *testing.T) { @@ -815,15 +764,8 @@ func Test_MpReachNLRIWithIPv4MappedIPv6Prefix(t *testing.T) { assert.Nil(err) err = n2.DecodeFromBytes(buf1) assert.Nil(err) - buf2, _ := n2.Serialize() - if reflect.DeepEqual(n1, n2) { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n1, buf1) - t.Error(len(buf2), n2, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + + assert.Equal(n1, n2) label := NewMPLSLabelStack(2) @@ -834,16 +776,8 @@ func Test_MpReachNLRIWithIPv4MappedIPv6Prefix(t *testing.T) { assert.Nil(err) err = n4.DecodeFromBytes(buf1) assert.Nil(err) - buf2, _ = n3.Serialize() - t.Log(n3, n4) - if reflect.DeepEqual(n3, n4) { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), n3, buf1) - t.Error(len(buf2), n4, buf2) - t.Log(bytes.Equal(buf1, buf2)) - } + + assert.Equal(n3, n4) } func Test_MpReachNLRIWithIPv6PrefixWithIPv4Peering(t *testing.T) { @@ -1133,7 +1067,7 @@ func Test_ParseEthernetSegmentIdentifier(t *testing.T) { // "single-homed" esiZero := EthernetSegmentIdentifier{} - args := make([]string, 0, 0) + args := make([]string, 0) esi, err := ParseEthernetSegmentIdentifier(args) assert.Nil(err) assert.Equal(esiZero, esi) diff --git a/packet/bgp/validate.go b/packet/bgp/validate.go index 1b8f27c9..60cf26e4 100644 --- a/packet/bgp/validate.go +++ b/packet/bgp/validate.go @@ -274,7 +274,7 @@ func validateAsPathValueBytes(data []byte) (bool, error) { return false, NewMessageError(eCode, eSubCode, nil, "AS PATH the number of AS is incorrect") } segLength := int(asNum) - if use4byte == true { + if use4byte { segLength *= 4 } else { segLength *= 2 diff --git a/packet/bgp/validate_test.go b/packet/bgp/validate_test.go index 4228c260..f780b5cc 100644 --- a/packet/bgp/validate_test.go +++ b/packet/bgp/validate_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func bgpupdate() *BGPMessage { @@ -47,6 +48,7 @@ func Test_Validate_CapV4(t *testing.T) { assert.Error(err) res, err = ValidateUpdateMsg(message, map[RouteFamily]BGPAddPathMode{RF_IPv4_UC: BGP_ADD_PATH_BOTH}, false, false) + require.NoError(t, err) assert.Equal(true, res) } @@ -58,6 +60,7 @@ func Test_Validate_CapV6(t *testing.T) { assert.NoError(err) res, err = ValidateUpdateMsg(message, map[RouteFamily]BGPAddPathMode{RF_IPv4_UC: BGP_ADD_PATH_BOTH}, false, false) + require.NoError(t, err) assert.Equal(false, res) } @@ -295,12 +298,12 @@ func Test_Validate_unrecognized_well_known(t *testing.T) { } func Test_Validate_aspath(t *testing.T) { - assert := assert.New(t) message := bgpupdate().Body.(*BGPUpdate) // VALID AS_PATH res, err := ValidateUpdateMsg(message, map[RouteFamily]BGPAddPathMode{RF_IPv4_UC: BGP_ADD_PATH_BOTH}, true, false) + require.NoError(t, err) assert.Equal(true, res) // CONFED_SET @@ -360,6 +363,7 @@ func Test_Validate_aspath(t *testing.T) { assert.Nil(e.Data) res, err = ValidateUpdateMsg(message, map[RouteFamily]BGPAddPathMode{RF_IPv4_UC: BGP_ADD_PATH_BOTH}, true, true) + require.NoError(t, err) assert.Equal(true, res) } diff --git a/packet/bmp/bmp_test.go b/packet/bmp/bmp_test.go index 266d6b99..fde4cd0f 100644 --- a/packet/bmp/bmp_test.go +++ b/packet/bmp/bmp_test.go @@ -16,27 +16,21 @@ package bmp import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/osrg/gobgp/packet/bgp" + "github.com/stretchr/testify/assert" - "reflect" - "testing" ) func verify(t *testing.T, m1 *BMPMessage) { buf1, _ := m1.Serialize() m2, err := ParseBMPMessage(buf1) - if err != nil { - t.Error(err) - } - buf2, _ := m2.Serialize() + require.NoError(t, err) - if reflect.DeepEqual(m1, m2) == true { - t.Log("OK") - } else { - t.Error("Something wrong") - t.Error(len(buf1), m1, buf1) - t.Error(len(buf2), m2, buf2) - } + assert.Equal(t, m1, m2) } func Test_Initiation(t *testing.T) { diff --git a/packet/rtr/rtr_test.go b/packet/rtr/rtr_test.go index 961805d5..08f930b2 100644 --- a/packet/rtr/rtr_test.go +++ b/packet/rtr/rtr_test.go @@ -19,26 +19,22 @@ import ( "encoding/hex" "math/rand" "net" - "reflect" "testing" "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func verifyRTRMessage(t *testing.T, m1 RTRMessage) { buf1, _ := m1.Serialize() m2, err := ParseRTR(buf1) - if err != nil { - t.Error(err) - } - buf2, _ := m2.Serialize() - - if reflect.DeepEqual(buf1, buf2) == true { - t.Log("OK") - } else { - t.Errorf("Something wrong") - t.Error(len(buf1), m1, hex.EncodeToString(buf1)) - t.Error(len(buf2), m2, hex.EncodeToString(buf2)) - } + require.NoError(t, err) + + buf2, err := m2.Serialize() + require.NoError(t, err) + + assert.Equal(t, buf1, buf2, "buf1: %v buf2: %v", hex.EncodeToString(buf1), hex.EncodeToString(buf2)) } func randUint32() uint32 { diff --git a/server/fsm.go b/server/fsm.go index 9951fe68..d65107df 100644 --- a/server/fsm.go +++ b/server/fsm.go @@ -23,14 +23,14 @@ import ( "strconv" "time" - "github.com/eapache/channels" - log "github.com/sirupsen/logrus" - "gopkg.in/tomb.v2" - "github.com/osrg/gobgp/config" "github.com/osrg/gobgp/packet/bgp" "github.com/osrg/gobgp/packet/bmp" "github.com/osrg/gobgp/table" + + "github.com/eapache/channels" + log "github.com/sirupsen/logrus" + "gopkg.in/tomb.v2" ) type PeerDownReason int @@ -1048,9 +1048,7 @@ func open2Cap(open *bgp.BGPOpen, n *config.Neighbor) (map[bgp.BGPCapabilityCode] if caps, y := capMap[bgp.BGP_CAP_ADD_PATH]; y { items := make([]*bgp.CapAddPathTuple, 0, len(caps)) for _, c := range caps { - for _, i := range c.(*bgp.CapAddPath).Tuples { - items = append(items, i) - } + items = append(items, c.(*bgp.CapAddPath).Tuples...) } capMap[bgp.BGP_CAP_ADD_PATH] = []bgp.ParameterCapabilityInterface{bgp.NewCapAddPath(items)} } diff --git a/server/peer.go b/server/peer.go index e3981024..3a43fac9 100644 --- a/server/peer.go +++ b/server/peer.go @@ -42,8 +42,8 @@ type PeerGroup struct { func NewPeerGroup(c *config.PeerGroup) *PeerGroup { return &PeerGroup{ Conf: c, - members: make(map[string]config.Neighbor, 0), - dynamicNeighbors: make(map[string]*config.DynamicNeighbor, 0), + members: make(map[string]config.Neighbor), + dynamicNeighbors: make(map[string]*config.DynamicNeighbor), } } @@ -293,7 +293,7 @@ func (peer *Peer) markLLGRStale(fs []bgp.RouteFamily) []*table.Path { for i, p := range paths { doStale := true for _, c := range p.GetCommunities() { - if c == bgp.COMMUNITY_NO_LLGR { + if c == uint32(bgp.COMMUNITY_NO_LLGR) { doStale = false p = p.Clone(true) break @@ -301,7 +301,7 @@ func (peer *Peer) markLLGRStale(fs []bgp.RouteFamily) []*table.Path { } if doStale { p = p.Clone(false) - p.SetCommunities([]uint32{bgp.COMMUNITY_LLGR_STALE}, false) + p.SetCommunities([]uint32{uint32(bgp.COMMUNITY_LLGR_STALE)}, false) } paths[i] = p } @@ -317,10 +317,6 @@ func (peer *Peer) stopPeerRestarting() { } -func (peer *Peer) getAccepted(rfList []bgp.RouteFamily) []*table.Path { - return peer.adjRibIn.PathList(rfList, true) -} - func (peer *Peer) filterPathFromSourcePeer(path, old *table.Path) *table.Path { if peer.ID() != path.GetSource().Address.String() { return path diff --git a/server/server.go b/server/server.go index 8198294d..c4e5f5de 100644 --- a/server/server.go +++ b/server/server.go @@ -242,10 +242,12 @@ func (server *BgpServer) Serve() { } return true }(peer.fsm.pConf.Transport.Config.LocalAddress) - if localAddrValid == false { + + if !localAddrValid { conn.Close() return } + log.WithFields(log.Fields{ "Topic": "Peer", }).Debugf("Accepted a new passive connection from:%s", remoteAddr) @@ -1375,7 +1377,6 @@ func (server *BgpServer) handleFSMMessage(peer *Peer, e *FsmMsg) { }).Panic("unknown msg type") } } - return } func (s *BgpServer) AddCollector(c *config.CollectorConfig) error { @@ -1828,7 +1829,7 @@ func (s *BgpServer) softResetOut(addr string, family bgp.RouteFamily, deferral b if len(pathList) > 0 { sendFsmOutgoingMsg(peer, pathList, nil, false) } - if deferral == false && len(filtered) > 0 { + if !deferral && len(filtered) > 0 { withdrawnList := make([]*table.Path, 0, len(filtered)) for _, p := range filtered { withdrawnList = append(withdrawnList, p.Clone(true)) @@ -2293,7 +2294,8 @@ func (s *BgpServer) updateNeighbor(c *config.Neighbor) (needsSoftResetIn bool, e if original.Config.AdminDown != c.Config.AdminDown { sub = bgp.BGP_ERROR_SUB_ADMINISTRATIVE_SHUTDOWN state := "Admin Down" - if c.Config.AdminDown == false { + + if !c.Config.AdminDown { state = "Admin Up" } log.WithFields(log.Fields{ @@ -2734,7 +2736,6 @@ type watchOptions struct { initPeerState bool tableName string recvMessage bool - sentMessage bool } type WatchOption func(*watchOptions) @@ -2859,16 +2860,10 @@ func (w *Watcher) notify(v WatchEvent) { } func (w *Watcher) loop() { - for { - select { - case ev, ok := <-w.ch.Out(): - if !ok { - close(w.realCh) - return - } - w.realCh <- ev.(WatchEvent) - } + for ev := range w.ch.Out() { + w.realCh <- ev.(WatchEvent) } + close(w.realCh) } func (w *Watcher) Stop() { diff --git a/server/server_test.go b/server/server_test.go index c75003ed..ff9bfd40 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -16,11 +16,14 @@ package server import ( + "context" "net" "runtime" "testing" "time" + "github.com/stretchr/testify/require" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -277,14 +280,8 @@ func process(rib *table.TableManager, l []*table.Path) (*table.Path, *table.Path if len(news) != 1 { panic("can't handle multiple paths") } - for idx, path := range news { - var old *table.Path - if olds != nil { - old = olds[idx] - } - return path, old - } - return nil, nil + + return news[0], olds[0] } func TestFilterpathWitheBGP(t *testing.T) { @@ -656,22 +653,25 @@ func TestGracefulRestartTimerExpired(t *testing.T) { // Create dummy session which does NOT send BGP OPEN message in order to // cause Graceful Restart timer expired. var conn net.Conn - for { - time.Sleep(time.Second) - var err error - conn, err = net.Dial("tcp", "127.0.0.1:10179") - if err != nil { - log.Warn("net.Dial:", err) - } - break - } + + conn, err = net.Dial("tcp", "127.0.0.1:10179") + require.NoError(t, err) defer conn.Close() + ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) + defer cancel() + // Waiting for Graceful Restart timer expired and moving on to IDLE state. for { - time.Sleep(time.Second) if s1.GetNeighbor("", false)[0].State.SessionState == config.SESSION_STATE_IDLE { break } + + select { + case <-time.After(time.Second): + case <-ctx.Done(): + t.Fatalf("failed to enter IDLE state in the deadline") + return + } } } diff --git a/server/zclient.go b/server/zclient.go index d5b026ac..37281f7f 100644 --- a/server/zclient.go +++ b/server/zclient.go @@ -283,10 +283,6 @@ type zebraClient struct { dead chan struct{} } -func (z *zebraClient) stop() { - close(z.dead) -} - func (z *zebraClient) getPathListWithNexthopUpdate(body *zebra.NexthopUpdateBody) []*table.Path { rib := &table.TableManager{ Tables: make(map[bgp.RouteFamily]*table.Table), @@ -326,7 +322,6 @@ func (z *zebraClient) updatePathByNexthopCache(paths []*table.Path) { }).Error("failed to update nexthop reachability") } } - return } func (z *zebraClient) loop() { diff --git a/table/destination.go b/table/destination.go index bf375cd4..e0d0bc90 100644 --- a/table/destination.go +++ b/table/destination.go @@ -255,18 +255,6 @@ func (dd *Destination) GetMultiBestPath(id string) []*Path { return getMultiBestPath(id, dd.knownPathList) } -func (dd *Destination) validatePath(path *Path) { - if path == nil || path.GetRouteFamily() != dd.routeFamily { - - log.WithFields(log.Fields{ - "Topic": "Table", - "Key": dd.GetNlri().String(), - "Path": path, - "ExpectedRF": dd.routeFamily, - }).Error("path is nil or invalid route family") - } -} - // Calculates best-path among known paths for this destination. // // Modifies destination's state related to stored paths. Removes withdrawn @@ -530,10 +518,7 @@ func (dst *Destination) sort() { better.reason = reason - if better == path1 { - return true - } - return false + return better == path1 }) } diff --git a/table/message.go b/table/message.go index a7195ac6..8b9a4440 100644 --- a/table/message.go +++ b/table/message.go @@ -201,9 +201,7 @@ func UpdatePathAttrs4ByteAs(msg *bgp.BGPUpdate) error { } newIntfParams := make([]bgp.AsPathParamInterface, 0, len(asAttr.Value)) - for _, p := range newParams { - newIntfParams = append(newIntfParams, p) - } + newIntfParams = append(newIntfParams, newParams...) msg.PathAttributes[asAttrPos] = bgp.NewPathAttributeAsPath(newIntfParams) return nil @@ -384,7 +382,7 @@ func (p *packerV4) add(path *Path) { if cages, y := p.hashmap[key]; y { added := false for _, c := range cages { - if bytes.Compare(c.attrsBytes, attrsB.Bytes()) == 0 { + if bytes.Equal(c.attrsBytes, attrsB.Bytes()) { c.paths = append(c.paths, path) added = true break diff --git a/table/message_test.go b/table/message_test.go index f7cb6f5e..11c8dd19 100644 --- a/table/message_test.go +++ b/table/message_test.go @@ -578,9 +578,7 @@ func TestMergeV4NLRIs(t *testing.T) { for _, msg := range msgs { u := msg.Body.(*bgp.BGPUpdate) assert.Equal(t, len(u.PathAttributes), 3) - for _, n := range u.NLRI { - l = append(l, n) - } + l = append(l, u.NLRI...) } assert.Equal(t, len(l), nr) @@ -651,9 +649,7 @@ func TestMergeV4Withdraw(t *testing.T) { for _, msg := range msgs { u := msg.Body.(*bgp.BGPUpdate) assert.Equal(t, len(u.PathAttributes), 0) - for _, n := range u.WithdrawnRoutes { - l = append(l, n) - } + l = append(l, u.WithdrawnRoutes...) } assert.Equal(t, len(l), nr) for i, addr := range addrs { diff --git a/table/path.go b/table/path.go index 246f6aff..fa26f2e3 100644 --- a/table/path.go +++ b/table/path.go @@ -407,7 +407,7 @@ func (path *Path) SetAsLooped(y bool) { func (path *Path) IsLLGRStale() bool { for _, c := range path.GetCommunities() { - if c == bgp.COMMUNITY_LLGR_STALE { + if c == uint32(bgp.COMMUNITY_LLGR_STALE) { return true } } @@ -798,7 +798,6 @@ func (path *Path) RemovePrivateAS(localAS uint32, option config.RemovePrivateAsO } path.setPathAttr(bgp.NewPathAttributeAsPath(newASParams)) } - return } func (path *Path) removeConfedAs() { @@ -925,9 +924,7 @@ func (path *Path) GetExtCommunities() []bgp.ExtendedCommunityInterface { eCommunityList := make([]bgp.ExtendedCommunityInterface, 0) if attr := path.getPathAttr(bgp.BGP_ATTR_TYPE_EXTENDED_COMMUNITIES); attr != nil { eCommunities := attr.(*bgp.PathAttributeExtendedCommunities).Value - for _, eCommunity := range eCommunities { - eCommunityList = append(eCommunityList, eCommunity) - } + eCommunityList = append(eCommunityList, eCommunities...) } return eCommunityList } @@ -951,9 +948,7 @@ func (path *Path) GetLargeCommunities() []*bgp.LargeCommunity { if a := path.getPathAttr(bgp.BGP_ATTR_TYPE_LARGE_COMMUNITY); a != nil { v := a.(*bgp.PathAttributeLargeCommunities).Values ret := make([]*bgp.LargeCommunity, 0, len(v)) - for _, c := range v { - ret = append(ret, c) - } + ret = append(ret, v...) return ret } return nil @@ -979,20 +974,19 @@ func (path *Path) GetMed() (uint32, error) { // SetMed replace, add or subtraction med with new ones. func (path *Path) SetMed(med int64, doReplace bool) error { - parseMed := func(orgMed uint32, med int64, doReplace bool) (*bgp.PathAttributeMultiExitDisc, error) { - newMed := &bgp.PathAttributeMultiExitDisc{} if doReplace { - newMed = bgp.NewPathAttributeMultiExitDisc(uint32(med)) - } else { - if int64(orgMed)+med < 0 { - return nil, fmt.Errorf("med value invalid. it's underflow threshold.") - } else if int64(orgMed)+med > int64(math.MaxUint32) { - return nil, fmt.Errorf("med value invalid. it's overflow threshold.") - } - newMed = bgp.NewPathAttributeMultiExitDisc(uint32(int64(orgMed) + med)) + return bgp.NewPathAttributeMultiExitDisc(uint32(med)), nil + } + + medVal := int64(orgMed) + med + if medVal < 0 { + return nil, fmt.Errorf("med value invalid. it's underflow threshold: %v", medVal) + } else if medVal > int64(math.MaxUint32) { + return nil, fmt.Errorf("med value invalid. it's overflow threshold: %v", medVal) } - return newMed, nil + + return bgp.NewPathAttributeMultiExitDisc(uint32(int64(orgMed) + med)), nil } m := uint32(0) diff --git a/table/policy.go b/table/policy.go index da564abd..325121c0 100644 --- a/table/policy.go +++ b/table/policy.go @@ -26,12 +26,11 @@ import ( "strings" "sync" - log "github.com/sirupsen/logrus" - - radix "github.com/armon/go-radix" - "github.com/osrg/gobgp/config" "github.com/osrg/gobgp/packet/bgp" + + radix "github.com/armon/go-radix" + log "github.com/sirupsen/logrus" ) type PolicyOptions struct { @@ -310,6 +309,8 @@ func (p *Prefix) PrefixString() string { return p.Prefix.String() } +var _regexpPrefixRange = regexp.MustCompile(`(\d+)\.\.(\d+)`) + func NewPrefix(c config.Prefix) (*Prefix, error) { _, prefix, err := net.ParseCIDR(c.IpPrefix) if err != nil { @@ -325,28 +326,30 @@ func NewPrefix(c config.Prefix) (*Prefix, error) { AddressFamily: rf, } maskRange := c.MasklengthRange + if maskRange == "" { l, _ := prefix.Mask.Size() maskLength := uint8(l) p.MasklengthRangeMax = maskLength p.MasklengthRangeMin = maskLength - } else { - exp := regexp.MustCompile("(\\d+)\\.\\.(\\d+)") - elems := exp.FindStringSubmatch(maskRange) - if len(elems) != 3 { - log.WithFields(log.Fields{ - "Topic": "Policy", - "Type": "Prefix", - "MaskRangeFormat": maskRange, - }).Warn("mask length range format is invalid.") - return nil, fmt.Errorf("mask length range format is invalid") - } - // we've already checked the range is sane by regexp - min, _ := strconv.ParseUint(elems[1], 10, 8) - max, _ := strconv.ParseUint(elems[2], 10, 8) - p.MasklengthRangeMin = uint8(min) - p.MasklengthRangeMax = uint8(max) + return p, nil } + + elems := _regexpPrefixRange.FindStringSubmatch(maskRange) + if len(elems) != 3 { + log.WithFields(log.Fields{ + "Topic": "Policy", + "Type": "Prefix", + "MaskRangeFormat": maskRange, + }).Warn("mask length range format is invalid.") + return nil, fmt.Errorf("mask length range format is invalid") + } + + // we've already checked the range is sane by regexp + min, _ := strconv.ParseUint(elems[1], 10, 8) + max, _ := strconv.ParseUint(elems[2], 10, 8) + p.MasklengthRangeMin = uint8(min) + p.MasklengthRangeMax = uint8(max) return p, nil } @@ -817,32 +820,35 @@ func (m *singleAsPathMatch) Match(aspath []uint32) bool { return false } +var ( + _regexpLeftMostRe = regexp.MustCompile(`$\^([0-9]+)_^`) + _regexpOriginRe = regexp.MustCompile(`^_([0-9]+)\$$`) + _regexpIncludeRe = regexp.MustCompile("^_([0-9]+)_$") + _regexpOnlyRe = regexp.MustCompile(`^\^([0-9]+)\$$`) +) + func NewSingleAsPathMatch(arg string) *singleAsPathMatch { - leftMostRe := regexp.MustCompile("$\\^([0-9]+)_^") - originRe := regexp.MustCompile("^_([0-9]+)\\$$") - includeRe := regexp.MustCompile("^_([0-9]+)_$") - onlyRe := regexp.MustCompile("^\\^([0-9]+)\\$$") switch { - case leftMostRe.MatchString(arg): - asn, _ := strconv.ParseUint(leftMostRe.FindStringSubmatch(arg)[1], 10, 32) + case _regexpLeftMostRe.MatchString(arg): + asn, _ := strconv.ParseUint(_regexpLeftMostRe.FindStringSubmatch(arg)[1], 10, 32) return &singleAsPathMatch{ asn: uint32(asn), mode: LEFT_MOST, } - case originRe.MatchString(arg): - asn, _ := strconv.ParseUint(originRe.FindStringSubmatch(arg)[1], 10, 32) + case _regexpOriginRe.MatchString(arg): + asn, _ := strconv.ParseUint(_regexpOriginRe.FindStringSubmatch(arg)[1], 10, 32) return &singleAsPathMatch{ asn: uint32(asn), mode: ORIGIN, } - case includeRe.MatchString(arg): - asn, _ := strconv.ParseUint(includeRe.FindStringSubmatch(arg)[1], 10, 32) + case _regexpIncludeRe.MatchString(arg): + asn, _ := strconv.ParseUint(_regexpIncludeRe.FindStringSubmatch(arg)[1], 10, 32) return &singleAsPathMatch{ asn: uint32(asn), mode: INCLUDE, } - case onlyRe.MatchString(arg): - asn, _ := strconv.ParseUint(onlyRe.FindStringSubmatch(arg)[1], 10, 32) + case _regexpOnlyRe.MatchString(arg): + asn, _ := strconv.ParseUint(_regexpOnlyRe.FindStringSubmatch(arg)[1], 10, 32) return &singleAsPathMatch{ asn: uint32(asn), mode: ONLY, @@ -1085,13 +1091,15 @@ func (s *CommunitySet) MarshalJSON() ([]byte, error) { return json.Marshal(s.ToConfig()) } +var _regexpCommunity = regexp.MustCompile(`(\d+):(\d+)`) + func ParseCommunity(arg string) (uint32, error) { i, err := strconv.ParseUint(arg, 10, 32) if err == nil { return uint32(i), nil } - exp := regexp.MustCompile("(\\d+):(\\d+)") - elems := exp.FindStringSubmatch(arg) + + elems := _regexpCommunity.FindStringSubmatch(arg) if len(elems) == 3 { fst, _ := strconv.ParseUint(elems[1], 10, 16) snd, _ := strconv.ParseUint(elems[2], 10, 16) @@ -1136,24 +1144,25 @@ func ParseExtCommunity(arg string) (bgp.ExtendedCommunityInterface, error) { return bgp.ParseExtendedCommunity(subtype, value) } +var _regexpCommunity2 = regexp.MustCompile(`(\d+.)*\d+:\d+`) + func ParseCommunityRegexp(arg string) (*regexp.Regexp, error) { i, err := strconv.ParseUint(arg, 10, 32) if err == nil { - return regexp.MustCompile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff)), nil + return regexp.Compile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff)) } - if regexp.MustCompile("(\\d+.)*\\d+:\\d+").MatchString(arg) { - return regexp.MustCompile(fmt.Sprintf("^%s$", arg)), nil + + if _regexpCommunity2.MatchString(arg) { + return regexp.Compile(fmt.Sprintf("^%s$", arg)) } + for i, v := range bgp.WellKnownCommunityNameMap { if strings.Replace(strings.ToLower(arg), "_", "-", -1) == v { - return regexp.MustCompile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff)), nil + return regexp.Compile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff)) } } - exp, err := regexp.Compile(arg) - if err != nil { - return nil, fmt.Errorf("invalid community format: %s", arg) - } - return exp, nil + + return regexp.Compile(arg) } func ParseExtCommunityRegexp(arg string) (bgp.ExtendedCommunityAttrSubType, *regexp.Regexp, error) { @@ -1304,14 +1313,17 @@ func (s *LargeCommunitySet) MarshalJSON() ([]byte, error) { return json.Marshal(s.ToConfig()) } +var _regexpCommunityLarge = regexp.MustCompile(`\d+:\d+:\d+`) + func ParseLargeCommunityRegexp(arg string) (*regexp.Regexp, error) { - if regexp.MustCompile("\\d+:\\d+:\\d+").MatchString(arg) { - return regexp.MustCompile(fmt.Sprintf("^%s$", arg)), nil + if _regexpCommunityLarge.MatchString(arg) { + return regexp.Compile(fmt.Sprintf("^%s$", arg)) } exp, err := regexp.Compile(arg) if err != nil { - return nil, fmt.Errorf("invalid large-community format: %s", arg) + return nil, fmt.Errorf("invalid large-community format: %v", err) } + return exp, nil } @@ -2072,7 +2084,7 @@ func RegexpRemoveCommunities(path *Path, exps []*regexp.Regexp) { break } } - if match == false { + if !match { newComms = append(newComms, comm) } } @@ -2095,7 +2107,7 @@ func RegexpRemoveExtCommunities(path *Path, exps []*regexp.Regexp, subtypes []bg break } } - if match == false { + if !match { newComms = append(newComms, comm) } } @@ -2114,7 +2126,7 @@ func RegexpRemoveLargeCommunities(path *Path, exps []*regexp.Regexp) { break } } - if match == false { + if !match { newComms = append(newComms, comm) } } @@ -2156,10 +2168,12 @@ func (a *CommunityAction) MarshalJSON() ([]byte, error) { return json.Marshal(a.ToConfig()) } +// TODO: this is not efficient use of regexp, probably slow +var _regexpCommunityReplaceString = regexp.MustCompile(`[\^\$]`) + func (a *CommunityAction) String() string { list := a.ToConfig().SetCommunityMethod.CommunitiesList - exp := regexp.MustCompile("[\\^\\$]") - l := exp.ReplaceAllString(strings.Join(list, ", "), "") + l := _regexpCommunityReplaceString.ReplaceAllString(strings.Join(list, ", "), "") return fmt.Sprintf("%s[%s]", a.action, l) } @@ -2253,8 +2267,7 @@ func (a *ExtCommunityAction) ToConfig() *config.SetExtCommunity { func (a *ExtCommunityAction) String() string { list := a.ToConfig().SetExtCommunityMethod.CommunitiesList - exp := regexp.MustCompile("[\\^\\$]") - l := exp.ReplaceAllString(strings.Join(list, ", "), "") + l := _regexpCommunityReplaceString.ReplaceAllString(strings.Join(list, ", "), "") return fmt.Sprintf("%s[%s]", a.action, l) } @@ -2342,8 +2355,7 @@ func (a *LargeCommunityAction) ToConfig() *config.SetLargeCommunity { func (a *LargeCommunityAction) String() string { list := a.ToConfig().SetLargeCommunityMethod.CommunitiesList - exp := regexp.MustCompile("[\\^\\$]") - l := exp.ReplaceAllString(strings.Join(list, ", "), "") + l := _regexpCommunityReplaceString.ReplaceAllString(strings.Join(list, ", "), "") return fmt.Sprintf("%s[%s]", a.action, l) } @@ -2432,12 +2444,14 @@ func (a *MedAction) MarshalJSON() ([]byte, error) { return json.Marshal(a.ToConfig()) } +var _regexpParseMedAction = regexp.MustCompile(`^(\+|\-)?(\d+)$`) + func NewMedAction(c config.BgpSetMedType) (*MedAction, error) { if string(c) == "" { return nil, nil } - exp := regexp.MustCompile("^(\\+|\\-)?(\\d+)$") - elems := exp.FindStringSubmatch(string(c)) + + elems := _regexpParseMedAction.FindStringSubmatch(string(c)) if len(elems) != 3 { return nil, fmt.Errorf("invalid med action format") } diff --git a/table/policy_test.go b/table/policy_test.go index 007936c1..858e1098 100644 --- a/table/policy_test.go +++ b/table/policy_test.go @@ -24,11 +24,12 @@ import ( "testing" "time" - log "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" - "github.com/osrg/gobgp/config" "github.com/osrg/gobgp/packet/bgp" + + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPrefixCalcurateNoRange(t *testing.T) { @@ -3060,6 +3061,7 @@ func TestLargeCommunityMatchAction(t *testing.T) { func TestAfiSafiInMatchPath(t *testing.T) { condition, err := NewAfiSafiInCondition([]config.AfiSafiType{config.AFI_SAFI_TYPE_L3VPN_IPV4_UNICAST, config.AFI_SAFI_TYPE_L3VPN_IPV6_UNICAST}) + require.NoError(t, err) rtExtCom, err := bgp.ParseExtendedCommunity(bgp.EC_SUBTYPE_ROUTE_TARGET, "100:100") assert.NoError(t, err) diff --git a/table/table_manager.go b/table/table_manager.go index 0bee18c3..26581cea 100644 --- a/table/table_manager.go +++ b/table/table_manager.go @@ -57,9 +57,7 @@ func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time) reach = a case *bgp.PathAttributeMpUnreachNLRI: l := make([]bgp.AddrPrefixInterface, 0, len(a.Value)) - for _, nlri := range a.Value { - l = append(l, nlri) - } + l = append(l, a.Value...) dels = append(dels, l...) default: attrs = append(attrs, attr) diff --git a/table/table_manager_test.go b/table/table_manager_test.go index ee94d7e5..11a7b066 100644 --- a/table/table_manager_test.go +++ b/table/table_manager_test.go @@ -18,12 +18,10 @@ package table import ( _ "fmt" "net" - "os" "testing" "time" "github.com/osrg/gobgp/packet/bgp" - log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) @@ -42,16 +40,6 @@ func (manager *TableManager) ProcessUpdate(fromPeer *PeerInfo, message *bgp.BGPM return pathList, nil } -func getLogger(lv log.Level) *log.Logger { - var l *log.Logger = &log.Logger{ - Out: os.Stderr, - Formatter: new(log.JSONFormatter), - Hooks: make(map[log.Level][]log.Hook), - Level: lv, - } - return l -} - func peerR1() *PeerInfo { peer := &PeerInfo{ AS: 65000, diff --git a/table/vrf.go b/table/vrf.go index 480f963f..f58bee2a 100644 --- a/table/vrf.go +++ b/table/vrf.go @@ -30,10 +30,7 @@ type Vrf struct { func (v *Vrf) Clone() *Vrf { f := func(rt []bgp.ExtendedCommunityInterface) []bgp.ExtendedCommunityInterface { l := make([]bgp.ExtendedCommunityInterface, 0, len(rt)) - for _, v := range rt { - l = append(l, v) - } - return l + return append(l, rt...) } return &Vrf{ Name: v.Name, diff --git a/tools/route-server/quagga-rsconfig.go b/tools/route-server/quagga-rsconfig.go index 80b9a9d0..517125f3 100644 --- a/tools/route-server/quagga-rsconfig.go +++ b/tools/route-server/quagga-rsconfig.go @@ -45,8 +45,6 @@ func (qt *QuaggaConfig) Config() *bytes.Buffer { } func create_config_files(nr int, outputDir string) { - quaggaConfigList := make([]*QuaggaConfig, 0) - gobgpConf := config.Bgp{} gobgpConf.Global.Config.As = 65000 gobgpConf.Global.Config.RouterId = "192.168.255.1" @@ -60,21 +58,24 @@ func create_config_files(nr int, outputDir string) { gobgpConf.Neighbors = append(gobgpConf.Neighbors, c) q := NewQuaggaConfig(i, &gobgpConf.Global, &c, net.ParseIP("10.0.255.1")) - quaggaConfigList = append(quaggaConfigList, q) - os.Mkdir(fmt.Sprintf("%s/q%d", outputDir, i), 0755) - err := ioutil.WriteFile(fmt.Sprintf("%s/q%d/bgpd.conf", outputDir, i), q.Config().Bytes(), 0644) - if err != nil { + + if err := os.Mkdir(fmt.Sprintf("%s/q%d", outputDir, i), 0755); err != nil { + log.Fatalf("failed to make directory: %v", err) + } + + if err := ioutil.WriteFile(fmt.Sprintf("%s/q%d/bgpd.conf", outputDir, i), q.Config().Bytes(), 0644); err != nil { log.Fatal(err) } } var buffer bytes.Buffer encoder := toml.NewEncoder(&buffer) - encoder.Encode(gobgpConf) + if err := encoder.Encode(gobgpConf); err != nil { + log.Fatalf("failed to encode config: %v", err) + } - err := ioutil.WriteFile(fmt.Sprintf("%s/gobgpd.conf", outputDir), buffer.Bytes(), 0644) - if err != nil { - log.Fatal(err) + if err := ioutil.WriteFile(fmt.Sprintf("%s/gobgpd.conf", outputDir), buffer.Bytes(), 0644); err != nil { + log.Fatalf("failed to write config file: %v", err) } } diff --git a/zebra/zapi.go b/zebra/zapi.go index 8b71f483..3b303082 100644 --- a/zebra/zapi.go +++ b/zebra/zapi.go @@ -907,7 +907,7 @@ func (b *HelloBody) Serialize(version uint8) ([]byte, error) { if version <= 3 { return []byte{uint8(b.RedistDefault)}, nil } else { // version >= 4 - buf := make([]byte, 3, 3) + buf := make([]byte, 3) buf[0] = uint8(b.RedistDefault) binary.BigEndian.PutUint16(buf[1:3], b.Instance) return buf, nil @@ -942,7 +942,7 @@ func (b *RedistributeBody) Serialize(version uint8) ([]byte, error) { if version <= 3 { return []byte{uint8(b.Redist)}, nil } else { // version >= 4 - buf := make([]byte, 4, 4) + buf := make([]byte, 4) buf[0] = uint8(b.Afi) buf[1] = uint8(b.Redist) binary.BigEndian.PutUint16(buf[2:4], b.Instance) @@ -1407,71 +1407,6 @@ func (n *Nexthop) String() string { return s } -func serializeNexthops(nexthops []*Nexthop, isV4 bool, version uint8) ([]byte, error) { - buf := make([]byte, 0) - if len(nexthops) == 0 { - return buf, nil - } - buf = append(buf, byte(len(nexthops))) - - nhIfindex := NEXTHOP_IFINDEX - nhIfname := NEXTHOP_IFNAME - nhIPv4 := NEXTHOP_IPV4 - nhIPv4Ifindex := NEXTHOP_IPV4_IFINDEX - nhIPv4Ifname := NEXTHOP_IPV4_IFNAME - nhIPv6 := NEXTHOP_IPV6 - nhIPv6Ifindex := NEXTHOP_IPV6_IFINDEX - nhIPv6Ifname := NEXTHOP_IPV6_IFNAME - if version >= 4 { - nhIfindex = FRR_NEXTHOP_IFINDEX - nhIfname = NEXTHOP_FLAG(0) - nhIPv4 = FRR_NEXTHOP_IPV4 - nhIPv4Ifindex = FRR_NEXTHOP_IPV4_IFINDEX - nhIPv4Ifname = NEXTHOP_FLAG(0) - nhIPv6 = FRR_NEXTHOP_IPV6 - nhIPv6Ifindex = FRR_NEXTHOP_IPV6_IFINDEX - nhIPv6Ifname = NEXTHOP_FLAG(0) - } - - for _, nh := range nexthops { - buf = append(buf, byte(nh.Type)) - - switch nh.Type { - case nhIfindex, nhIfname: - bbuf := make([]byte, 4) - binary.BigEndian.PutUint32(bbuf, nh.Ifindex) - buf = append(buf, bbuf...) - - case nhIPv4, nhIPv6: - if isV4 { - buf = append(buf, nh.Addr.To4()...) - } else { - buf = append(buf, nh.Addr.To16()...) - } - if version >= 4 { - // On FRRouting version 3.0 or later, NEXTHOP_IPV4 and - // NEXTHOP_IPV6 have the same structure with - // NEXTHOP_TYPE_IPV4_IFINDEX and NEXTHOP_TYPE_IPV6_IFINDEX. - bbuf := make([]byte, 4) - binary.BigEndian.PutUint32(bbuf, nh.Ifindex) - buf = append(buf, bbuf...) - } - - case nhIPv4Ifindex, nhIPv4Ifname, nhIPv6Ifindex, nhIPv6Ifname: - if isV4 { - buf = append(buf, nh.Addr.To4()...) - } else { - buf = append(buf, nh.Addr.To16()...) - } - bbuf := make([]byte, 4) - binary.BigEndian.PutUint32(bbuf, nh.Ifindex) - buf = append(buf, bbuf...) - } - } - - return buf, nil -} - func decodeNexthopsFromBytes(nexthops *[]*Nexthop, data []byte, isV4 bool, version uint8) (int, error) { addrLen := net.IPv4len if !isV4 { diff --git a/zebra/zapi_test.go b/zebra/zapi_test.go index 733feb93..9fda5416 100644 --- a/zebra/zapi_test.go +++ b/zebra/zapi_test.go @@ -21,6 +21,8 @@ import ( "syscall" "testing" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" ) @@ -113,6 +115,8 @@ func Test_InterfaceAddressUpdateBody(t *testing.T) { b := &InterfaceAddressUpdateBody{} err := b.DecodeFromBytes(buf, 2) + require.NoError(t, err) + assert.Equal(uint32(0), b.Index) assert.Equal(INTERFACE_ADDRESS_FLAG(1), b.Flags) assert.Equal("192.168.100.1", b.Prefix.String()) |