Review: Approve
LGTM with some minor comments
Diff comments:
> diff --git a/src/maasagent/cmd/netmon/main.go
> b/src/maasagent/cmd/netmon/main.go
> index e83655f..c88d9bf 100644
> --- a/src/maasagent/cmd/netmon/main.go
> +++ b/src/maasagent/cmd/netmon/main.go
> @@ -1,9 +1,84 @@
> package main
>
> +/*
> + Copyright 2023 Canonical Ltd. This software is licensed under the
> + GNU Affero General Public License version 3 (see the file LICENSE).
> +*/
> +
> import (
> + "context"
> + "encoding/json"
> + "errors"
> + "os"
> +
> + "github.com/rs/zerolog"
> + "github.com/rs/zerolog/log"
> + "golang.org/x/sync/errgroup"
> +
> "launchpad.net/maas/maas/src/maasagent/internal/netmon"
> )
>
> +var (
> + ErrMissingIface = errors.New("Missing interface argument")
> +)
> +
> +func Run() int {
> + zerolog.SetGlobalLevel(zerolog.InfoLevel)
> + log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr})
> +
> + if envLogLevel, ok := os.LookupEnv("LOG_LEVEL"); ok {
> + if logLevel, err := zerolog.ParseLevel(envLogLevel); err != nil
> {
> + log.Warn().Str("LOG_LEVEL", envLogLevel).Msg("Unknown
> log level, defaulting to INFO")
> + } else {
> + zerolog.SetGlobalLevel(logLevel)
> + }
> + }
> +
> + if len(os.Args) < 2 {
> + log.Error().Err(ErrMissingIface).Msg("Please provide an
> interface to monitor")
Since ErrMissingIface error is not used anywhere to check the actual error, we
can remove that variable and just print Msg("Missing interface argument")?
> + return 2
> + }
> + iface := os.Args[1]
> +
> + ctx, cancel := context.WithCancel(context.Background())
> +
> + sigC := make(chan os.Signal)
> + resultC := make(chan netmon.Result)
> +
> + g, ctx := errgroup.WithContext(ctx)
> + g.SetLimit(2)
> +
> + svc := netmon.NewService(iface)
> + g.Go(func() error {
> + return svc.Start(ctx, resultC)
> + })
> + g.Go(func() error {
> + encoder := json.NewEncoder(os.Stdout)
> + for {
> + select {
> + case <-sigC:
> + cancel()
> + return nil
> + case res, ok := <-resultC:
> + if !ok {
> + log.Debug().Msg("result channel has
> been closed")
> + return nil
> + }
> + err := encoder.Encode(res)
> + if err != nil {
> + return err
> + }
> + }
> + }
> + })
> + log.Info().Msg("Service netmon started")
> + if err := g.Wait(); err != nil {
> + log.Error().Err(err).Send()
> + return 1
> + }
> + return 0
> +}
> +
> func main() {
> - netmon.NewService()
> + os.Exit(Run())
> }
> diff --git a/src/maasagent/internal/arp/ethernet.go
> b/src/maasagent/internal/arp/ethernet.go
> new file mode 100644
> index 0000000..cb029aa
> --- /dev/null
> +++ b/src/maasagent/internal/arp/ethernet.go
> @@ -0,0 +1,142 @@
> +package arp
Maybe we could name it `package ethernet` because it contains not only ARP
specific things?
> +
> +/*
> + Copyright 2023 Canonical Ltd. This software is licensed under the
> + GNU Affero General Public License version 3 (see the file LICENSE).
> +*/
> +
> +import (
> + "encoding/binary"
> + "errors"
> + "io"
> + "net"
> +)
> +
> +const (
> + minEthernetLen = 14
> +)
> +
> +const (
> + // EthernetTypeLLC is a special ethernet type, if found the frame is
> truncated
> + EthernetTypeLLC uint16 = 0
> + // EthernetTypeIPv4 is the ethernet type for a frame containing an IPv4
> packet
> + EthernetTypeIPv4 uint16 = 0x0800
> + // EthernetTypeARP is the ethernet type for a frame containing an ARP
> packet
> + EthernetTypeARP uint16 = 0x0806
> + // EthernetTypeIPv6 is the ethernet type for a frame containing an IPv6
> packet
> + EthernetTypeIPv6 uint16 = 0x86dd
> + // EthernetTypeVLAN is the ethernet type for a frame containing a VLAN
> tag,
> + // the VLAN tag bytes will indicate the actual type of packet the frame
> contains
> + EthernetTypeVLAN uint16 = 0x8100
> +
> + // NonStdLenEthernetTypes is a magic number to find any non-standard
> types
> + // and mark them as EthernetTypeLLC
> + NonStdLenEthernetTypes uint16 = 0x600
> +)
> +
> +var (
> + // ErrNotVLAN is an error returned when calling
> EthernetFrame.ExtractVLAN
> + // if the frame is not of type EthernetTypeVLAN
> + ErrNotVLAN = errors.New("ethernet frame not of type VLAN")
> + // ErrMalformedVLAN is an error returned when parsing a VLAN tag
> + // that is malformed
> + ErrMalformedVLAN = errors.New("VLAN tag is malformed")
> + // ErrMalformedFrame is an error returned when parsing an ethernet frame
> + // that is malformed
> + ErrMalformedFrame = errors.New("malformed ethernet frame")
> +)
> +
> +// VLAN represents a VLAN tag within an ethernet frame
> +type VLAN struct {
> + Priority uint8
> + DropEligible bool
> + ID uint16
> + EthernetType uint16
> +}
> +
> +// UnmarshalBinary will take the ethernet frame's payload
> +// and extract a VLAN tag if one is present
> +func (v *VLAN) UnmarshalBinary(buf []byte) error {
> + if len(buf) < 4 {
> + return ErrMalformedVLAN
> + }
> +
> + // extract the first 3 bits
> + v.Priority = (buf[0] & 0xe0) >> 5
> + // extract the next bit and turn it into a bool
> + v.DropEligible = buf[0]&0x10 != 0
> + // extract the next 12 bits for an ID
> + v.ID = binary.BigEndian.Uint16(buf[:2]) & 0x0fff
> + // last 2 bytes are ethernet type
> + v.EthernetType = binary.BigEndian.Uint16(buf[2:])
> + return nil
> +}
> +
> +// EthernetFrame represents an ethernet frame
> +type EthernetFrame struct {
> + SrcMAC net.HardwareAddr
> + DstMAC net.HardwareAddr
> + EthernetType uint16
> + Len uint16
> + Payload []byte
> +}
> +
> +// ExtractARPPacket will extract an ARP packet from the ethernet frame's
> +// payload
> +func (e *EthernetFrame) ExtractARPPacket() (*Packet, error) {
> + var buf []byte
> + if e.EthernetType == EthernetTypeVLAN {
> + buf = e.Payload[4:]
> + } else {
> + buf = e.Payload
> + }
> + a := &Packet{}
> + err := a.UnmarshalBinary(buf)
> + if err != nil {
> + return nil, err
> + }
> + return a, nil
> +}
> +
> +// ExtractVLAN will extract the VLAN tag from the ethernet frame's
> +// payload if one is present and return ErrNotVLAN if not
> +func (e *EthernetFrame) ExtractVLAN() (*VLAN, error) {
> + if e.EthernetType != EthernetTypeVLAN {
> + return nil, ErrNotVLAN
> + }
> + v := &VLAN{}
> + err := v.UnmarshalBinary(e.Payload[0:4])
> + if err != nil {
> + return nil, err
> + }
> + return v, nil
> +}
> +
> +// UnmarshalBinary parses ethernet frame bytes into an EthernetFrame
> +func (eth *EthernetFrame) UnmarshalBinary(buf []byte) error {
> + if len(buf) < minEthernetLen {
> + if len(buf) == 0 {
> + return io.ErrUnexpectedEOF
> + }
> + return ErrMalformedFrame
> + }
> +
> + eth.DstMAC = buf[0:6]
> + eth.SrcMAC = buf[6:12]
> + eth.EthernetType = binary.BigEndian.Uint16(buf[12:14])
> + eth.Payload = buf[14:]
> + if eth.EthernetType < NonStdLenEthernetTypes {
> + // see IEEE 802.3, non-standard ethernet may contain padding
> + // this calculation is used to truncate the payload to the
> length
> + // specified for that ethernet type
> + eth.Len = eth.EthernetType
> + eth.EthernetType = EthernetTypeLLC
> + cmp := len(eth.Payload) - int(eth.Len)
> + if cmp < 0 {
> + return ErrMalformedFrame
> + } else if cmp > 0 {
> + eth.Payload = eth.Payload[:len(eth.Payload)-cmp]
> + }
> + }
> + return nil
> +}
> diff --git a/src/maasagent/internal/netmon/service.go
> b/src/maasagent/internal/netmon/service.go
> index 14ebdc6..8f8ef27 100644
> --- a/src/maasagent/internal/netmon/service.go
> +++ b/src/maasagent/internal/netmon/service.go
> @@ -1,3 +1,251 @@
> package netmon
>
> -func NewService() {}
> +/*
> + Copyright 2023 Canonical Ltd. This software is licensed under the
> + GNU Affero General Public License version 3 (see the file LICENSE).
> +*/
> +
> +import (
> + "bytes"
> + "context"
> + "errors"
> + "fmt"
> + "net"
> + "net/netip"
> + "time"
> +
> + pcap "github.com/packetcap/go-pcap"
> + "github.com/rs/zerolog/log"
> +
> + "launchpad.net/maas/maas/src/maasagent/internal/arp"
> +)
> +
> +const (
> + snapLen int32 = 64
> + timeout time.Duration = -1
> + seenAgainThreshold time.Duration = 600 * time.Second
> +)
> +
> +const (
> + // EventNew is the Event value for a new Result
> + EventNew = "NEW"
> + // EventRefreshed is the Event value for a Result that is for
> + // refreshed ARP values
> + EventRefreshed = "REFRESHED"
> + // EventMoved is the Event value for a Result where the IP has
> + // changed its MAC address
> + EventMoved = "MOVED"
> +)
> +
> +var (
> + // ErrEmptyPacket is returned when a packet of 0 bytes has been received
> + ErrEmptyPacket = errors.New("received an empty packet")
> + // ErrPacketCaptureClosed is returned when the packet capture channel
> + // has been closed unexpectedly
> + ErrPacketCaptureClosed = errors.New("packet capture channel closed")
> +)
> +
> +// Binding represents the binding between an IP address and MAC address
> +type Binding struct {
> + // IP is the IP a binding is tracking
> + IP netip.Addr
> + // MAC is the MAC address the IP is currently bound to
> + MAC net.HardwareAddr
> + // VID is the associated VLAN ID, if one exists
> + VID *uint16
> + // Time is the time the packet creating / updating the binding
> + // was observed
> + Time time.Time
> +}
> +
> +// Result is the result of observed ARP packets
> +type Result struct {
> + // IP is the presentation format of an observed IP
> + IP string `json:"ip"`
> + // MAC is the presentation format of an observed MAC
> + MAC string `json:"mac"`
> + // Previous MAC is the presentation format of a previous MAC if
> + // an EventMoved was observed
> + PreviousMAC string `json:"previous_mac,omitempty"`
> + // Event is the type of event the Result is
> + Event string `json:"event"`
> + // Time is the time the packet creating the Result was observed
> + Time int64 `json:"time"`
> + // VID is the VLAN ID if one exists
> + VID *uint16 `json:"vid"`
> +}
> +
> +// Service is responsible for starting packet capture and
> +// converting observed ARP packets into discovered Results
> +type Service struct {
> + iface string
> + bindings map[string]Binding
> +}
> +
> +// NewService returns a pointer to a Service. It
> +// takes the desired interface to observe's name as an argument
> +func NewService(iface string) *Service {
> + return &Service{
> + iface: iface,
> + bindings: make(map[string]Binding),
> + }
> +}
> +
> +func (s *Service) updateBindings(pkt *arp.Packet, vid *uint16, timestamp
> time.Time) (res []Result) {
> + if timestamp.IsZero() {
> + timestamp = time.Now()
> + }
> +
> + var vidLabel int
> + if vid != nil {
> + vidLabel = int(*vid)
> + }
> +
> + discoveredBindings := []Binding{
> + {
> + IP: pkt.SendIPAddr,
> + MAC: pkt.SendHwdAddr,
> + VID: vid,
> + Time: timestamp,
> + },
> + }
> + if pkt.OpCode == arp.OpReply {
> + discoveredBindings = append(discoveredBindings, Binding{
> + IP: pkt.TgtIPAddr,
> + MAC: pkt.TgtHwdAddr,
> + VID: vid,
> + Time: timestamp,
> + })
> + }
> +
> + for _, discoveredBinding := range discoveredBindings {
It seems that here we iterate over a known slice length.
Can't we use slice initialiser here and have `res []Result` of a known length
instead of having a possible slice re-alloc?
https://git.launchpad.net/maas/tree/go-style-guide.md#n641
> + key := fmt.Sprintf("%d_%s", vidLabel,
> discoveredBinding.IP.String())
> + binding, ok := s.bindings[key]
> + if ok {
> + if bytes.Compare(binding.MAC, discoveredBinding.MAC) !=
> 0 {
You can use `!bytes.Equal` here
> + s.bindings[key] = discoveredBinding
It seems that we have this assignment in all three branches.
Can we just have it one place?
> + res = append(res, Result{
Would it be more readable to have an instance of Result with a common
properties being initialised once?
```
r := Result{
IP: discoveredBinding.IP.String(),
MAC: discoveredBinding.MAC.String(),
Time: discoveredBinding.Time.Unix(),
VID: discoveredBinding.VID,
}
```
And then in each if-else if-else branch fill in the things that are different?
I think it makes it more readable to understand what exactly is the difference.
> + IP:
> discoveredBinding.IP.String(),
> + PreviousMAC: binding.MAC.String(),
> + MAC:
> discoveredBinding.MAC.String(),
> + VID: discoveredBinding.VID,
> + Time:
> discoveredBinding.Time.Unix(),
> + Event: EventMoved,
> + })
> + } else if discoveredBinding.Time.Sub(binding.Time) >=
> seenAgainThreshold {
> + s.bindings[key] = discoveredBinding
> + res = append(res, Result{
> + IP: discoveredBinding.IP.String(),
> + MAC: discoveredBinding.MAC.String(),
> + VID: discoveredBinding.VID,
> + Time: discoveredBinding.Time.Unix(),
> + Event: EventRefreshed,
> + })
> + }
> + } else {
> + s.bindings[key] = discoveredBinding
> + res = append(res, Result{
> + IP: discoveredBinding.IP.String(),
> + MAC: discoveredBinding.MAC.String(),
> + VID: discoveredBinding.VID,
> + Time: discoveredBinding.Time.Unix(),
> + Event: EventNew,
> + })
> + }
> + }
> +
> + return res
> +}
> +
> +func isValidARPPacket(pkt *arp.Packet) bool {
> + if pkt.HardwareType != arp.HardwareTypeEthernet {
> + return false
> + }
> + if pkt.ProtocolType != arp.ProtocolTypeIPv4 {
> + return false
> + }
> + if pkt.HardwareAddrLen != 6 {
> + return false
> + }
> + if pkt.ProtocolAddrLen != 4 {
> + return false
> + }
> + return true
> +}
> +
> +func (s *Service) handlePacket(pkt pcap.Packet) ([]Result, error) {
> + if pkt.Error != nil {
> + return nil, pkt.Error
> + }
> + if len(pkt.B) == 0 {
> + return nil, ErrEmptyPacket
> + }
> + eth := &arp.EthernetFrame{}
> + err := eth.UnmarshalBinary(pkt.B)
> + if err != nil {
> + return nil, err
> + }
> +
> + if eth.EthernetType != arp.EthernetTypeVLAN && eth.EthernetType !=
> arp.EthernetTypeARP {
> + log.Debug().Msg("skipping non-ARP packet")
> + return nil, nil
> + }
> +
> + var vid *uint16
> + if eth.EthernetType == arp.EthernetTypeVLAN {
> + vlan, err := eth.ExtractVLAN()
> + if err != nil {
> + return nil, err
> + }
> + vid = &vlan.ID
> + }
> +
> + arpPkt, err := eth.ExtractARPPacket()
> + if err != nil {
> + return nil, err
> + }
> +
> + if !isValidARPPacket(arpPkt) {
Why we want retrieve ARP packet from `eth.ExtractARPPacket()` and check if it
is valid later? Why cant we return `error` from `eth.ExtractARPPacket` in case
of invalid packet?
> + log.Debug().Msg("skipping non-ethernet+IPv4 ARP packet")
> + return nil, nil
> + }
> + return s.updateBindings(arpPkt, vid, pkt.Info.Timestamp), nil
> +}
> +
> +func isRecoverableError(err error) bool {
> + return errors.Is(err, arp.ErrMalformedPacket) || errors.Is(err,
> arp.ErrMalformedVLAN) || errors.Is(err, arp.ErrMalformedFrame)
> +}
> +
> +// Start will start packet capture and send results to a channel
> +func (s *Service) Start(ctx context.Context, resultC chan<- Result) error {
> + defer close(resultC)
> +
> + hndlr, err := pcap.OpenLive(s.iface, snapLen, false, timeout, true)
> + if err != nil {
> + return err
> + }
> + defer hndlr.Close()
> + pkts := hndlr.Listen()
> + for {
> + select {
> + case <-ctx.Done():
> + return nil
> + case pkt, ok := <-pkts:
> + if !ok {
> + log.Debug().Msg("packet capture has closed")
> + return ErrPacketCaptureClosed
> + }
> + res, err := s.handlePacket(pkt)
> + if err != nil {
> + if isRecoverableError(err) {
> + log.Error().Err(err).Send()
> + continue
> + }
> + return err
> + }
> + for _, r := range res {
> + resultC <- r
> + }
> + }
> + }
> +}
> diff --git a/src/maasagent/internal/netmon/service_test.go
> b/src/maasagent/internal/netmon/service_test.go
> new file mode 100644
> index 0000000..158a651
> --- /dev/null
> +++ b/src/maasagent/internal/netmon/service_test.go
> @@ -0,0 +1,382 @@
> +package netmon
> +
> +/*
> + Copyright 2023 Canonical Ltd. This software is licensed under the
> + GNU Affero General Public License version 3 (see the file LICENSE).
> +*/
> +
> +import (
> + "net"
> + "net/netip"
> + "testing"
> + "time"
> +
> + "github.com/google/gopacket"
> + pcap "github.com/packetcap/go-pcap"
> + "github.com/stretchr/testify/assert"
> +
> + "launchpad.net/maas/maas/src/maasagent/internal/arp"
> +)
> +
> +func uint16Pointer(v uint16) *uint16 {
> + return &v
> +}
> +
> +type isValidARPPacketCase struct {
> + Name string
> + In *arp.Packet
> + Out bool
> +}
> +
> +func TestIsValidARPPacket(t *testing.T) {
> + table := []isValidARPPacketCase{
I am wondering why you decided to define a type for a testCase here and not use
approach defined on the style guide?
https://git.launchpad.net/maas/tree/go-style-guide.md#n722
> + {
> + Name: "ValidARPPacket",
> + In: &arp.Packet{
> + HardwareType: arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + },
> + Out: true,
> + },
> + {
> + Name: "InvalidHardwareTypeARPPacket",
> + In: &arp.Packet{
> + HardwareType: arp.HardwareTypeChaos,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + },
> + Out: false,
> + },
> + {
> + Name: "InvalidProtocolTypeARPPacket",
> + In: &arp.Packet{
> + HardwareType: arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv6,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + },
> + Out: false,
> + },
> + {
> + Name: "InvalidHardwareAddrLenARPPacket",
> + In: &arp.Packet{
> + HardwareType: arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 8,
> + ProtocolAddrLen: 4,
> + },
> + Out: false,
> + },
> + {
> + Name: "InvalidProtocolAddrLenARPPacket",
> + In: &arp.Packet{
> + HardwareType: arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 16,
> + },
> + Out: false,
> + },
> + }
> + for _, tcase := range table {
> + t.Run(tcase.Name, func(tt *testing.T) {
> + assert.Equalf(tt, tcase.Out,
> isValidARPPacket(tcase.In), "expected the result to be %v", tcase.Out)
> + })
> + }
> +}
> +
> +type updateBindingsArgs struct {
> + Pkt *arp.Packet
> + VID *uint16
> + Time time.Time
> +}
> +
> +type updateBindingsCase struct {
> + Name string
> + BindingsFixture map[string]Binding
> + In updateBindingsArgs
> + Out []Result
> +}
> +
> +func TestUpdateBindings(t *testing.T) {
> + timestamp := time.Now()
> + testIP1 := net.ParseIP("10.0.0.1").To4()
> + testIP2 := net.ParseIP("10.0.0.2").To4()
> + table := []updateBindingsCase{
> + {
> + Name: "NewRequestPacket",
> + In: updateBindingsArgs{
> + Pkt: &arp.Packet{
> + HardwareType:
> arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + OpCode: arp.OpRequest,
> + SendHwdAddr: net.HardwareAddr{0xc0,
> 0xff, 0xee, 0x15, 0xc0, 0x01},
> + SendIPAddr:
> netip.AddrFrom4([4]byte{testIP1[0], testIP1[1], testIP1[2], testIP1[3]}),
> + TgtIPAddr:
> netip.AddrFrom4([4]byte{testIP2[0], testIP2[1], testIP2[2], testIP2[3]}),
> + },
> + Time: timestamp,
> + },
> + Out: []Result{
> + {
> + IP: "10.0.0.1",
> + MAC: "c0:ff:ee:15:c0:01",
> + Time: timestamp.Unix(),
> + Event: EventNew,
> + },
> + },
> + },
> + {
> + Name: "NewReplyPacket",
> + In: updateBindingsArgs{
> + Pkt: &arp.Packet{
> + HardwareType:
> arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + OpCode: arp.OpReply,
> + SendHwdAddr: net.HardwareAddr{0xc0,
> 0xff, 0xee, 0x15, 0xc0, 0x01},
> + SendIPAddr:
> netip.AddrFrom4([4]byte{testIP1[0], testIP1[1], testIP1[2], testIP1[3]}),
> + TgtHwdAddr: net.HardwareAddr{0xc0,
> 0xff, 0xee, 0x15, 0xc0, 0x1d},
> + TgtIPAddr:
> netip.AddrFrom4([4]byte{testIP2[0], testIP2[1], testIP2[2], testIP2[3]}),
> + },
> + Time: timestamp,
> + },
> + Out: []Result{
> + {
> + IP: "10.0.0.1",
> + MAC: "c0:ff:ee:15:c0:01",
> + Time: timestamp.Unix(),
> + Event: EventNew,
> + },
> + {
> + IP: "10.0.0.2",
> + MAC: "c0:ff:ee:15:c0:1d",
> + Time: timestamp.Unix(),
> + Event: EventNew,
> + },
> + },
> + },
> + {
> + Name: "NewVLANPacket",
> + In: updateBindingsArgs{
> + Pkt: &arp.Packet{
> + HardwareType:
> arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + OpCode: arp.OpRequest,
> + SendHwdAddr: net.HardwareAddr{0xc0,
> 0xff, 0xee, 0x15, 0xc0, 0x01},
> + SendIPAddr:
> netip.AddrFrom4([4]byte{testIP1[0], testIP1[1], testIP1[2], testIP1[3]}),
> + TgtIPAddr:
> netip.AddrFrom4([4]byte{testIP2[0], testIP2[1], testIP2[2], testIP2[3]}),
> + },
> + VID: uint16Pointer(2),
> + Time: timestamp,
> + },
> + Out: []Result{
> + {
> + IP: "10.0.0.1",
> + MAC: "c0:ff:ee:15:c0:01",
> + Time: timestamp.Unix(),
> + VID: uint16Pointer(2),
> + Event: EventNew,
> + },
> + },
> + },
> + {
> + Name: "Refresh",
> + BindingsFixture: map[string]Binding{
> + "0_10.0.0.1": Binding{
> + IP:
> netip.AddrFrom4([4]byte{testIP1[0], testIP1[1], testIP1[2], testIP1[3]}),
> + MAC: net.HardwareAddr{0xc0, 0xff,
> 0xee, 0x15, 0xc0, 0x01},
> + Time: timestamp,
> + },
> + },
> + In: updateBindingsArgs{
> + Pkt: &arp.Packet{
> + HardwareType:
> arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + OpCode: arp.OpRequest,
> + SendHwdAddr: net.HardwareAddr{0xc0,
> 0xff, 0xee, 0x15, 0xc0, 0x01},
> + SendIPAddr:
> netip.AddrFrom4([4]byte{testIP1[0], testIP1[1], testIP1[2], testIP1[3]}),
> + TgtIPAddr:
> netip.AddrFrom4([4]byte{testIP2[0], testIP2[1], testIP2[2], testIP2[3]}),
> + },
> + Time: timestamp.Add(seenAgainThreshold +
> time.Second),
> + },
> + Out: []Result{
> + {
> + IP: "10.0.0.1",
> + MAC: "c0:ff:ee:15:c0:01",
> + Time: timestamp.Add(seenAgainThreshold
> + time.Second).Unix(),
> + Event: EventRefreshed,
> + },
> + },
> + },
> + {
> + Name: "Move",
> + BindingsFixture: map[string]Binding{
> + "0_10.0.0.1": Binding{
> + IP:
> netip.AddrFrom4([4]byte{testIP1[0], testIP1[1], testIP1[2], testIP1[3]}),
> + MAC: net.HardwareAddr{0xc0, 0xff,
> 0xee, 0x15, 0xc0, 0x01},
> + Time: timestamp,
> + },
> + },
> + In: updateBindingsArgs{
> + Pkt: &arp.Packet{
> + HardwareType:
> arp.HardwareTypeEthernet,
> + ProtocolType: arp.ProtocolTypeIPv4,
> + HardwareAddrLen: 6,
> + ProtocolAddrLen: 4,
> + OpCode: arp.OpRequest,
> + SendHwdAddr: net.HardwareAddr{0xc0,
> 0xff, 0xee, 0x15, 0xc0, 0x1d},
> + SendIPAddr:
> netip.AddrFrom4([4]byte{testIP1[0], testIP1[1], testIP1[2], testIP1[3]}),
> + TgtIPAddr:
> netip.AddrFrom4([4]byte{testIP2[0], testIP2[1], testIP2[2], testIP2[3]}),
> + },
> + Time: timestamp,
> + },
> + Out: []Result{
> + {
> + IP: "10.0.0.1",
> + MAC: "c0:ff:ee:15:c0:1d",
> + Time: timestamp.Unix(),
> + Event: EventMoved,
> + },
> + },
> + },
> + }
> + for _, tcase := range table {
> + t.Run(tcase.Name, func(tt *testing.T) {
> + svc := NewService("lo")
> + if tcase.BindingsFixture != nil {
> + svc.bindings = tcase.BindingsFixture
> + }
> + res := svc.updateBindings(tcase.In.Pkt, tcase.In.VID,
> tcase.In.Time)
> + for i, expected := range tcase.Out {
> + var expectedVID int
> + if expected.VID != nil {
> + expectedVID = int(*expected.VID)
> + }
> + assert.Equalf(tt, expected.IP, res[i].IP,
> "expected Result at index of %d to have the IP %s", i, expected.IP)
> + assert.Equalf(tt, expected.MAC, res[i].MAC,
> "expected Result at index of %d to have the MAC %s", i, expected.MAC)
> + assert.Equalf(tt, expected.VID, res[i].VID,
> "expected Result at index of %d to have the VID %d", i, expectedVID)
> + assert.Equalf(tt, expected.Time, res[i].Time,
> "expected Result at index of %d to have the Time of %d", i,
> int(expected.Time))
> + assert.Equalf(tt, expected.Event, res[i].Event,
> "expected Result at index of %d to have the Event of %s", i, expected.Event)
> + }
> + })
> + }
> +}
> +
> +type handlePacketCase struct {
> + Name string
> + In pcap.Packet
> + Out []Result
> + Err error
> +}
> +
> +func TestServiceHandlePacket(t *testing.T) {
> + timestamp := time.Now()
> + table := []handlePacketCase{
> + {
> + Name: "ValidRequestPacket",
> + In: pcap.Packet{
> + // generated from tcpdump
> + B: []byte{
> + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> 0x84, 0x39, 0xc0, 0x0b, 0x22, 0x25, 0x81, 0x00, 0x00, 0x02,
> + 0x08, 0x06, 0x00, 0x01, 0x08, 0x00,
> 0x06, 0x04, 0x00, 0x01, 0x84, 0x39, 0xc0, 0x0b, 0x22, 0x25,
> + 0xc0, 0xa8, 0x0a, 0x1a, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0xc0, 0xa8, 0x0a, 0x19, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + },
> + Info: gopacket.CaptureInfo{
> + Timestamp: timestamp,
> + },
> + },
> + Out: []Result{
> + {
> + IP: "192.168.10.26",
> + MAC: "84:39:c0:0b:22:25",
> + VID: uint16Pointer(2),
> + Time: timestamp.Unix(),
> + Event: EventNew,
> + },
> + },
> + },
> + {
> + Name: "ValidReplyPacket",
> + In: pcap.Packet{
> + B: []byte{
> + 0x24, 0x4b, 0xfe, 0xe1, 0xea, 0x26,
> 0x80, 0x61, 0x5f, 0x08, 0xfc, 0x16, 0x08, 0x06, 0x00, 0x01,
> + 0x08, 0x00, 0x06, 0x04, 0x00, 0x02,
> 0x80, 0x61, 0x5f, 0x08, 0xfc, 0x16, 0xc0, 0xa8, 0x01, 0x6c,
> + 0x24, 0x4b, 0xfe, 0xe1, 0xea, 0x26,
> 0xc0, 0xa8, 0x01, 0x50,
> + },
> + },
> + Out: []Result{
> + {
> + IP: "192.168.1.108",
> + MAC: "80:61:5f:08:fc:16",
> + VID: nil,
> + Time: timestamp.Unix(),
> + Event: EventNew,
> + },
> + {
> + IP: "192.168.1.80",
> + MAC: "24:4b:fe:e1:ea:26",
> + VID: nil,
> + Time: timestamp.Unix(),
> + Event: EventNew,
> + },
> + },
> + },
> + {
> + Name: "EmptyPacket",
> + Err: ErrEmptyPacket,
> + },
> + {
> + Name: "MalformedPacket",
> + In: pcap.Packet{
> + B: []byte{
> + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> 0x84, 0x39, 0xc0, 0x0b, 0x22,
> + 0x08, 0x06, 0x00, 0x01, 0x08, 0x06,
> 0x04, 0x00, 0x01, 0x84,
> + 0xc0, 0xa8, 0x0a, 0x1a, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0xc0,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00,
> + },
> + },
> + // should return nil, nil
> + },
> + {
> + Name: "ShortPacket",
> + In: pcap.Packet{
> + B: []byte{
> + 0x24, 0x4b, 0xfe, 0xe1, 0xea, 0x26,
> 0x80, 0x61, 0x5f, 0x08, 0xfc, 0x16, 0x08, 0x06, 0x00, 0x01,
> + 0x08, 0x00, 0x06, 0x04, 0x00, 0x02,
> 0x80, 0xfc, 0x16, 0xc0, 0xa8, 0x01, 0x6c,
> + 0x24, 0x4b, 0xfe, 0xe1, 0xea, 0x26,
> 0xc0, 0xa8, 0x01, 0x50,
> + },
> + },
> + Err: arp.ErrMalformedPacket,
> + },
> + }
> +
> + svc := NewService("")
> +
> + for _, tcase := range table {
> + t.Run(tcase.Name, func(tt *testing.T) {
> + res, err := svc.handlePacket(tcase.In)
> + assert.ErrorIsf(tt, err, tcase.Err, "expected
> handlePacket to return an error of: %s", tcase.Err)
> + if tcase.Out != nil {
> + for i, expected := range tcase.Out {
> + assert.Equalf(tt, expected.IP,
> res[i].IP, "expected result at index %d to have an IP address of %s", i,
> expected.IP)
> + assert.Equalf(tt, expected.MAC,
> res[i].MAC, "expected result at index %d to have a MAC address of %s", i,
> expected.MAC)
> + assert.Equalf(tt, expected.VID,
> res[i].VID, "expected result at index %d to have a VID of %v", i,
> expected.VID)
> + assert.Equalf(tt, expected.Time,
> res[i].Time, "expected result at index %d to have a Time of %s", i,
> expected.Time)
> + }
> + } else {
> + assert.Nil(tt, res)
> + }
> + })
> + }
> +}
--
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/441702
Your team MAAS Committers is subscribed to branch maas:master.
--
Mailing list: https://launchpad.net/~sts-sponsors
Post to : sts-sponsors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~sts-sponsors
More help : https://help.launchpad.net/ListHelp
