Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add unit test for ListIPForwardRows #6673

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add unit test for ListIPForwardRows #6673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions pkg/agent/util/syscall/syscall_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,14 @@ type NetIOInterface interface {
}

type netIO struct {
syscallN func(trap uintptr, args ...uintptr) (r1, r2 uintptr, err syscall.Errno)
syscallN func(trap uintptr, args ...uintptr) (r1, r2 uintptr, err syscall.Errno)
getIPForwardTableFn func(family uint16, ipForwardTable **MibIPForwardTable) (errcode error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't we just fake syscallN in tests, instead of faking individual functions like getIPForwardTable? It seems that getIPForwardTable is just a thin wrapper around a syscallN call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I did in an earlier version: b2e1f7a. However, checkptr failed when --race is enabled, as I commented in #6673 (comment). The error can be found here: https://github.com/antrea-io/antrea/actions/runs/10863106542/job/30146883490.

I suspect the compiler doesn't recognize the fake syscall and mistakenly raise the error. But even the latest version failed due to another checkptr error in code (as opposed the previous error in test code), and this error seems valid: the code shouldn't assign uintptr to a variable.

}

func NewNetIO() NetIOInterface {
return &netIO{syscallN: syscall.SyscallN}
io := &netIO{syscallN: syscall.SyscallN}
io.getIPForwardTableFn = io.getIPForwardTable
return io
}

func (n *netIO) GetIPInterfaceEntry(ipInterfaceRow *MibIPInterfaceRow) (errcode error) {
Expand Down Expand Up @@ -361,7 +364,7 @@ func (n *netIO) getIPForwardTable(family uint16, ipForwardTable **MibIPForwardTa

func (n *netIO) ListIPForwardRows(family uint16) ([]MibIPForwardRow, error) {
var table *MibIPForwardTable
err := n.getIPForwardTable(family, &table)
err := n.getIPForwardTableFn(family, &table)
if table != nil {
defer n.freeMibTable(unsafe.Pointer(table))
}
Expand Down
102 changes: 97 additions & 5 deletions pkg/agent/util/syscall/syscall_windows_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ import (
"os"
"syscall"
"testing"
"unsafe"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRawSockAddrTranslation(t *testing.T) {
Expand Down Expand Up @@ -202,18 +204,108 @@ func TestIPForwardEntryOperations(t *testing.T) {
}
}

func TestListIPForwardRows(t *testing.T) {
func TestListIPForwardRowsFailure(t *testing.T) {
wantErr := os.NewSyscallError("iphlpapi.GetIpForwardTable", syscall.Errno(22))
testNetIO := NewTestNetIO(22)
// Skipping no error case because converting uintptr back to Pointer is not valid in general.
gotRow, gotErr := testNetIO.ListIPForwardRows(AF_INET)
assert.Nil(t, gotRow)
gotRows, gotErr := testNetIO.ListIPForwardRows(AF_INET)
assert.Nil(t, gotRows)
assert.Equal(t, wantErr, gotErr)
}

func NewTestNetIO(wantR1 uintptr) NetIOInterface {
func TestListIPForwardRowsSuccess(t *testing.T) {
// The table contains two rows. Its memory address will be assigned to ipForwardTable when getIPForwardTableFn is called.
table := struct {
NumEntries uint32
Table [2]MibIPForwardRow
}{
NumEntries: 2,
Table: [2]MibIPForwardRow{
{
Luid: 10,
Index: 11,
DestinationPrefix: AddressPrefix{
Prefix: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{10, 10, 10, 0},
},
prefixLength: 24,
},
NextHop: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{11, 11, 11, 11},
},
},
{
Luid: 20,
Index: 21,
DestinationPrefix: AddressPrefix{
Prefix: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{20, 20, 20, 0},
},
prefixLength: 24,
},
NextHop: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{21, 21, 21, 21},
},
},
},
}
testNetIO := NewTestNetIO(0)
testNetIO.getIPForwardTableFn = func(family uint16, ipForwardTable **MibIPForwardTable) (errcode error) {
*ipForwardTable = (*MibIPForwardTable)(unsafe.Pointer(&table))
return nil
}

gotRows, gotErr := testNetIO.ListIPForwardRows(AF_INET)
require.Nil(t, gotErr)

// It verifies that the returned rows are independent copies, not referencing to the original table's memory, by
// asserting they retain the exact same content as the original table after it has been set.
table.Table[0] = MibIPForwardRow{}
table.Table[1] = MibIPForwardRow{}
expectedRows := []MibIPForwardRow{
{
Luid: 10,
Index: 11,
DestinationPrefix: AddressPrefix{
Prefix: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{10, 10, 10, 0},
},
prefixLength: 24,
},
NextHop: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{11, 11, 11, 11},
},
},
{
Luid: 20,
Index: 21,
DestinationPrefix: AddressPrefix{
Prefix: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{20, 20, 20, 0},
},
prefixLength: 24,
},
NextHop: RawSockAddrInet{
Family: AF_INET,
data: [26]byte{21, 21, 21, 21},
},
},
}
assert.Equal(t, expectedRows, gotRows)
}

func NewTestNetIO(wantR1 uintptr) *netIO {
mockSyscallN := func(trap uintptr, args ...uintptr) (r1, r2 uintptr, err syscall.Errno) {
return wantR1, 0, 0
}
return &netIO{syscallN: mockSyscallN}
io := &netIO{syscallN: mockSyscallN}
io.getIPForwardTableFn = io.getIPForwardTable
return io
}
Loading