Repository: cilium/ebpf
Pull request #514 (2021-12-02 16:53:02)

asm: Add Jump32 support
The unused 0x6 class now represents jumps with 32bit comparisons:

https://lore.kernel.org/all/1548523574-18316-16-git-send-email-jiong.wang@netronome.com/T/
http://patchwork.ozlabs.org/project/netdev/list/?series=88386&state=*

This complements 32bit ALU operations by allowing comparisons without
having to sign extend registers beforehand.

Add a Jump32Class to represent this. As some JumpOps are not supported
by Jump32 (Call, Exit, Ja), centralize the logic for validating jump
OpCodes in OpCode.JumpOp().

arthurfabre: Fixed a typo in a test.
arthurfabre: I force pushed to re-run the tests, but CI failed again with:

```
E: Failed to fetch https://storage.googleapis.com/bazel-apt/dists/stable/jdk1.8/binary-amd64/Packages.gz  File has unexpected size (6503 != 6709). Mirror sync in progress? [IP: 142.250.186.112 443]
```

I'll try again in a bit.
lmb: Ping @arthurfabre do you want to pick this up again?

Patch:
diff --git a/asm/instruction.go b/asm/instruction.go
index e2c0919..cc1140c 100644
--- a/asm/instruction.go
+++ b/asm/instruction.go
@@ -231,8 +231,8 @@ func (ins Instruction) Format(f fmt.State, c rune) {
 	}
 
 	fmt.Fprintf(f, "%v ", op)
-	switch cls := op.Class(); cls {
-	case LdClass, LdXClass, StClass, StXClass:
+	switch cls := op.Class(); {
+	case cls.isLoadOrStore():
 		switch op.Mode() {
 		case ImmMode:
 			fmt.Fprintf(f, "dst: %s imm: %d", ins.Dst, ins.Constant)
@@ -246,7 +246,7 @@ func (ins Instruction) Format(f fmt.State, c rune) {
 			fmt.Fprintf(f, "dst: %s src: %s", ins.Dst, ins.Src)
 		}
 
-	case ALU64Class, ALUClass:
+	case cls.IsALU():
 		fmt.Fprintf(f, "dst: %s ", ins.Dst)
 		if op.ALUOp() == Swap || op.Source() == ImmSource {
 			fmt.Fprintf(f, "imm: %d", ins.Constant)
@@ -254,7 +254,7 @@ func (ins Instruction) Format(f fmt.State, c rune) {
 			fmt.Fprintf(f, "src: %s", ins.Src)
 		}
 
-	case JumpClass:
+	case cls.IsJump():
 		switch jop := op.JumpOp(); jop {
 		case Call:
 			if ins.Src == PseudoCall {
diff --git a/asm/jump.go b/asm/jump.go
index 7757179..199c069 100644
--- a/asm/jump.go
+++ b/asm/jump.go
@@ -60,14 +60,22 @@ func (op JumpOp) Op(source Source) OpCode {
 	return OpCode(JumpClass).SetJumpOp(op).SetSource(source)
 }
 
-// Imm compares dst to value, and adjusts PC by offset if the condition is fulfilled.
+// Imm compares 64 bit dst to 64 bit value (sign extended), and adjusts PC by offset if the condition is fulfilled.
 func (op JumpOp) Imm(dst Register, value int32, label string) Instruction {
-	if op == Exit || op == Call || op == Ja {
-		return Instruction{OpCode: InvalidOpCode}
+	return Instruction{
+		OpCode:    op.opCode(JumpClass, ImmSource),
+		Dst:       dst,
+		Offset:    -1,
+		Constant:  int64(value),
+		Reference: label,
 	}
+}
 
+// Imm32 compares 32 bit dst to 32 bit value, and adjusts PC by offset if the condition is fulfilled.
+// Requires kernel 5.1.
+func (op JumpOp) Imm32(dst Register, value int32, label string) Instruction {
 	return Instruction{
-		OpCode:    OpCode(JumpClass).SetJumpOp(op).SetSource(ImmSource),
+		OpCode:    op.opCode(Jump32Class, ImmSource),
 		Dst:       dst,
 		Offset:    -1,
 		Constant:  int64(value),
@@ -75,14 +83,22 @@ func (op JumpOp) Imm(dst Register, value int32, label string) Instruction {
 	}
 }
 
-// Reg compares dst to src, and adjusts PC by offset if the condition is fulfilled.
+// Reg compares 64 bit dst to 64 bit src, and adjusts PC by offset if the condition is fulfilled.
 func (op JumpOp) Reg(dst, src Register, label string) Instruction {
-	if op == Exit || op == Call || op == Ja {
-		return Instruction{OpCode: InvalidOpCode}
+	return Instruction{
+		OpCode:    op.opCode(JumpClass, RegSource),
+		Dst:       dst,
+		Src:       src,
+		Offset:    -1,
+		Reference: label,
 	}
+}
 
+// Reg32 compares 32 bit dst to 32 bit src, and adjusts PC by offset if the condition is fulfilled.
+// Requires kernel 5.1.
+func (op JumpOp) Reg32(dst, src Register, label string) Instruction {
 	return Instruction{
-		OpCode:    OpCode(JumpClass).SetJumpOp(op).SetSource(RegSource),
+		OpCode:    op.opCode(Jump32Class, RegSource),
 		Dst:       dst,
 		Src:       src,
 		Offset:    -1,
@@ -90,6 +106,14 @@ func (op JumpOp) Reg(dst, src Register, label string) Instruction {
 	}
 }
 
+func (op JumpOp) opCode(class Class, source Source) OpCode {
+	if op == Exit || op == Call || op == Ja {
+		return InvalidOpCode
+	}
+
+	return OpCode(class).SetJumpOp(op).SetSource(source)
+}
+
 // Label adjusts PC to the address of the label.
 func (op JumpOp) Label(label string) Instruction {
 	if op == Call {
diff --git a/asm/opcode.go b/asm/opcode.go
index 6edc3cf..f6d8e06 100644
--- a/asm/opcode.go
+++ b/asm/opcode.go
@@ -7,14 +7,6 @@ import (
 
 //go:generate stringer -output opcode_string.go -type=Class
 
-type encoding int
-
-const (
-	unknownEncoding encoding = iota
-	loadOrStore
-	jumpOrALU
-)
-
 // Class of operations
 //
 //    msb      lsb
@@ -38,19 +30,39 @@ const (
 	ALUClass Class = 0x04
 	// JumpClass jump operators
 	JumpClass Class = 0x05
+	// Jump32Class jump operators with 32 bit comparaisons
+	// Requires kernel 5.1
+	Jump32Class Class = 0x06
 	// ALU64Class arithmetic in 64 bit mode
 	ALU64Class Class = 0x07
 )
 
-func (cls Class) encoding() encoding {
-	switch cls {
-	case LdClass, LdXClass, StClass, StXClass:
-		return loadOrStore
-	case ALU64Class, ALUClass, JumpClass:
-		return jumpOrALU
-	default:
-		return unknownEncoding
-	}
+// IsLoad checks if this is either LdClass or LdXClass.
+func (cls Class) IsLoad() bool {
+	return cls == LdClass || cls == LdXClass
+}
+
+// IsStore checks if this is either StClass or StXClass.
+func (cls Class) IsStore() bool {
+	return cls == StClass || cls == StXClass
+}
+
+func (cls Class) isLoadOrStore() bool {
+	return cls.IsLoad() || cls.IsStore()
+}
+
+// IsALU checks if this is either ALUClass or ALU64Class.
+func (cls Class) IsALU() bool {
+	return cls == ALUClass || cls == ALU64Class
+}
+
+// IsJump checks if this is either JumpClass or Jump32Class.
+func (cls Class) IsJump() bool {
+	return cls == JumpClass || cls == Jump32Class
+}
+
+func (cls Class) isJumpOrALU() bool {
+	return cls.IsJump() || cls.IsALU()
 }
 
 // OpCode is a packed eBPF opcode.
@@ -86,7 +98,7 @@ func (op OpCode) Class() Class {
 
 // Mode returns the mode for load and store operations.
 func (op OpCode) Mode() Mode {
-	if op.Class().encoding() != loadOrStore {
+	if !op.Class().isLoadOrStore() {
 		return InvalidMode
 	}
 	return Mode(op & modeMask)
@@ -94,7 +106,7 @@ func (op OpCode) Mode() Mode {
 
 // Size returns the size for load and store operations.
 func (op OpCode) Size() Size {
-	if op.Class().encoding() != loadOrStore {
+	if !op.Class().isLoadOrStore() {
 		return InvalidSize
 	}
 	return Size(op & sizeMask)
@@ -102,7 +114,7 @@ func (op OpCode) Size() Size {
 
 // Source returns the source for branch and ALU operations.
 func (op OpCode) Source() Source {
-	if op.Class().encoding() != jumpOrALU || op.ALUOp() == Swap {
+	if !op.Class().isJumpOrALU() || op.ALUOp() == Swap {
 		return InvalidSource
 	}
 	return Source(op & sourceMask)
@@ -110,7 +122,7 @@ func (op OpCode) Source() Source {
 
 // ALUOp returns the ALUOp.
 func (op OpCode) ALUOp() ALUOp {
-	if op.Class().encoding() != jumpOrALU {
+	if !op.Class().IsALU() {
 		return InvalidALUOp
 	}
 	return ALUOp(op & aluMask)
@@ -125,18 +137,27 @@ func (op OpCode) Endianness() Endianness {
 }
 
 // JumpOp returns the JumpOp.
+// Returns InvalidJumpOp if it doesn't encode a jump.
 func (op OpCode) JumpOp() JumpOp {
-	if op.Class().encoding() != jumpOrALU {
+	if !op.Class().IsJump() {
 		return InvalidJumpOp
 	}
-	return JumpOp(op & jumpMask)
+
+	jumpOp := JumpOp(op & jumpMask)
+
+	// Some JumpOps are only supported by JumpClass, not Jump32Class.
+	if op.Class() == Jump32Class && (jumpOp == Exit || jumpOp == Call || jumpOp == Ja) {
+		return InvalidJumpOp
+	}
+
+	return jumpOp
 }
 
 // SetMode sets the mode on load and store operations.
 //
 // Returns InvalidOpCode if op is of the wrong class.
 func (op OpCode) SetMode(mode Mode) OpCode {
-	if op.Class().encoding() != loadOrStore || !valid(OpCode(mode), modeMask) {
+	if !op.Class().isLoadOrStore() || !valid(OpCode(mode), modeMask) {
 		return InvalidOpCode
 	}
 	return (op & ^modeMask) | OpCode(mode)
@@ -146,7 +167,7 @@ func (op OpCode) SetMode(mode Mode) OpCode {
 //
 // Returns InvalidOpCode if op is of the wrong class.
 func (op OpCode) SetSize(size Size) OpCode {
-	if op.Class().encoding() != loadOrStore || !valid(OpCode(size), sizeMask) {
+	if !op.Class().isLoadOrStore() || !valid(OpCode(size), sizeMask) {
 		return InvalidOpCode
 	}
 	return (op & ^sizeMask) | OpCode(size)
@@ -156,7 +177,7 @@ func (op OpCode) SetSize(size Size) OpCode {
 //
 // Returns InvalidOpCode if op is of the wrong class.
 func (op OpCode) SetSource(source Source) OpCode {
-	if op.Class().encoding() != jumpOrALU || !valid(OpCode(source), sourceMask) {
+	if !op.Class().isJumpOrALU() || !valid(OpCode(source), sourceMask) {
 		return InvalidOpCode
 	}
 	return (op & ^sourceMask) | OpCode(source)
@@ -166,8 +187,7 @@ func (op OpCode) SetSource(source Source) OpCode {
 //
 // Returns InvalidOpCode if op is of the wrong class.
 func (op OpCode) SetALUOp(alu ALUOp) OpCode {
-	class := op.Class()
-	if (class != ALUClass && class != ALU64Class) || !valid(OpCode(alu), aluMask) {
+	if !op.Class().IsALU() || !valid(OpCode(alu), aluMask) {
 		return InvalidOpCode
 	}
 	return (op & ^aluMask) | OpCode(alu)
@@ -177,17 +197,25 @@ func (op OpCode) SetALUOp(alu ALUOp) OpCode {
 //
 // Returns InvalidOpCode if op is of the wrong class.
 func (op OpCode) SetJumpOp(jump JumpOp) OpCode {
-	if op.Class() != JumpClass || !valid(OpCode(jump), jumpMask) {
+	if !op.Class().IsJump() || !valid(OpCode(jump), jumpMask) {
+		return InvalidOpCode
+	}
+
+	newOp := (op & ^jumpMask) | OpCode(jump)
+
+	// Check newOp is legal.
+	if newOp.JumpOp() == InvalidJumpOp {
 		return InvalidOpCode
 	}
-	return (op & ^jumpMask) | OpCode(jump)
+
+	return newOp
 }
 
 func (op OpCode) String() string {
 	var f strings.Builder
 
-	switch class := op.Class(); class {
-	case LdClass, LdXClass, StClass, StXClass:
+	switch class := op.Class(); {
+	case class.isLoadOrStore():
 		f.WriteString(strings.TrimSuffix(class.String(), "Class"))
 
 		mode := op.Mode()
@@ -204,7 +232,7 @@ func (op OpCode) String() string {
 			f.WriteString("B")
 		}
 
-	case ALU64Class, ALUClass:
+	case class.IsALU():
 		f.WriteString(op.ALUOp().String())
 
 		if op.ALUOp() == Swap {
@@ -218,8 +246,13 @@ func (op OpCode) String() string {
 			f.WriteString(strings.TrimSuffix(op.Source().String(), "Source"))
 		}
 
-	case JumpClass:
+	case class.IsJump():
 		f.WriteString(op.JumpOp().String())
+
+		if class == Jump32Class {
+			f.WriteString("32")
+		}
+
 		if jop := op.JumpOp(); jop != Exit && jop != Call {
 			f.WriteString(strings.TrimSuffix(op.Source().String(), "Source"))
 		}
diff --git a/asm/opcode_string.go b/asm/opcode_string.go
index 079ce1d..58bc3e7 100644
--- a/asm/opcode_string.go
+++ b/asm/opcode_string.go
@@ -14,25 +14,17 @@ func _() {
 	_ = x[StXClass-3]
 	_ = x[ALUClass-4]
 	_ = x[JumpClass-5]
+	_ = x[Jump32Class-6]
 	_ = x[ALU64Class-7]
 }
 
-const (
-	_Class_name_0 = "LdClassLdXClassStClassStXClassALUClassJumpClass"
-	_Class_name_1 = "ALU64Class"
-)
+const _Class_name = "LdClassLdXClassStClassStXClassALUClassJumpClassJump32ClassALU64Class"
 
-var (
-	_Class_index_0 = [...]uint8{0, 7, 15, 22, 30, 38, 47}
-)
+var _Class_index = [...]uint8{0, 7, 15, 22, 30, 38, 47, 58, 68}
 
 func (i Class) String() string {
-	switch {
-	case 0 <= i && i <= 5:
-		return _Class_name_0[_Class_index_0[i]:_Class_index_0[i+1]]
-	case i == 7:
-		return _Class_name_1
-	default:
+	if i >= Class(len(_Class_index)-1) {
 		return "Class(" + strconv.FormatInt(int64(i), 10) + ")"
 	}
+	return _Class_name[_Class_index[i]:_Class_index[i+1]]
 }
diff --git a/linker.go b/linker.go
index a16df87..943296a 100644
--- a/linker.go
+++ b/linker.go
@@ -124,7 +124,7 @@ func fixupJumpsAndCalls(insns asm.Instructions) error {
 			ins.Constant = int64(symOffset - offset - 1)
 			continue
 
-		case ins.OpCode.Class() == asm.JumpClass && ins.Offset == -1:
+		case ins.OpCode.Class().IsJump() && ins.Offset == -1:
 			if !ok {
 				break
 			}


Test patch:
diff --git a/asm/dsl_test.go b/asm/dsl_test.go
index 243a42f..ddbf817 100644
--- a/asm/dsl_test.go
+++ b/asm/dsl_test.go
@@ -24,6 +24,18 @@ func TestDSL(t *testing.T) {
 		{"Add.Imm32", Add.Imm32(R1, 22), Instruction{
 			OpCode: 0x04, Dst: R1, Constant: 22,
 		}},
+		{"JSGT.Imm", JSGT.Imm(R1, 4, "foo"), Instruction{
+			OpCode: 0x65, Dst: R1, Constant: 4, Offset: -1, Reference: "foo",
+		}},
+		{"JSGT.Imm32", JSGT.Imm32(R1, -2, "foo"), Instruction{
+			OpCode: 0x66, Dst: R1, Constant: -2, Offset: -1, Reference: "foo",
+		}},
+		{"JSLT.Reg", JSLT.Reg(R1, R2, "foo"), Instruction{
+			OpCode: 0xcd, Dst: R1, Src: R2, Offset: -1, Reference: "foo",
+		}},
+		{"JSLT.Reg32", JSLT.Reg32(R1, R3, "foo"), Instruction{
+			OpCode: 0xce, Dst: R1, Src: R3, Offset: -1, Reference: "foo",
+		}},
 	}
 
 	for _, tc := range testcases {
diff --git a/asm/opcode_test.go b/asm/opcode_test.go
new file mode 100644
index 0000000..4e55727
--- /dev/null
+++ b/asm/opcode_test.go
@@ -0,0 +1,52 @@
+package asm
+
+import (
+	"fmt"
+	"testing"
+
+	qt "github.com/frankban/quicktest"
+)
+
+func TestGetSetJumpOp(t *testing.T) {
+	test := func(class Class, op JumpOp, valid bool) {
+		t.Run(fmt.Sprintf("%s-%s", class, op), func(t *testing.T) {
+			opcode := OpCode(class).SetJumpOp(op)
+
+			if valid {
+				qt.Assert(t, opcode, qt.Not(qt.Equals), InvalidOpCode)
+				qt.Assert(t, opcode.JumpOp(), qt.Equals, op)
+			} else {
+				qt.Assert(t, opcode, qt.Equals, InvalidOpCode)
+				qt.Assert(t, opcode.JumpOp(), qt.Equals, InvalidJumpOp)
+			}
+		})
+	}
+
+	// Exit, call and JA aren't allowed with Jump32
+	test(Jump32Class, Exit, false)
+	test(Jump32Class, Call, false)
+	test(Jump32Class, Ja, false)
+
+	// But are with Jump
+	test(JumpClass, Exit, true)
+	test(JumpClass, Call, true)
+	test(JumpClass, Ja, true)
+
+	// All other ops work
+	for _, op := range []JumpOp{
+		JEq,
+		JGT,
+		JGE,
+		JSet,
+		JNE,
+		JSGT,
+		JSGE,
+		JLT,
+		JLE,
+		JSLT,
+		JSLE,
+	} {
+		test(Jump32Class, op, true)
+		test(JumpClass, op, true)
+	}
+}