diff --git a/.editorconfig b/.editorconfig index 2e6c2ea..2bb698b 100644 --- a/.editorconfig +++ b/.editorconfig @@ -18,4 +18,4 @@ indent_size = 4 [*.go] indent_style = tab tab_width = 4 -indent_size = 4 \ No newline at end of file +indent_size = 4 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..b5f8499 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,65 @@ +name: CI + +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + +jobs: + pre-commit: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Install Golang + uses: actions/setup-go@v6 + with: + go-version-file: go.mod + - name: Run pre-commit + uses: pre-commit/action@v3.0.1 + + test: + runs-on: ubuntu-latest + container: + image: ghcr.io/aibor/virtrun-test:${{ matrix.arch }} + strategy: + matrix: + arch: + - amd64 + - arm64 + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Install Golang + uses: actions/setup-go@v6 + with: + go-version-file: go.mod + - name: Download kernel modules + working-directory: /kernel + # Downloading not only the modules but also the kernel again to make + # sure they stem from the exact same version. + run: | + apk fetch --arch ${{ matrix.arch }} --no-cache --allow-untrusted linux-virt + tar xf linux-virt-*.apk --wildcards --transform='s,.*/,,' \ + 'boot/vmlinuz-virt' \ + 'lib/modules/*/kernel/drivers/net/tap.ko.gz' \ + 'lib/modules/*/kernel/drivers/vhost/vhost_iotlb.ko.gz' \ + 'lib/modules/*/kernel/drivers/vhost/vhost.ko.gz' \ + 'lib/modules/*/kernel/drivers/vhost/vhost_net.ko.gz' + rm linux-virt-*.apk + - name: Run tests + env: + GOARCH: ${{ matrix.arch }} + VIRTRUN_ARGS: > + -kernel /kernel/vmlinuz-virt + -addModule /kernel/tap.ko.gz + -addModule /kernel/vhost_iotlb.ko.gz + -addModule /kernel/vhost.ko.gz + -addModule /kernel/vhost_net.ko.gz + -transport pci + -verbose + run: go test -v -exec "go tool virtrun" ./... diff --git a/.gitignore b/.gitignore index 462d23b..06b9378 100644 --- a/.gitignore +++ b/.gitignore @@ -32,4 +32,4 @@ go.work.sum !.vscode/*.code-snippets # Ignore all IntelliJ/GoLand project files for now -.idea/ \ No newline at end of file +.idea/ diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..31965cc --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,118 @@ +version: "2" + +run: + timeout: 5m + +linters: + default: all + disable: + - cyclop + - depguard + - dupword + - exhaustruct + - forcetypeassert + - funcorder + - funlen + - gochecknoglobals + - gocognit + - goconst + - gocyclo + - godox + - gomoddirectives + - gosec + - iface + - inamedparam + - interfacebloat + - ireturn + - maintidx + - mnd + - musttag + - nestif + - nilnil + - nlreturn + - noinlineerr + - nonamedreturns + - paralleltest + - recvcheck + - tagliatelle + - testifylint + - varnamelen + - wsl + - wsl_v5 + settings: + exhaustive: + default-signifies-exhaustive: true + godot: + exclude: + - "^.*:" + gomodguard: + blocked: + modules: + - github.com/tj/assert: + recommendations: + - github.com/stretchr/testify + reason: testify provides more assertions and one is enough + - gotest.tools/v3/assert: + recommendations: + - github.com/stretchr/testify + reason: testify provides more assertions and one is enough + govet: + disable: + # This linter produces false positives for memory not managed by Go. + - unsafeptr + lll: + line-length: 120 + tab-width: 4 + misspell: + locale: US + perfsprint: + strconcat: false + revive: + rules: + - name: atomic + disabled: false + - name: defer + disabled: false + - name: exported + disabled: false + - name: if-return + disabled: false + - name: struct-tag + disabled: false + arguments: + - "json,inline" + - "bson,outline,gnu" + staticcheck: + checks: + - all + - -ST1000 + - -ST1003 + - -ST1016 + - -ST1020 + - -ST1021 + - -ST1022 + wrapcheck: + ignore-sigs: + - .Errorf( + - errors.New( + - errors.Unwrap( + - errors.Join( + - .Wrap( + - .Wrapf( + - .WithMessage( + - .WithMessagef( + - .WithStack( + ignore-sig-regexps: + - \.New.*Error\( + +formatters: + enable: + - gci + - gofmt + - gofumpt + - goimports + - golines + settings: + golines: + max-len: 120 + tab-len: 4 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..202ec43 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,39 @@ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v6.0.0 + hooks: + - id: check-added-large-files + - id: check-case-conflict + - id: check-executables-have-shebangs + - id: check-shebang-scripts-are-executable + - id: check-symlinks + - id: destroyed-symlinks + + - id: check-json + - id: check-yaml + - id: check-toml + + - id: check-merge-conflict + - id: end-of-file-fixer + - id: mixed-line-ending + args: [--fix=lf] + - id: trailing-whitespace + + - repo: local + hooks: + - id: go-mod-tidy + name: go mod tidy + language: golang + entry: go mod tidy + pass_filenames: false + + - id: go-generate + name: go generate + language: golang + entry: go generate ./... + pass_filenames: false + + - repo: https://github.com/golangci/golangci-lint + rev: v2.5.0 + hooks: + - id: golangci-lint-full diff --git a/LICENSE b/LICENSE index 08e1787..d681f62 100644 --- a/LICENSE +++ b/LICENSE @@ -18,4 +18,4 @@ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. \ No newline at end of file +SOFTWARE. diff --git a/internal/testsupport/packet.go b/internal/testsupport/packet.go index 609a72f..5d72c4d 100644 --- a/internal/testsupport/packet.go +++ b/internal/testsupport/packet.go @@ -14,11 +14,14 @@ import ( // PrependPacket encodes the given [virtio.NetHdr], puts it in front of the // given packet slice and returns the new slice. func PrependPacket(tb testing.TB, vnethdr virtio.NetHdr, pkt []byte) []byte { + tb.Helper() + result := make([]byte, virtio.NetHdrSize+len(pkt)) if err := vnethdr.Encode(result); err != nil { tb.Fatalf("Encode vnethdr: %v", err) } copy(result[virtio.NetHdrSize:], pkt) + return result } diff --git a/tuntap/device.go b/tuntap/device.go index 3fb149c..5d765f1 100644 --- a/tuntap/device.go +++ b/tuntap/device.go @@ -44,7 +44,7 @@ func NewDevice(options ...Option) (_ *Device, err error) { // Get a file descriptor. The device will exist as long as we keep this // file descriptor open. - fd, err := unix.Open("/dev/net/tun", os.O_RDWR, 0666) + fd, err := unix.Open("/dev/net/tun", os.O_RDWR, 0o666) if err != nil { return nil, fmt.Errorf("access tuntap driver: %w", err) } diff --git a/tuntap/options.go b/tuntap/options.go index b32cf04..7a99383 100644 --- a/tuntap/options.go +++ b/tuntap/options.go @@ -33,6 +33,7 @@ func (o *optionValues) apply(options []Option) { } } +//nolint:err113 // Allow errors.New for validation functions. func (o *optionValues) validate() error { if len(o.name) >= unix.IFNAMSIZ { return errors.New("name must not be longer that 15 characters") diff --git a/vhostnet/device.go b/vhostnet/device.go index 34fd3a5..beada70 100644 --- a/vhostnet/device.go +++ b/vhostnet/device.go @@ -70,7 +70,7 @@ func NewDevice(options ...Option) (_ *Device, err error) { // Retrieve a new control file descriptor. This will be used to configure // the vhost networking device in the kernel. - dev.controlFD, err = unix.Open("/dev/vhost-net", os.O_RDWR, 0666) + dev.controlFD, err = unix.Open("/dev/vhost-net", os.O_RDWR, 0o666) if err != nil { return nil, fmt.Errorf("get control file descriptor: %w", err) } @@ -172,6 +172,8 @@ func (dev *Device) monitorTransmitQueue() { // transmit the packet. This method will not return before the packet was // transmitted and the device notifies that it has used the packet buffer. func (dev *Device) TransmitPacket(vnethdr virtio.NetHdr, packet []byte) error { + dev.ensureInitialized() + // Prepend the packet with its virtio-net header. vnethdrBuf := make([]byte, virtio.NetHdrSize) if err := vnethdr.Encode(vnethdrBuf); err != nil { @@ -203,6 +205,8 @@ func (dev *Device) TransmitPacket(vnethdr virtio.NetHdr, packet []byte) error { // broken state which this implementation cannot recover from. The caller should // close the device and not attempt any additional receives. func (dev *Device) ReceivePacket() (virtio.NetHdr, []byte, error) { + dev.ensureInitialized() + var ( chainHeads []uint16 @@ -400,7 +404,7 @@ func truncateBuffers(buffers [][]byte, length int) (out [][]byte) { for _, buffer := range buffers { if length < len(buffer) { out = append(out, buffer[:length]) - return + return out } out = append(out, buffer) length -= len(buffer) @@ -408,5 +412,5 @@ func truncateBuffers(buffers [][]byte, length int) (out [][]byte) { if length > 0 { panic("length exceeds the combined length of all buffers") } - return + return out } diff --git a/vhostnet/device_test.go b/vhostnet/device_test.go index d59b96c..c6fd6da 100644 --- a/vhostnet/device_test.go +++ b/vhostnet/device_test.go @@ -156,6 +156,7 @@ type TestFixture struct { } func NewTestFixture(t *testing.T) *TestFixture { + t.Helper() testsupport.VirtrunOnly(t) // In case something doesn't work, some more debug logging from the kernel diff --git a/vhostnet/options.go b/vhostnet/options.go index 3e68436..512ac7d 100644 --- a/vhostnet/options.go +++ b/vhostnet/options.go @@ -18,11 +18,13 @@ func (o *optionValues) apply(options []Option) { } } +//nolint:err113 // Allow errors.New for validation functions. func (o *optionValues) validate() error { if o.queueSize == -1 { return errors.New("queue size is required") } if err := virtqueue.CheckQueueSize(o.queueSize); err != nil { + //nolint:wrapcheck // Error is already descriptive enough. return err } if o.backendFD == -1 { diff --git a/virtio/net_hdr_test.go b/virtio/net_hdr_test.go index 81fd22b..bfe300b 100644 --- a/virtio/net_hdr_test.go +++ b/virtio/net_hdr_test.go @@ -1,20 +1,21 @@ -package virtio +package virtio_test import ( "testing" "unsafe" + "github.com/hetznercloud/virtio-go/virtio" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" ) func TestNetHdr_Size(t *testing.T) { - assert.EqualValues(t, NetHdrSize, unsafe.Sizeof(NetHdr{})) + assert.EqualValues(t, virtio.NetHdrSize, unsafe.Sizeof(virtio.NetHdr{})) } func TestNetHdr_Encoding(t *testing.T) { - vnethdr := NetHdr{ + vnethdr := virtio.NetHdr{ Flags: unix.VIRTIO_NET_HDR_F_NEEDS_CSUM, GSOType: unix.VIRTIO_NET_HDR_GSO_UDP_L4, HdrLen: 42, @@ -24,7 +25,7 @@ func TestNetHdr_Encoding(t *testing.T) { NumBuffers: 16, } - buf := make([]byte, NetHdrSize) + buf := make([]byte, virtio.NetHdrSize) require.NoError(t, vnethdr.Encode(buf)) assert.Equal(t, []byte{ @@ -36,7 +37,7 @@ func TestNetHdr_Encoding(t *testing.T) { 0x10, 0x00, }, buf) - var decoded NetHdr + var decoded virtio.NetHdr require.NoError(t, decoded.Decode(buf)) assert.Equal(t, vnethdr, decoded) diff --git a/virtqueue/available_ring.go b/virtqueue/available_ring.go index 20ad609..b66b43d 100644 --- a/virtqueue/available_ring.go +++ b/virtqueue/available_ring.go @@ -13,7 +13,7 @@ const ( // availableRingFlagNoInterrupt is used by the guest to advise the host to // not interrupt it when consuming a buffer. It's unreliable, so it's simply // an optimization. - availableRingFlagNoInterrupt availableRingFlag = 1 << iota + availableRingFlagNoInterrupt availableRingFlag = 1 << iota //nolint:unused ) // availableRingSize is the number of bytes needed to store an [AvailableRing] diff --git a/virtqueue/descriptor.go b/virtqueue/descriptor.go index fc6b3ef..7dbda60 100644 --- a/virtqueue/descriptor.go +++ b/virtqueue/descriptor.go @@ -14,7 +14,7 @@ const ( // descriptors to provide an additional layer of indirection. // Only allowed when the [virtio.FeatureIndirectDescriptors] feature was // negotiated. - descriptorFlagIndirect + descriptorFlagIndirect //nolint:unused ) // descriptorSize is the number of bytes needed to store a [Descriptor] in diff --git a/virtqueue/descriptor_table.go b/virtqueue/descriptor_table.go index 8f99872..3fea609 100644 --- a/virtqueue/descriptor_table.go +++ b/virtqueue/descriptor_table.go @@ -347,7 +347,7 @@ func (dt *DescriptorTable) getDescriptorChain(head uint16) (outBuffers, inBuffer next = desc.next } - return + return outBuffers, inBuffers, nil } // freeDescriptorChain can be used to free a descriptor chain when it is no diff --git a/virtqueue/descriptor_table_internal_test.go b/virtqueue/descriptor_table_internal_test.go index 38c0b89..32b3dfb 100644 --- a/virtqueue/descriptor_table_internal_test.go +++ b/virtqueue/descriptor_table_internal_test.go @@ -51,7 +51,7 @@ func TestDescriptorTable_DescriptorChains(t *testing.T) { next uint16 } assertDescriptorTable := func(expected [queueSize]desc) { - for i := 0; i < queueSize; i++ { + for i := range queueSize { actualDesc := &dt.descriptors[i] expectedDesc := &expected[i] assert.Equal(t, uint32(len(expectedDesc.buffer)), actualDesc.length) @@ -400,7 +400,7 @@ func TestDescriptorTable_DescriptorChains(t *testing.T) { func makeTestBuffer(t *testing.T, length int) []byte { t.Helper() buf := make([]byte, length) - for i := 0; i < length; i++ { + for i := range length { buf[i] = byte(length - i) } return buf diff --git a/virtqueue/eventfd_test.go b/virtqueue/eventfd_test.go index c42b3be..430c988 100644 --- a/virtqueue/eventfd_test.go +++ b/virtqueue/eventfd_test.go @@ -1,4 +1,4 @@ -package virtqueue +package virtqueue_test import ( "testing" diff --git a/virtqueue/size_test.go b/virtqueue/size_test.go index 707f58c..1b3825c 100644 --- a/virtqueue/size_test.go +++ b/virtqueue/size_test.go @@ -1,8 +1,9 @@ -package virtqueue +package virtqueue_test import ( "testing" + "github.com/hetznercloud/virtio-go/virtqueue" "github.com/stretchr/testify/assert" ) @@ -48,7 +49,7 @@ func TestCheckQueueSize(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := CheckQueueSize(tt.queueSize) + err := virtqueue.CheckQueueSize(tt.queueSize) if tt.containsErr != "" { assert.ErrorContains(t, err, tt.containsErr) } else { diff --git a/virtqueue/used_ring.go b/virtqueue/used_ring.go index b823184..537a7ab 100644 --- a/virtqueue/used_ring.go +++ b/virtqueue/used_ring.go @@ -13,7 +13,7 @@ const ( // usedRingFlagNoNotify is used by the host to advise the guest to not // kick it when adding a buffer. It's unreliable, so it's simply an // optimization. Guest will still kick when it's out of buffers. - usedRingFlagNoNotify usedRingFlag = 1 << iota + usedRingFlagNoNotify usedRingFlag = 1 << iota //nolint:unused ) // usedRingSize is the number of bytes needed to store a [UsedRing] with the