Skip to content

Commit

Permalink
prog: fix shadowing log size variable in verifier log retry loop
Browse files Browse the repository at this point in the history
When ProgramOptions.LogSize was removed in 0.16, the tests weren't updated to
exercise the retry loop, since a minimum log size was chosen that was larger
than what the test program could generate.

With the addition of LogSizeStart, this notoriously fragile code broke when
logSize was again tracked as a separate variable, while being accidentally
shadowed within the scope of the for loop. This resulted in an endless loop
on kernels without the LogTrueSize field.

Remove the shadowing and fix the tests.

Signed-off-by: Timo Beckers <timo@isovalent.com>
  • Loading branch information
ti-mo committed Dec 20, 2024
1 parent 228bb4e commit e439d37
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
2 changes: 1 addition & 1 deletion prog.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
// Make an educated guess how large the buffer should be by multiplying.
// Ensure the size doesn't overflow.
const factor = 2
logSize := internal.Between(logSize, minVerifierLogSize, maxVerifierLogSize/factor)
logSize = internal.Between(logSize, minVerifierLogSize, maxVerifierLogSize/factor)
logSize *= factor

if attr.LogTrueSize != 0 {
Expand Down
21 changes: 17 additions & 4 deletions prog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,12 +469,16 @@ func TestProgramVerifierLog(t *testing.T) {

var ve *internal.VerifierError
qt.Assert(t, qt.ErrorAs(err, &ve))

loglen := len(fmt.Sprintf("%+v", ve))
qt.Assert(t, qt.IsTrue(loglen > minVerifierLogSize),
qt.Commentf("Log buffer didn't grow past minimum, got %d bytes", loglen))
}

// Generate a base program of sufficient size whose verifier log does not fit
// a 128-byte buffer. This should always result in ENOSPC.
// in the minimum buffer size. Stay under 4096 insn limit of older kernels.
var base asm.Instructions
for i := 0; i < 32; i++ {
for i := 0; i < 4093; i++ {
base = append(base, asm.Mov.Reg(asm.R0, asm.R1))
}

Expand All @@ -493,8 +497,7 @@ func TestProgramVerifierLog(t *testing.T) {
Instructions: invalid,
}

// Set an undersized log buffer without explicitly requesting a verifier log
// for an invalid program.
// Don't explicitly request a verifier log for an invalid program.
_, err := NewProgramWithOptions(spec, ProgramOptions{})
check(t, err)

Expand Down Expand Up @@ -528,6 +531,16 @@ func TestProgramVerifierLog(t *testing.T) {
LogLevel: LogLevelInstruction,
})
qt.Assert(t, qt.IsNil(err))
qt.Assert(t, qt.IsTrue(len(prog.VerifierLog) > minVerifierLogSize))
prog.Close()

// Repeat the previous test with a larger starting buffer size.
prog, err = NewProgramWithOptions(spec, ProgramOptions{
LogLevel: LogLevelInstruction,
LogSizeStart: minVerifierLogSize * 2,
})
qt.Assert(t, qt.IsNil(err))
qt.Assert(t, qt.IsTrue(len(prog.VerifierLog) > minVerifierLogSize))
prog.Close()
}

Expand Down

0 comments on commit e439d37

Please sign in to comment.