netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Minor refactor of constructor for comparison operator.

Browse Source 
Justification: Since the comparison operator is only valid for bytesData, the
constructor should recieve a byte slice as input and then convert it to
registerData. It makes less sense to have the caller create the registerData
from the byte slice if bytes data is the only type of registerData, we accept.

This change removes unnecessary checks and makes the code a lot less wordy by
removing the profuse amount of newBytesData calls for comparison construction.

PiperOrigin-RevId: 670652077
This commit is contained in:
Jayden Nyamiaka
2024-09-03 12:08:01 -07:00
committed by gVisor bot
parent 57902f651c
commit 341a018fd3
4 changed files with 145 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/tcpip/nftables/nftables.go
+6 -5
View File
@@ -638,18 +638,19 @@ func validateComparisonOp(cop cmpOp) error {
}
// newComparison creates a new Comparison operation.
func newComparison(sreg uint8, op int, data registerData) (*comparison, error) {
func newComparison(sreg uint8, op int, data []byte) (*comparison, error) {
if sreg == linux.NFT_REG_VERDICT {
return nil, fmt.Errorf("comparison operation cannot use verdict register as source")
}
if err := data.validateRegister(sreg); err != nil {
bytesData := newBytesData(data)
if err := bytesData.validateRegister(sreg); err != nil {
return nil, err
}
cop := cmpOp(op)
if err := validateComparisonOp(cop); err != nil {
return nil, err
}
return &comparison{sreg: sreg, cop: cop, data: data}, nil
return &comparison{sreg: sreg, cop: cop, data: bytesData}, nil
}
// evaluate for Comparison compares the data in the source register to the given
@@ -1003,7 +1004,7 @@ type verdictData struct {
data Verdict
}
// newVerdictData creates a RegisterData for a verdict.
// newVerdictData creates a registerData for a verdict.
func newVerdictData(verdict Verdict) registerData { return verdictData{data: verdict} }
// String returns a string representation of the verdict data.
@@ -1044,7 +1045,7 @@ type bytesData struct {
data []byte
}
// newBytesData creates a RegisterData for <= 16 bytes of data.
// newBytesData creates a registerData for <= 16 bytes of data.
func newBytesData(bytes []byte) registerData {
if len(bytes) == 0 {
panic("bytes data cannot be empty")
pkg/tcpip/nftables/nftables_test.go
+101 -101
View File
File diff suppressed because it is too large Load Diff
pkg/tcpip/nftables/nftinterp.go
+8 -9
View File
@@ -250,8 +250,8 @@ func InterpretComparison(line string, lnIdx int) (operation, error) {
}
tkIdx++
// Fifth token should be the value.
nextIdx, data, err := parseRegisterData(reg, tokens, lnIdx, tkIdx)
// Fifth token should be the bytesData representing the value.
nextIdx, data, err := parseHexData(tokens, lnIdx, tkIdx)
if err != nil {
return nil, err
}
@@ -558,18 +558,17 @@ func parseRegisterData(reg uint8, tokens []string, lnIdx int, tkIdx int) (int, r
}
return nextIdx, newVerdictData(verdict), nil
}
// Handles hex data (4-, 8-, 12-, or 16-byte).
// Handles hex data.
if len(tokens[tkIdx]) > 1 && tokens[tkIdx][:2] == "0x" {
nextIdx, data, err := parseHexData(tokens, lnIdx, tkIdx)
if err != nil {
return 0, nil, err
}
// 4-byte data is only valid for 4-byte register. Any byte data can be
// stored in 16-byte registerValidates the register data type.
if err := data.validateRegister(reg); err != nil {
bytesData := newBytesData(data)
if err := bytesData.validateRegister(reg); err != nil {
return 0, nil, &LogicError{lnIdx, tkIdx, err}
}
return nextIdx, data, nil
return nextIdx, bytesData, nil
}
// TODO(b/345684870): cases will be added here as more types are supported.
return 0, nil, &SyntaxError{lnIdx, tkIdx, fmt.Sprintf("invalid register data: '%s'", tokens[tkIdx])}
@@ -619,7 +618,7 @@ func parseVerdict(tokens []string, lnIdx int, tkIdx int) (int, Verdict, error) {
// parseHexData parses little endian hexadecimal data from the given token,
// converts to big endian, and returns the index of the next token to process.
func parseHexData(tokens []string, lnIdx int, tkIdx int) (int, registerData, error) {
func parseHexData(tokens []string, lnIdx int, tkIdx int) (int, []byte, error) {
var bytes []byte
for ; tkIdx < len(tokens); tkIdx++ {
if len(tokens[tkIdx]) <= 2 || tokens[tkIdx][:2] != "0x" {
@@ -643,7 +642,7 @@ func parseHexData(tokens []string, lnIdx int, tkIdx int) (int, registerData, err
if len(bytes) > 16 {
return 0, nil, &SyntaxError{lnIdx, tkIdx, fmt.Sprintf("cannot have more than 16 bytes of hexadecimal data, got %d", len(bytes))}
}
return tkIdx, newBytesData(bytes), nil
return tkIdx, bytes, nil
}
// parseCmpOp parses the int representing the cmpOp from the given string.
pkg/tcpip/nftables/nftinterp_test.go
+30 -30
View File
@@ -188,32 +188,32 @@ func TestInterpretComparisonOps(t *testing.T) {
{
tname: "4-byte register == 4-byte data",
opStr: "[ cmp eq reg 8 0x0302010a ]",
expected: mustCreateComparison(t, linux.NFT_REG32_00, linux.NFT_CMP_EQ, newBytesData([]byte{0x0a, 0x01, 0x02, 0x03})),
expected: mustCreateComparison(t, linux.NFT_REG32_00, linux.NFT_CMP_EQ, []byte{0x0a, 0x01, 0x02, 0x03}),
},
{
tname: "4-byte register != 4-byte data",
opStr: "[ cmp neq reg 9 0x00000064 ]",
expected: mustCreateComparison(t, linux.NFT_REG32_01, linux.NFT_CMP_NEQ, newBytesData([]byte{0x64, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG32_01, linux.NFT_CMP_NEQ, []byte{0x64, 0x00, 0x00, 0x00}),
},
{
tname: "4-byte register < 4-byte data",
opStr: "[ cmp lt reg 10 0x00000000 ]",
expected: mustCreateComparison(t, linux.NFT_REG32_02, linux.NFT_CMP_LT, newBytesData([]byte{0x00, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG32_02, linux.NFT_CMP_LT, []byte{0x00, 0x00, 0x00, 0x00}),
},
{
tname: "4-byte register <= 4-byte data",
opStr: "[ cmp lte reg 11 0x00000164 ]",
expected: mustCreateComparison(t, linux.NFT_REG32_03, linux.NFT_CMP_LTE, newBytesData([]byte{0x64, 0x01, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG32_03, linux.NFT_CMP_LTE, []byte{0x64, 0x01, 0x00, 0x00}),
},
{
tname: "4-byte register > 4-byte data",
opStr: "[ cmp gt reg 12 0xe8030000 ]",
expected: mustCreateComparison(t, linux.NFT_REG32_04, linux.NFT_CMP_GT, newBytesData([]byte{0x00, 0x00, 0x03, 0xe8})),
expected: mustCreateComparison(t, linux.NFT_REG32_04, linux.NFT_CMP_GT, []byte{0x00, 0x00, 0x03, 0xe8}),
},
{
tname: "4-byte register >= 4-byte data",
opStr: "[ cmp gte reg 13 0xc02b0000 ]",
expected: mustCreateComparison(t, linux.NFT_REG32_05, linux.NFT_CMP_GTE, newBytesData([]byte{0x00, 0x00, 0x2b, 0xc0})),
expected: mustCreateComparison(t, linux.NFT_REG32_05, linux.NFT_CMP_GTE, []byte{0x00, 0x00, 0x2b, 0xc0}),
},
{
tname: "4-byte register with 8-byte data comparison",
@@ -233,122 +233,122 @@ func TestInterpretComparisonOps(t *testing.T) {
{
tname: "16-byte register == 4-byte data",
opStr: "[ cmp eq reg 1 0x0302010a ]",
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, newBytesData([]byte{0x0a, 0x01, 0x02, 0x03})),
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, []byte{0x0a, 0x01, 0x02, 0x03}),
},
{
tname: "16-byte register != 4-byte data",
opStr: "[ cmp neq reg 2 0x00000064 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, newBytesData([]byte{0x64, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, []byte{0x64, 0x00, 0x00, 0x00}),
},
{
tname: "16-byte register < 4-byte data",
opStr: "[ cmp lt reg 3 0x00000000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, newBytesData([]byte{0x00, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, []byte{0x00, 0x00, 0x00, 0x00}),
},
{
tname: "16-byte register <= 4-byte data",
opStr: "[ cmp lte reg 4 0x00000164 ]",
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, newBytesData([]byte{0x64, 0x01, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, []byte{0x64, 0x01, 0x00, 0x00}),
},
{
tname: "16-byte register > 4-byte data",
opStr: "[ cmp gt reg 1 0xe8030000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_GT, newBytesData([]byte{0x00, 0x00, 0x03, 0xe8})),
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_GT, []byte{0x00, 0x00, 0x03, 0xe8}),
},
{
tname: "16-byte register >= 4-byte data",
opStr: "[ cmp gte reg 2 0xc02b0000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GTE, newBytesData([]byte{0x00, 0x00, 0x2b, 0xc0})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GTE, []byte{0x00, 0x00, 0x2b, 0xc0}),
},
{
tname: "16-byte register == 8-byte data",
opStr: "[ cmp eq reg 1 0x0302010a 0x12345678 ]",
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, newBytesData([]byte{0x0a, 0x01, 0x02, 0x03, 0x78, 0x56, 0x34, 0x12})),
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, []byte{0x0a, 0x01, 0x02, 0x03, 0x78, 0x56, 0x34, 0x12}),
},
{
tname: "16-byte register != 8-byte data",
opStr: "[ cmp neq reg 2 0x00000064 0x00000020 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, newBytesData([]byte{0x64, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, []byte{0x64, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00}),
},
{
tname: "16-byte register < 8-byte data",
opStr: "[ cmp lt reg 3 0x00000000 0x00000000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, newBytesData([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}),
},
{
tname: "16-byte register <= 8-byte data",
opStr: "[ cmp lte reg 4 0x00000164 0x00000164 ]",
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, newBytesData([]byte{0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, []byte{0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00}),
},
{
tname: "16-byte register > 8-byte data",
opStr: "[ cmp gt reg 2 0xe8030000 0x00000f13 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GT, newBytesData([]byte{0x00, 0x00, 0x03, 0xe8, 0x13, 0x0f, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GT, []byte{0x00, 0x00, 0x03, 0xe8, 0x13, 0x0f, 0x00, 0x00}),
},
{
tname: "16-byte register >= 8-byte data",
opStr: "[ cmp gte reg 3 0x0a000120 0xc0090000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_GTE, newBytesData([]byte{0x20, 0x01, 0x00, 0x0a, 0x00, 0x00, 0x09, 0xc0})),
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_GTE, []byte{0x20, 0x01, 0x00, 0x0a, 0x00, 0x00, 0x09, 0xc0}),
},
{
tname: "16-byte register == 12-byte data",
opStr: "[ cmp eq reg 1 0x0302010a 0x00000000 0x12345678 ]",
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, newBytesData([]byte{0x0a, 0x01, 0x02, 0x03, 0x00, 0x00, 0x00, 0x00, 0x78, 0x56, 0x34, 0x12})),
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, []byte{0x0a, 0x01, 0x02, 0x03, 0x00, 0x00, 0x00, 0x00, 0x78, 0x56, 0x34, 0x12}),
},
{
tname: "16-byte register != 12-byte data",
opStr: "[ cmp neq reg 2 0x00000064 0x00000000 0x00000020 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, newBytesData([]byte{0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, []byte{0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00}),
},
{
tname: "16-byte register < 12-byte data",
opStr: "[ cmp lt reg 3 0x00000000 0x00000000 0x00000000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, newBytesData([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}),
},
{
tname: "16-byte register <= 12-byte data",
opStr: "[ cmp lte reg 4 0x00000164 0x00000164 0x00000164 ]",
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, newBytesData([]byte{0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, []byte{0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00}),
},
{
tname: "16-byte register > 12-byte data",
opStr: "[ cmp gt reg 2 0xe8030000 0x00000f13 0xc0090000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GT, newBytesData([]byte{0x00, 0x00, 0x03, 0xe8, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GT, []byte{0x00, 0x00, 0x03, 0xe8, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0}),
},
{
tname: "16-byte register >= 12-byte data",
opStr: "[ cmp gte reg 3 0x0a000120 0x00000f13 0xc0090000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_GTE, newBytesData([]byte{0x20, 0x01, 0x00, 0x0a, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0})),
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_GTE, []byte{0x20, 0x01, 0x00, 0x0a, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0}),
},
{
tname: "16-byte register == 16-byte data",
opStr: "[ cmp eq reg 1 0x0302010a 0x00000000 0x00000000 0x02000002 ]",
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, newBytesData([]byte{0x0a, 0x01, 0x02, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x02})),
expected: mustCreateComparison(t, linux.NFT_REG_1, linux.NFT_CMP_EQ, []byte{0x0a, 0x01, 0x02, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x02}),
},
{
tname: "16-byte register != 16-byte data",
opStr: "[ cmp neq reg 2 0x00000064 0x00000000 0x00000000 0x02000000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, newBytesData([]byte{0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_NEQ, []byte{0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}),
},
{
tname: "16-byte register < 16-byte data",
opStr: "[ cmp lt reg 3 0x00000000 0x00000000 0x00000000 0x00000000 ]",
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, newBytesData([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_LT, []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}),
},
{
tname: "16-byte register <= 16-byte data",
opStr: "[ cmp lte reg 4 0x00000164 0x00000164 0x00000164 0x00000164 ]",
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, newBytesData([]byte{0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00})),
expected: mustCreateComparison(t, linux.NFT_REG_4, linux.NFT_CMP_LTE, []byte{0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00, 0x64, 0x01, 0x00, 0x00}),
},
{
tname: "16-byte register > 16-byte data",
opStr: "[ cmp gt reg 2 0xe8030000 0x00000f13 0xc0090000 0x0b136a87 ]",
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GT, newBytesData([]byte{0x00, 0x00, 0x03, 0xe8, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0, 0x87, 0x6a, 0x13, 0x0b})),
expected: mustCreateComparison(t, linux.NFT_REG_2, linux.NFT_CMP_GT, []byte{0x00, 0x00, 0x03, 0xe8, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0, 0x87, 0x6a, 0x13, 0x0b}),
},
{
tname: "16-byte register >= 16-byte data",
opStr: "[ cmp gte reg 3 0x0a000120 0x00000f13 0xc0090000 0x0b136a87 ]",
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_GTE, newBytesData([]byte{0x20, 0x01, 0x00, 0x0a, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0, 0x87, 0x6a, 0x13, 0x0b})),
expected: mustCreateComparison(t, linux.NFT_REG_3, linux.NFT_CMP_GTE, []byte{0x20, 0x01, 0x00, 0x0a, 0x13, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x09, 0xc0, 0x87, 0x6a, 0x13, 0x0b}),
},
} {
t.Run(test.tname, func(t *testing.T) { checkOp(t, test, checkComparisonOp) })