From 7ab22699982f31b326e870153be0f5375fb34d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Fri, 24 Apr 2026 05:30:58 +0800 Subject: [PATCH] Fix USB/IP unlink and runtime test gating --- service/usbip/data_protocol.go | 1 + service/usbip/data_protocol_test.go | 4 +-- service/usbip/linux_interop_test.go | 38 ++++++++++++++++--------- service/usbip/linux_test.go | 36 ++++++++++++++++++----- service/usbip/server_darwin.go | 44 +++++++++++++++++++++-------- service/usbip/server_darwin_test.go | 31 ++++++++++++++++++++ 6 files changed, 120 insertions(+), 34 deletions(-) create mode 100644 service/usbip/server_darwin_test.go diff --git a/service/usbip/data_protocol.go b/service/usbip/data_protocol.go index cc6b86459..3cd454c3c 100644 --- a/service/usbip/data_protocol.go +++ b/service/usbip/data_protocol.go @@ -22,6 +22,7 @@ const ( maxUSBIPTransferBufferLength = 16 << 20 maxUSBIPIsoPackets = 4096 nonIsoPacketCount = -1 + usbipStatusECONNRESET = -104 ) type DataHeader struct { diff --git a/service/usbip/data_protocol_test.go b/service/usbip/data_protocol_test.go index 084700189..419057df4 100644 --- a/service/usbip/data_protocol_test.go +++ b/service/usbip/data_protocol_test.go @@ -267,7 +267,7 @@ func TestUSBIPUnlinkDelayedFakeTransfer(t *testing.T) { Direction: unlink.Header.Direction, Endpoint: unlink.Header.Endpoint, }, - Status: 0, + Status: usbipStatusECONNRESET, }) }() @@ -297,7 +297,7 @@ func TestUSBIPUnlinkDelayedFakeTransfer(t *testing.T) { require.Equal(t, RetUnlink, header.Command) response, err := ReadUnlinkResponseBody(client, header) require.NoError(t, err) - require.Equal(t, int32(0), response.Status) + require.Equal(t, int32(usbipStatusECONNRESET), response.Status) require.NoError(t, <-serverDone) } diff --git a/service/usbip/linux_interop_test.go b/service/usbip/linux_interop_test.go index 200424c5f..96c769e78 100644 --- a/service/usbip/linux_interop_test.go +++ b/service/usbip/linux_interop_test.go @@ -139,7 +139,9 @@ func ensureTestUDCs(t *testing.T, minCount int) []string { } modprobePath, err := findModprobePath() - require.NoError(t, err) + if err != nil { + t.Skipf("dummy_hcd unavailable: %v", err) + } command := exec.Command(modprobePath, "-r", "dummy_hcd") command.Env = os.Environ() @@ -148,13 +150,20 @@ func ensureTestUDCs(t *testing.T, minCount int) []string { command = exec.Command(modprobePath, "dummy_hcd", "num="+strconv.Itoa(minCount)) command.Env = os.Environ() output, err := command.CombinedOutput() - require.NoErrorf(t, err, "modprobe dummy_hcd num=%d\n%s", minCount, string(output)) + if err != nil { + t.Skipf("dummy_hcd with %d UDCs unavailable: %v\n%s", minCount, err, string(output)) + } - require.Eventually(t, func() bool { - return len(currentUDCNames()) >= minCount - }, 5*time.Second, 100*time.Millisecond) + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if udcs = currentUDCNames(); len(udcs) >= minCount { + return udcs + } + time.Sleep(100 * time.Millisecond) + } - return currentUDCNames() + t.Skipf("dummy_hcd provided %d UDCs, need %d", len(currentUDCNames()), minCount) + return nil } func reserveTestUDC(t *testing.T) string { @@ -276,6 +285,7 @@ func pickFreeTCPPort(t *testing.T) uint16 { func startRealUSBIPServer(t *testing.T, devices []option.USBIPDeviceMatch) (*ServerService, M.Socksaddr) { t.Helper() + requireUSBIPHost(t) serviceInstance, err := NewServerService(context.Background(), newTestLogger(), "usbip-server-test", option.USBIPServerServiceOptions{ ListenOptions: option.ListenOptions{ @@ -297,6 +307,7 @@ func startRealUSBIPServer(t *testing.T, devices []option.USBIPDeviceMatch) (*Ser func startRealUSBIPClient(t *testing.T, destination M.Socksaddr, devices []option.USBIPDeviceMatch) *ClientService { t.Helper() + requireVHCI(t) serviceInstance, err := NewClientService(context.Background(), newTestLogger(), "usbip-client-test", option.USBIPClientServiceOptions{ ServerOptions: option.ServerOptions{ @@ -853,6 +864,7 @@ func (g *testHIDGadget) exerciseImportedIO(t *testing.T, importedHID string) { func bindWithOfficialUSBIP(t *testing.T, tools testUSBIPTools, busid string) { t.Helper() + requireUSBIPHost(t) if driver, err := currentDriver(busid); err == nil && driver == "usbip-host" { return @@ -877,7 +889,7 @@ func TestUSBIPInteropOurServerWithOfficialClientACM(t *testing.T) { requireRoot(t) resetUSBIPInteropState(t) tools := requireUSBIPTools(t) - require.NoError(t, ensureVHCI()) + requireVHCI(t) gadget := newTestACMGadget(t) server, address := startRealUSBIPServer(t, []option.USBIPDeviceMatch{{Serial: gadget.serial}}) @@ -905,7 +917,7 @@ func TestUSBIPInteropOurServerWithOfficialClientHID(t *testing.T) { requireRoot(t) resetUSBIPInteropState(t) tools := requireUSBIPTools(t) - require.NoError(t, ensureVHCI()) + requireVHCI(t) gadget := newTestHIDGadget(t) _, address := startRealUSBIPServer(t, []option.USBIPDeviceMatch{{Serial: gadget.serial}}) @@ -931,7 +943,7 @@ func TestUSBIPInteropOurClientWithOfficialServerACM(t *testing.T) { requireRoot(t) resetUSBIPInteropState(t) tools := requireUSBIPTools(t) - require.NoError(t, ensureVHCI()) + requireVHCI(t) gadget := newTestACMGadget(t) bindWithOfficialUSBIP(t, tools, gadget.busid) @@ -960,7 +972,7 @@ func TestUSBIPInteropOurClientWithOfficialServerHID(t *testing.T) { requireRoot(t) resetUSBIPInteropState(t) tools := requireUSBIPTools(t) - require.NoError(t, ensureVHCI()) + requireVHCI(t) gadget := newTestHIDGadget(t) bindWithOfficialUSBIP(t, tools, gadget.busid) @@ -989,7 +1001,7 @@ func TestUSBIPOfficialServerHasStaticDiscoveryOnly(t *testing.T) { requireRoot(t) resetUSBIPInteropState(t) tools := requireUSBIPTools(t) - require.NoError(t, ensureVHCI()) + requireVHCI(t) first := newTestACMGadget(t) bindWithOfficialUSBIP(t, tools, first.busid) @@ -1023,7 +1035,7 @@ func TestUSBIPOfficialServerHasStaticDiscoveryOnly(t *testing.T) { func TestUSBIPControlHotplugACMReattach(t *testing.T) { requireRoot(t) resetUSBIPInteropState(t) - require.NoError(t, ensureVHCI()) + requireVHCI(t) ensureTestUDCs(t, testUDCCount) server, address := startRealUSBIPServer(t, []option.USBIPDeviceMatch{{ @@ -1056,7 +1068,7 @@ func TestUSBIPControlHotplugACMReattach(t *testing.T) { func TestUSBIPControlImportAllACMAndHID(t *testing.T) { requireRoot(t) resetUSBIPInteropState(t) - require.NoError(t, ensureVHCI()) + requireVHCI(t) ensureTestUDCs(t, testUDCCount) _, address := startRealUSBIPServer(t, []option.USBIPDeviceMatch{ diff --git a/service/usbip/linux_test.go b/service/usbip/linux_test.go index 967a3f17d..2509eedf7 100644 --- a/service/usbip/linux_test.go +++ b/service/usbip/linux_test.go @@ -380,7 +380,9 @@ func requireKernelModule(t *testing.T, module string) { } modprobePath, err := findModprobePath() - require.NoError(t, err) + if err != nil { + t.Skipf("kernel module %s unavailable: %v", module, err) + } ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -389,9 +391,25 @@ func requireKernelModule(t *testing.T, module string) { command.Env = os.Environ() output, err := command.CombinedOutput() if ctx.Err() != nil { - t.Fatalf("modprobe %s timed out: %s", module, string(output)) + t.Skipf("modprobe %s timed out: %s", module, string(output)) + } + if err != nil { + t.Skipf("kernel module %s unavailable: %v: %s", module, err, string(output)) + } +} + +func requireUSBIPHost(t *testing.T) { + t.Helper() + if err := ensureHostDriver(); err != nil { + t.Skipf("usbip-host unavailable: %v", err) + } +} + +func requireVHCI(t *testing.T) { + t.Helper() + if err := ensureVHCI(); err != nil { + t.Skipf("vhci_hcd unavailable: %v", err) } - require.NoErrorf(t, err, "modprobe %s: %s", module, string(output)) } func writeSysfsLine(path string, content string) error { @@ -407,8 +425,12 @@ func newTestUSBGadget(t *testing.T) *testUSBGadget { requireKernelModule(t, "dummy_hcd") udcs, err := os.ReadDir("/sys/class/udc") - require.NoError(t, err) - require.NotEmpty(t, udcs) + if err != nil { + t.Skipf("USB device controllers unavailable: %v", err) + } + if len(udcs) == 0 { + t.Skip("USB device controllers unavailable") + } gadget := &testUSBGadget{ path: filepath.Join("/sys/kernel/config/usb_gadget", fmt.Sprintf("codex_usbip_%d", time.Now().UnixNano())), @@ -1777,8 +1799,8 @@ func TestClientRunControlSessionSyncsAssignmentsOnChanged(t *testing.T) { func TestUSBIPLinuxSmoke(t *testing.T) { requireRoot(t) - require.NoError(t, ensureHostDriver()) - require.NoError(t, ensureVHCI()) + requireUSBIPHost(t) + requireVHCI(t) gadget := newTestUSBGadget(t) device, err := readSysfsDevice(gadget.busid, sysBusDevicePath(gadget.busid)) diff --git a/service/usbip/server_darwin.go b/service/usbip/server_darwin.go index 30b3fa7a8..f27abffb8 100644 --- a/service/usbip/server_darwin.go +++ b/service/usbip/server_darwin.go @@ -483,17 +483,22 @@ type darwinServerDataSession struct { device *darwinUSBHostDevice writeMu sync.Mutex mu sync.Mutex - pending map[uint32]uint8 + pending map[uint32]darwinServerPendingSubmit wg sync.WaitGroup } +type darwinServerPendingSubmit struct { + endpoint uint8 + unlinked bool +} + func newDarwinServerDataSession(ctx context.Context, logger log.ContextLogger, conn net.Conn, device *darwinUSBHostDevice) *darwinServerDataSession { return &darwinServerDataSession{ ctx: ctx, logger: logger, conn: conn, device: device, - pending: make(map[uint32]uint8), + pending: make(map[uint32]darwinServerPendingSubmit), } } @@ -519,8 +524,10 @@ func (s *darwinServerDataSession) serve() error { s.wg.Add(1) go func() { defer s.wg.Done() - defer s.untrackSubmit(command.Header.SeqNum) response := s.handleSubmit(command) + if !s.finishSubmit(command.Header.SeqNum) { + return + } s.writeMu.Lock() err := WriteSubmitResponse(s.conn, response) s.writeMu.Unlock() @@ -533,12 +540,12 @@ func (s *darwinServerDataSession) serve() error { if err != nil { return err } - status := -int32(unix.ECONNRESET) - if endpoint, ok := s.untrackSubmit(command.SeqNum); ok { + status := int32(0) + if endpoint, ok := s.markSubmitUnlinked(command.SeqNum); ok { if err := s.device.abortEndpoint(endpoint); err != nil { s.logger.Debug("abort endpoint 0x", hex8(endpoint), ": ", err) } - status = 0 + status = usbipStatusECONNRESET } s.writeMu.Lock() err = WriteUnlinkResponse(s.conn, UnlinkResponse{ @@ -604,17 +611,30 @@ func (s *darwinServerDataSession) handleSubmit(command SubmitCommand) SubmitResp func (s *darwinServerDataSession) trackSubmit(seq uint32, endpoint uint8) { s.mu.Lock() defer s.mu.Unlock() - s.pending[seq] = endpoint + s.pending[seq] = darwinServerPendingSubmit{endpoint: endpoint} } -func (s *darwinServerDataSession) untrackSubmit(seq uint32) (uint8, bool) { +func (s *darwinServerDataSession) markSubmitUnlinked(seq uint32) (uint8, bool) { s.mu.Lock() defer s.mu.Unlock() - endpoint, ok := s.pending[seq] - if ok { - delete(s.pending, seq) + pending, ok := s.pending[seq] + if !ok { + return 0, false } - return endpoint, ok + pending.unlinked = true + s.pending[seq] = pending + return pending.endpoint, true +} + +func (s *darwinServerDataSession) finishSubmit(seq uint32) bool { + s.mu.Lock() + defer s.mu.Unlock() + pending, ok := s.pending[seq] + if !ok { + return true + } + delete(s.pending, seq) + return !pending.unlinked } func commandEndpoint(command SubmitCommand) uint8 { diff --git a/service/usbip/server_darwin_test.go b/service/usbip/server_darwin_test.go new file mode 100644 index 000000000..77204a643 --- /dev/null +++ b/service/usbip/server_darwin_test.go @@ -0,0 +1,31 @@ +//go:build darwin && cgo + +package usbip + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDarwinServerPendingSubmitUnlinkState(t *testing.T) { + t.Parallel() + + session := &darwinServerDataSession{ + pending: make(map[uint32]darwinServerPendingSubmit), + } + + const endpoint uint8 = 0x81 + session.trackSubmit(7, endpoint) + + unlinkedEndpoint, active := session.markSubmitUnlinked(7) + require.True(t, active) + require.Equal(t, endpoint, unlinkedEndpoint) + require.False(t, session.finishSubmit(7)) + + session.trackSubmit(8, endpoint) + require.True(t, session.finishSubmit(8)) + + _, active = session.markSubmitUnlinked(8) + require.False(t, active) +}