From 8a4682ce3387000f3795c5b4510ae9a7c7c07e13 Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Tue, 11 Jun 2024 12:56:02 -0700 Subject: [PATCH] Add fake implementation for net.PacketConn interface (#6419) In the pkg/agent/util/nettest/ package. The ARPResponder unit tests are updated to use this new fake. It will also be useful when writing unit tests for the NodeLatencyMonitor. Signed-off-by: Antonin Bas --- .../responder/arp_responder_test.go | 176 +++++++----------- .../responder/ndp_responder_test.go | 15 +- pkg/agent/util/nettest/packetconn.go | 160 ++++++++++++++++ 3 files changed, 240 insertions(+), 111 deletions(-) create mode 100644 pkg/agent/util/nettest/packetconn.go diff --git a/pkg/agent/ipassigner/responder/arp_responder_test.go b/pkg/agent/ipassigner/responder/arp_responder_test.go index 748b00658f9..fcb05a8bb9a 100644 --- a/pkg/agent/ipassigner/responder/arp_responder_test.go +++ b/pkg/agent/ipassigner/responder/arp_responder_test.go @@ -15,10 +15,8 @@ package responder import ( - "bytes" "net" "testing" - "time" "github.com/mdlayher/arp" "github.com/mdlayher/ethernet" @@ -26,143 +24,111 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/sets" -) - -type fakePacketConn struct { - addr packet.Addr - buffer *bytes.Buffer -} - -var _ net.PacketConn = (*fakePacketConn)(nil) - -func (pc *fakePacketConn) ReadFrom(p []byte) (int, net.Addr, error) { - n, err := pc.buffer.Read(p) - return n, &pc.addr, err -} - -func (pc *fakePacketConn) WriteTo(p []byte, addr net.Addr) (int, error) { - return pc.buffer.Write(p) -} - -func (pc *fakePacketConn) Close() error { - return nil -} - -func (pc *fakePacketConn) LocalAddr() net.Addr { - return &pc.addr -} - -func (pc *fakePacketConn) SetDeadline(t time.Time) error { - return nil -} - -func (pc *fakePacketConn) SetReadDeadline(t time.Time) error { - return nil -} -func (pc *fakePacketConn) SetWriteDeadline(t time.Time) error { - return nil -} + "antrea.io/antrea/pkg/agent/util/nettest" +) -func newFakeARPClient(iface *net.Interface, conn *fakePacketConn) (*arp.Client, error) { +func newFakeARPClient(iface *net.Interface, conn *nettest.PacketConn) (*arp.Client, error) { return arp.New(iface, conn) } -func newFakeNetworkInterface() *net.Interface { +func newFakeNetworkInterface(hardwareAddr []byte) *net.Interface { return &net.Interface{ Index: 0, MTU: 1500, Name: "eth0", - HardwareAddr: []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55}, + HardwareAddr: hardwareAddr, + } +} + +func getEthernetForARPPacket(p *arp.Packet, addr net.HardwareAddr) []byte { + pb, _ := p.MarshalBinary() + f := ðernet.Frame{ + Destination: addr, + Source: p.SenderHardwareAddr, + EtherType: ethernet.EtherTypeARP, + Payload: pb, } + fb, _ := f.MarshalBinary() + return fb } func TestARPResponder_HandleARPRequest(t *testing.T) { + // The "local" endpoint is the one running the ARPRespondder. + // The "remote" endpoint is the one sending ARP requests to the "local" endpoint. + localHWAddr := net.HardwareAddr{0x00, 0x01, 0x02, 0x03, 0x04, 0x05} + remoteHWAddr := net.HardwareAddr{0x00, 0x11, 0x22, 0x33, 0x44, 0x55} + localIP := net.ParseIP("192.168.10.1") + remoteIP := net.ParseIP("192.168.10.2") + tests := []struct { - name string - iface *net.Interface - arpOperation arp.Operation - srcHWAddr, dstHWAddr net.HardwareAddr - srcIP, dstIP net.IP - assignedIPs []net.IP - expectError bool - expectedBytes []byte + name string + assignedIPs []net.IP + replyExpected bool }{ { - name: "Response for assigned IP", - iface: newFakeNetworkInterface(), - arpOperation: arp.OperationRequest, - srcHWAddr: net.HardwareAddr{0x00, 0x01, 0x02, 0x03, 0x04, 0x05}, - dstHWAddr: ethernet.Broadcast, - srcIP: net.ParseIP("192.168.10.2"), - dstIP: net.ParseIP("192.168.10.1"), - assignedIPs: []net.IP{net.ParseIP("192.168.10.1")}, - expectError: false, - expectedBytes: []byte{ - // ethernet header (16 bytes) - 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, // 6 bytes: destination hardware address - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, // 6 bytes: source hardware address - 0x08, 0x06, // 2 bytes: ethernet type - // arp payload (46 bytes) - 0x00, 0x01, // 2 bytes: hardware type - 0x08, 0x00, // 2 bytes: protocol type - 0x06, // 1 byte : hardware address length - 0x04, // 1 byte : protocol length - 0x00, 0x02, // 2 bytes: operation - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, // 6 bytes: source hardware address - 0xc0, 0xa8, 0x0a, 0x01, // 4 bytes: source protocol address - 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, // 6 bytes: target hardware address - 0xc0, 0xa8, 0x0a, 0x02, // 4 bytes: target protocol address - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 18 bytes: padding - }, + name: "Response for assigned IP", + assignedIPs: []net.IP{localIP}, + replyExpected: true, }, { name: "Response for not assigned IP", - iface: newFakeNetworkInterface(), - arpOperation: arp.OperationRequest, - srcHWAddr: net.HardwareAddr{0x00, 0x01, 0x02, 0x03, 0x04, 0x05}, - dstHWAddr: ethernet.Broadcast, - srcIP: net.ParseIP("192.168.10.2"), - dstIP: net.ParseIP("192.168.10.3"), - assignedIPs: []net.IP{net.ParseIP("192.168.10.1")}, - expectError: false, - expectedBytes: []byte{}, + assignedIPs: []net.IP{net.ParseIP("192.168.10.3")}, + replyExpected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - conn := &fakePacketConn{ - buffer: bytes.NewBuffer(nil), - addr: packet.Addr{ - HardwareAddr: tt.iface.HardwareAddr, - }, + localIface := newFakeNetworkInterface(localHWAddr) + remoteIface := newFakeNetworkInterface(remoteHWAddr) + localAddr := &packet.Addr{ + HardwareAddr: localHWAddr, } - fakeARPClient, err := newFakeARPClient(tt.iface, conn) + remoteAddr := &packet.Addr{ + HardwareAddr: remoteHWAddr, + } + localConn, remoteConn := nettest.PacketConnPipe(localAddr, remoteAddr, 1) + localARPClient, err := newFakeARPClient(localIface, localConn) + require.NoError(t, err) + remoteARPClient, err := newFakeARPClient(remoteIface, remoteConn) require.NoError(t, err) - packet, err := arp.NewPacket(tt.arpOperation, tt.srcHWAddr, tt.srcIP, tt.dstHWAddr, tt.dstIP) + request, err := arp.NewPacket(arp.OperationRequest, remoteHWAddr, remoteIP, ethernet.Broadcast, localIP) require.NoError(t, err) - err = fakeARPClient.WriteTo(packet, tt.dstHWAddr) + expectedReply, err := arp.NewPacket(arp.OperationReply, localHWAddr, localIP, remoteHWAddr, remoteIP) require.NoError(t, err) + expectedBytes := getEthernetForARPPacket(expectedReply, remoteHWAddr) + require.NoError(t, remoteARPClient.WriteTo(request, localAddr.HardwareAddr)) assignedIPs := sets.New[string]() for _, ip := range tt.assignedIPs { assignedIPs.Insert(ip.String()) } r := arpResponder{ - iface: tt.iface, - conn: fakeARPClient, + iface: localIface, + conn: localARPClient, assignedIPs: sets.New[string](), } for _, ip := range tt.assignedIPs { r.AddIP(ip) } err = r.handleARPRequest() - assert.NoError(t, err) - assert.Equal(t, tt.expectedBytes, conn.buffer.Bytes()) + require.NoError(t, err) + // We cannot use remoteARPClient.ReadFrom as it is blocking. + replyB, addr, err := remoteConn.Receive() + if !tt.replyExpected { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, remoteAddr, addr) + assert.Equal(t, expectedBytes, replyB) + } }) } } func Test_arpResponder_addIP(t *testing.T) { + hwAddr := []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55} + iface := newFakeNetworkInterface(hwAddr) + tests := []struct { name string ip net.IP @@ -193,19 +159,10 @@ func Test_arpResponder_addIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &arpResponder{ - iface: newFakeNetworkInterface(), + iface: iface, assignedIPs: tt.assignedIPs, } - conn := &fakePacketConn{ - buffer: bytes.NewBuffer(nil), - addr: packet.Addr{ - HardwareAddr: r.iface.HardwareAddr, - }, - } - fakeARPClient, err := newFakeARPClient(r.iface, conn) - require.NoError(t, err) - r.conn = fakeARPClient - err = r.AddIP(tt.ip) + err := r.AddIP(tt.ip) if tt.expectedError { assert.Error(t, err) } else { @@ -217,6 +174,9 @@ func Test_arpResponder_addIP(t *testing.T) { } func Test_arpResponder_removeIP(t *testing.T) { + hwAddr := []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55} + iface := newFakeNetworkInterface(hwAddr) + tests := []struct { name string ip net.IP @@ -247,7 +207,7 @@ func Test_arpResponder_removeIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &arpResponder{ - iface: newFakeNetworkInterface(), + iface: iface, assignedIPs: tt.assignedIPs, } err := r.RemoveIP(tt.ip) diff --git a/pkg/agent/ipassigner/responder/ndp_responder_test.go b/pkg/agent/ipassigner/responder/ndp_responder_test.go index 21b0c329d95..5d2f66d4c14 100644 --- a/pkg/agent/ipassigner/responder/ndp_responder_test.go +++ b/pkg/agent/ipassigner/responder/ndp_responder_test.go @@ -53,6 +53,9 @@ func (c *fakeNDPConn) LeaveGroup(ip net.IP) error { } func TestNDPResponder_handleNeighborSolicitation(t *testing.T) { + hwAddr := []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55} + iface := newFakeNetworkInterface(hwAddr) + tests := []struct { name string requestMessage []byte @@ -131,7 +134,7 @@ func TestNDPResponder_handleNeighborSolicitation(t *testing.T) { assignedIPs.Insert(ip.String()) } responder := &ndpResponder{ - iface: newFakeNetworkInterface(), + iface: iface, conn: fakeConn, assignedIPs: sets.New[string](), } @@ -185,6 +188,9 @@ func Test_parseIPv6SolicitedNodeMulticastAddress(t *testing.T) { } func Test_ndpResponder_addIP(t *testing.T) { + hwAddr := []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55} + iface := newFakeNetworkInterface(hwAddr) + tests := []struct { name string ip net.IP @@ -249,7 +255,7 @@ func Test_ndpResponder_addIP(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var joinedGroup, leftGroup []net.IP r := &ndpResponder{ - iface: newFakeNetworkInterface(), + iface: iface, conn: &fakeNDPConn{ joinGroup: func(ip net.IP) error { joinedGroup = append(joinedGroup, ip) @@ -281,6 +287,9 @@ func Test_ndpResponder_addIP(t *testing.T) { } func Test_ndpResponder_removeIP(t *testing.T) { + hwAddr := []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55} + iface := newFakeNetworkInterface(hwAddr) + tests := []struct { name string ip net.IP @@ -352,7 +361,7 @@ func Test_ndpResponder_removeIP(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var joinedGroup, leftGroup []net.IP r := &ndpResponder{ - iface: newFakeNetworkInterface(), + iface: iface, conn: &fakeNDPConn{ joinGroup: func(ip net.IP) error { joinedGroup = append(joinedGroup, ip) diff --git a/pkg/agent/util/nettest/packetconn.go b/pkg/agent/util/nettest/packetconn.go new file mode 100644 index 00000000000..f4ba56f4be7 --- /dev/null +++ b/pkg/agent/util/nettest/packetconn.go @@ -0,0 +1,160 @@ +// Copyright 2024 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package nettest + +import ( + "fmt" + "net" + "time" +) + +type Packet struct { + Bytes []byte + Addr net.Addr +} + +// PacketConn implements the net.PacketConn interface, and can be used to test networking code that +// requires a packet-oriented network connection. The connection peer is represented by 2 channels +// to send and receive packets. +type PacketConn struct { + addr net.Addr + inCh chan *Packet + outCh chan *Packet + closeCh chan struct{} +} + +var _ net.PacketConn = (*PacketConn)(nil) + +func NewPacketConn(localAddr net.Addr, inCh chan *Packet, outCh chan *Packet) *PacketConn { + return &PacketConn{ + addr: localAddr, + inCh: inCh, + outCh: outCh, + closeCh: make(chan struct{}), + } +} + +func (pc *PacketConn) ReadFrom(p []byte) (int, net.Addr, error) { + // Check if connection is closed once before the select statement. Otherwise we may end up + // reading a packet even if the connection was already closed before calling this + // function. It is still possible for the connection to be closed between this check and the + // select, but it doesn't matter in this case because that would mean the 2 function calls + // (Close and ReadFrom) are concurrent. + if pc.isClosed() { + return 0, nil, pc.closedConnectionError("read") + } + select { + case <-pc.closeCh: + return 0, nil, pc.closedConnectionError("read") + case packet := <-pc.inCh: + n := copy(p, packet.Bytes) + return n, packet.Addr, nil + } +} + +func (pc *PacketConn) WriteTo(p []byte, addr net.Addr) (int, error) { + // See the comment in ReadFrom. + if pc.isClosed() { + return 0, pc.closedConnectionError("write") + } + packet := &Packet{ + Bytes: make([]byte, len(p)), + Addr: addr, + } + n := copy(packet.Bytes, p) + select { + case <-pc.closeCh: + return 0, pc.closedConnectionError("write") + case pc.outCh <- packet: + return n, nil + } +} + +func (pc *PacketConn) Close() error { + // panic if the connection has already been closed + close(pc.closeCh) + return nil +} + +func (pc *PacketConn) LocalAddr() net.Addr { + return pc.addr +} + +func (pc *PacketConn) SetDeadline(t time.Time) error { + return fmt.Errorf("not implemented") +} + +func (pc *PacketConn) SetReadDeadline(t time.Time) error { + return fmt.Errorf("not implemented") +} + +func (pc *PacketConn) SetWriteDeadline(t time.Time) error { + return fmt.Errorf("not implemented") +} + +// Send is a convenience function that will send a packet without blocking. If this is not possible, +// it will return an error. Send does not check whether the connection is closed. +func (pc *PacketConn) Send(p []byte, addr net.Addr) (int, error) { + packet := &Packet{ + Bytes: make([]byte, len(p)), + Addr: addr, + } + n := copy(packet.Bytes, p) + select { + case pc.outCh <- packet: + return n, nil + default: + return 0, fmt.Errorf("cannot send packet") + } +} + +// Receive is a convenience function that will receive a packet without blocking. If no packet is +// available, it will return an error. Receive does not check whether the connection is closed. +func (pc *PacketConn) Receive() ([]byte, net.Addr, error) { + select { + case packet := <-pc.inCh: + return packet.Bytes, packet.Addr, nil + default: + return nil, nil, fmt.Errorf("no packet available") + } +} + +func (pc *PacketConn) isClosed() bool { + select { + case <-pc.closeCh: + return true + default: + return false + } +} + +func (pc *PacketConn) closedConnectionError(op string) error { + return &net.OpError{ + Op: op, + Net: pc.addr.Network(), + Source: pc.addr, + Addr: nil, + Err: fmt.Errorf("connection is closed"), + } +} + +// PacketConnPipe creates 2 instances of PacketConn which represent the 2 endpoints of a +// packet-oriented connection. Every packet sent on one side will be received on the other side, +// and we never check whether the address actually matches when sending a packet. +func PacketConnPipe(addr1, addr2 net.Addr, capacity int) (*PacketConn, *PacketConn) { + ch1 := make(chan *Packet, capacity) + ch2 := make(chan *Packet, capacity) + return NewPacketConn(addr1, ch1, ch2), NewPacketConn(addr2, ch2, ch1) +}