usbip: fix iso frame rebase, empty-devices, and lease visibility

Three review findings shared one shape: a contract documented in
comments but reimplemented at each call site. Each fix collapses
the contract into a single named home.

- RebaseFrame now rounds up to the least frame >= currentFrame
  with the matching low 8 bits, so scheduled iso submits never
  land in the past. The .m bridge trusts the asap flag instead
  of treating start_frame=0 as ASAP, which also fixes a latent
  int32-wraparound path.
- NewServerService and NewClientService reject empty devices
  with a clear error instead of starting up exporting nothing.
- exportLedger.reservedLocked unifies busy + outstanding lease;
  AvailableExports and snapshotDeviceState consult it so legacy
  devlist and control snapshots no longer advertise a leased
  busid as available. IsBusy renamed to IsReserved across the
  ExportHost interface and call sites.

Two supporting deepenings:
- Drop FrameOracle/IsoScheduler; iso submit is now the free
  EncodeIsoSubmit(currentFrame, ...). The darwin endpoint holds
  currentFrame func() uint64.
- Extract SelectMatches; host_linux and host_darwin Reconcile
  loops share the match/dedup step.
This commit is contained in:
世界
2026-05-16 02:13:13 +08:00
parent 0ebf46b0a3
commit 00e7b652b5
13 changed files with 157 additions and 154 deletions
+3
View File
@@ -43,6 +43,9 @@ type ClientService struct {
}
func NewClientService(ctx context.Context, logger log.ContextLogger, tag string, options option.USBIPClientServiceOptions) (adapter.Service, error) {
if len(options.Devices) == 0 {
return nil, E.New("devices: at least one match is required")
}
for i, m := range options.Devices {
if m.IsZero() {
return nil, E.New("devices[", i, "]: at least one of busid/vendor_id/product_id/serial is required")
+1 -3
View File
@@ -33,7 +33,6 @@ type darwinVirtualController struct {
startTime time.Time
peer *UsbIpPeer
iso *IsoScheduler
controller *darwinUSBHostController
events chan darwinControllerEvent
@@ -67,7 +66,6 @@ func newDarwinVirtualController(ctx context.Context, logger log.ContextLogger, c
devices: make(map[uint8]*darwinUSBHostDeviceSM),
endpoints: make(map[darwinEndpointKey]*darwinEndpoint),
}
c.iso = NewIsoScheduler(c)
return c
}
@@ -234,7 +232,7 @@ func (c *darwinVirtualController) handleEndpointCreate(message darwinCIMessage)
return err
}
key := darwinEndpointKey{device: message.deviceAddress(), endpoint: message.endpointAddress()}
endpoint := newDarwinEndpoint(c.ctx, c.logger, sm, c.peer, c.iso, c.info.DevID(), key)
endpoint := newDarwinEndpoint(c.ctx, c.logger, sm, c.peer, c.CurrentFrame, c.info.DevID(), key)
c.stateAccess.Lock()
c.endpoints[key] = endpoint
c.stateAccess.Unlock()
+18 -18
View File
@@ -17,11 +17,11 @@ type darwinEndpoint struct {
cancel context.CancelFunc
logger log.ContextLogger
sm *darwinUSBHostEndpointSM
peer *UsbIpPeer
iso *IsoScheduler
devID uint32
key darwinEndpointKey
sm *darwinUSBHostEndpointSM
peer *UsbIpPeer
currentFrame func() uint64
devID uint32
key darwinEndpointKey
cmdCh chan darwinCIMessage
doorbellCh chan uint32
@@ -40,20 +40,20 @@ type pendingTransfer struct {
noResponse bool
}
func newDarwinEndpoint(ctx context.Context, logger log.ContextLogger, sm *darwinUSBHostEndpointSM, peer *UsbIpPeer, iso *IsoScheduler, devID uint32, key darwinEndpointKey) *darwinEndpoint {
func newDarwinEndpoint(ctx context.Context, logger log.ContextLogger, sm *darwinUSBHostEndpointSM, peer *UsbIpPeer, currentFrame func() uint64, devID uint32, key darwinEndpointKey) *darwinEndpoint {
ctx, cancel := context.WithCancel(ctx)
e := &darwinEndpoint{
ctx: ctx,
cancel: cancel,
logger: logger,
sm: sm,
peer: peer,
iso: iso,
devID: devID,
key: key,
cmdCh: make(chan darwinCIMessage, 4),
doorbellCh: make(chan uint32, 16),
workerDone: make(chan struct{}),
ctx: ctx,
cancel: cancel,
logger: logger,
sm: sm,
peer: peer,
currentFrame: currentFrame,
devID: devID,
key: key,
cmdCh: make(chan darwinCIMessage, 4),
doorbellCh: make(chan uint32, 16),
workerDone: make(chan struct{}),
}
go e.worker()
return e
@@ -384,7 +384,7 @@ func (e *darwinEndpoint) startIsoTransfer(transfer darwinCITransfer, message dar
}
ciFrame := uint8(message.control >> ciIsochronousTransferControlFramePhase)
asap := message.control&ciIsochronousTransferControlASAP != 0
command := e.iso.EncodeSubmit(SubmitCommand{
command := EncodeIsoSubmit(e.currentFrame(), SubmitCommand{
Header: DataHeader{
Command: CmdSubmit,
DevID: e.devID,
+21 -11
View File
@@ -63,17 +63,31 @@ func newExportLedger(logger log.ContextLogger, ttl time.Duration, now func() tim
}
}
func (l *exportLedger) IsBusy(busid string) bool {
func (l *exportLedger) IsReserved(busid string) bool {
l.slow.Lock()
defer l.slow.Unlock()
return l.busy[busid]
return l.reservedLocked(busid)
}
// reservedLocked reports whether busid is unavailable for new admission:
// either marked busy by an active session or covered by an unexpired
// import lease. Caller must hold l.slow.
func (l *exportLedger) reservedLocked(busid string) bool {
if l.busy[busid] {
return true
}
lease, found := l.leases[busid]
if !found {
return false
}
return l.now().Before(lease.Expires)
}
func (l *exportLedger) AvailableExports() []Export {
l.slow.Lock()
out := make([]Export, 0, len(l.exports))
for busid, export := range l.exports {
if l.busy[busid] {
if l.reservedLocked(busid) {
continue
}
out = append(out, export)
@@ -151,14 +165,13 @@ func (l *exportLedger) BroadcastIfChanged(ctx context.Context) bool {
func (l *exportLedger) TryReserveForImport(ctx context.Context, busid string) (Export, bool, string) {
l.slow.Lock()
export, found := l.exports[busid]
busy := l.busy[busid]
_, leased := l.leases[busid]
reserved := found && l.reservedLocked(busid)
l.slow.Unlock()
if !found {
return nil, false, "unknown busid"
}
identity := export.LeaseIdentity()
if busy || leased {
if reserved {
return nil, false, deviceStateBusy
}
leaseOK, leaseReason := export.LeaseCheck(ctx)
@@ -171,10 +184,7 @@ func (l *exportLedger) TryReserveForImport(ctx context.Context, busid string) (E
if !stillExported || current.LeaseIdentity() != identity {
return nil, false, "unknown busid"
}
if l.busy[busid] {
return nil, false, deviceStateBusy
}
if _, leasedNow := l.leases[busid]; leasedNow {
if l.reservedLocked(busid) {
return nil, false, deviceStateBusy
}
l.busy[busid] = true
@@ -504,7 +514,7 @@ func (l *exportLedger) snapshotDeviceState(ctx context.Context) []DeviceInfoV2 {
l.slow.Lock()
entries := make([]entry, 0, len(l.exports))
for busid, export := range l.exports {
entries = append(entries, entry{export: export, busy: l.busy[busid]})
entries = append(entries, entry{export: export, busy: l.reservedLocked(busid)})
}
l.slow.Unlock()
if len(entries) == 0 {
+2 -2
View File
@@ -140,8 +140,8 @@ func TestConsumeLeaseAndReserveMarksBusyOnSuccess(t *testing.T) {
if reserved != exp {
t.Fatal("expected to reserve the original export instance")
}
if !ledger.IsBusy(exp.busid) {
t.Fatal("expected successful lease reservation to mark busid busy")
if !ledger.IsReserved(exp.busid) {
t.Fatal("expected successful lease reservation to mark busid reserved")
}
}
+1 -1
View File
@@ -16,7 +16,7 @@ import (
type ExportHost interface {
Start(ctx context.Context) error
Close() error
Reconcile(ctx context.Context, isBusy func(busid string) bool) (snapshot map[string]Export, released []string, err error)
Reconcile(ctx context.Context, isReserved func(busid string) bool) (snapshot map[string]Export, released []string, err error)
FinishImport(ctx context.Context, busid string) (released bool, err error)
// Events returning (nil, nil) means "no native event source; rely
// on polling".
+12 -13
View File
@@ -99,23 +99,22 @@ func (h *darwinExportHost) Events(ctx context.Context) (<-chan struct{}, error)
return ch, nil
}
func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid string) bool) (map[string]Export, []string, error) {
func (h *darwinExportHost) Reconcile(ctx context.Context, isReserved func(busid string) bool) (map[string]Export, []string, error) {
devices, err := darwinCopyUSBHostDevices()
if err != nil {
return h.snapshotSelf(), nil, E.Cause(err, "enumerate IOUSBHost devices")
}
keys := make([]DeviceKey, len(devices))
for i := range devices {
keys[i] = devices[i].key
}
desired := make(map[string]darwinUSBHostDeviceInfo)
for _, match := range h.matches {
for i := range devices {
if !matches(match, devices[i].key) {
continue
}
if devices[i].entry.Info.BDeviceClass == 0x09 {
h.logger.Warn("skip hub device ", devices[i].key.BusID, " matched by ", describeMatch(match))
continue
}
desired[devices[i].key.BusID] = devices[i]
for _, idx := range SelectMatches(h.matches, keys) {
if devices[idx].entry.Info.BDeviceClass == 0x09 {
h.logger.Warn("skip hub device ", devices[idx].key.BusID)
continue
}
desired[devices[idx].key.BusID] = devices[idx]
}
h.access.Lock()
@@ -134,7 +133,7 @@ func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid stri
continue
}
if exp, ok := current[busid]; ok {
if isBusy(busid) {
if isReserved(busid) {
toStale = append(toStale, darwinStaleMark{busid: busid, pendingRegistryID: info.registryID})
continue
}
@@ -160,7 +159,7 @@ func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid stri
if _, ok := desired[busid]; ok {
continue
}
if isBusy(busid) {
if isReserved(busid) {
toStale = append(toStale, darwinStaleMark{busid: busid})
continue
}
+34 -36
View File
@@ -245,18 +245,18 @@ const (
ueventListenerBackoffMax = 30 * time.Second
)
func classifyLinuxReconcile(current map[string]*linuxExport, desired map[string]sysfsDevice, isBusy func(busid string) bool) linuxReconcilePlan {
func classifyLinuxReconcile(current map[string]*linuxExport, desired map[string]sysfsDevice, isReserved func(busid string) bool) linuxReconcilePlan {
remainingDesired := maps.Clone(desired)
plan := linuxReconcilePlan{
toBind: make(map[string]sysfsDevice),
}
for busid, exp := range current {
device, wanted := remainingDesired[busid]
busy := isBusy(busid)
reserved := isReserved(busid)
identityMatches := wanted && exp.identity.Equal(newLinuxExportIdentity(device))
switch {
case exp.stale:
if busy {
if reserved {
delete(remainingDesired, busid)
continue
}
@@ -264,7 +264,7 @@ func classifyLinuxReconcile(current map[string]*linuxExport, desired map[string]
plan.released = append(plan.released, busid)
case identityMatches:
delete(remainingDesired, busid)
case busy:
case reserved:
plan.toStale = append(plan.toStale, busid)
delete(remainingDesired, busid)
default:
@@ -273,7 +273,7 @@ func classifyLinuxReconcile(current map[string]*linuxExport, desired map[string]
}
}
for busid, device := range remainingDesired {
if isBusy(busid) {
if isReserved(busid) {
continue
}
plan.toBind[busid] = device
@@ -281,49 +281,47 @@ func classifyLinuxReconcile(current map[string]*linuxExport, desired map[string]
return plan
}
func (h *linuxExportHost) Reconcile(ctx context.Context, isBusy func(busid string) bool) (map[string]Export, []string, error) {
func (h *linuxExportHost) Reconcile(ctx context.Context, isReserved func(busid string) bool) (map[string]Export, []string, error) {
devices, err := listUSBDevices()
if err != nil {
return h.snapshotSelf(), nil, E.Cause(err, "enumerate usb devices")
}
desired := make(map[string]sysfsDevice)
for _, m := range h.matches {
for i := range devices {
deviceKey := DeviceKey{
BusID: devices[i].BusID,
VendorID: devices[i].VendorID,
ProductID: devices[i].ProductID,
Serial: devices[i].Serial,
}
if !matches(m, deviceKey) {
continue
}
path := devices[i].Path
isVHCIImport := strings.Contains(path, "vhci_hcd")
if !isVHCIImport {
realPath, err := filepath.EvalSymlinks(path)
if err == nil {
isVHCIImport = strings.Contains(realPath, "vhci_hcd")
}
}
if isVHCIImport {
h.logger.Debug("skip vhci-imported device ", devices[i].BusID, " matched by ", describeMatch(m))
continue
}
if devices[i].DeviceClass == 0x09 {
h.logger.Warn("skip hub device ", devices[i].BusID, " matched by ", describeMatch(m))
continue
}
desired[devices[i].BusID] = devices[i]
keys := make([]DeviceKey, len(devices))
for i := range devices {
keys[i] = DeviceKey{
BusID: devices[i].BusID,
VendorID: devices[i].VendorID,
ProductID: devices[i].ProductID,
Serial: devices[i].Serial,
}
}
desired := make(map[string]sysfsDevice)
for _, idx := range SelectMatches(h.matches, keys) {
path := devices[idx].Path
isVHCIImport := strings.Contains(path, "vhci_hcd")
if !isVHCIImport {
realPath, err := filepath.EvalSymlinks(path)
if err == nil {
isVHCIImport = strings.Contains(realPath, "vhci_hcd")
}
}
if isVHCIImport {
h.logger.Debug("skip vhci-imported device ", devices[idx].BusID)
continue
}
if devices[idx].DeviceClass == 0x09 {
h.logger.Warn("skip hub device ", devices[idx].BusID)
continue
}
desired[devices[idx].BusID] = devices[idx]
}
h.access.Lock()
current := make(map[string]*linuxExport, len(h.exports))
maps.Copy(current, h.exports)
h.access.Unlock()
plan := classifyLinuxReconcile(current, desired, isBusy)
plan := classifyLinuxReconcile(current, desired, isReserved)
committed := make(map[string]*linuxExport, len(current)+len(plan.toBind))
maps.Copy(committed, current)
var reconcileErrors []error
+15 -29
View File
@@ -1,44 +1,30 @@
package usbip
type FrameOracle interface {
CurrentFrame() uint64
}
type IsoScheduler struct {
oracle FrameOracle
}
func NewIsoScheduler(oracle FrameOracle) *IsoScheduler {
return &IsoScheduler{oracle: oracle}
}
func (s *IsoScheduler) EncodeSubmit(base SubmitCommand, ciFrame uint8, asap bool) SubmitCommand {
// EncodeIsoSubmit fills the isochronous SUBMIT fields on base. When asap is
// true, the wire-level ASAP flag is set and StartFrame is zeroed. Otherwise
// RebaseFrame recovers the absolute frame number from the controller's
// monotonic counter and ciFrame's 8 bits, then StartFrame carries the low 32
// bits across the wire.
func EncodeIsoSubmit(currentFrame uint64, base SubmitCommand, ciFrame uint8, asap bool) SubmitCommand {
if asap {
base.TransferFlags |= usbipTransferFlagIsoASAP
base.StartFrame = 0
return base
}
rebased := RebaseFrame(s.oracle.CurrentFrame(), ciFrame)
rebased := RebaseFrame(currentFrame, ciFrame)
base.StartFrame = int32(uint32(rebased))
return base
}
// RebaseFrame returns the absolute USB frame number whose low 8 bits equal
// low8 and is closest to currentFrame. Apple's IOUSBHostCI iso messages only
// carry the low 8 bits of the frame number; this rebases against the
// controller's monotonic frame counter so the wire start_frame survives the
// 256-frame wrap.
// RebaseFrame returns the smallest absolute USB frame number whose low 8
// bits equal low8 and is >= currentFrame. Apple's IOUSBHostCI iso messages
// only carry the low 8 bits; the host recovers the high bits against the
// controller's monotonic counter. firstFrameNumber must be in the future;
// 0 is reserved for ASAP and handled separately by the caller.
func RebaseFrame(currentFrame uint64, low8 uint8) uint64 {
target := uint64(low8)
if currentFrame < target {
return target
}
delta := currentFrame - target
high := delta / 256
base := high*256 + target
nextBase := base + 256
if nextBase-currentFrame <= currentFrame-base {
return nextBase
base := currentFrame&^0xff | uint64(low8)
if base < currentFrame {
base += 256
}
return base
}
+17 -35
View File
@@ -6,40 +6,23 @@ import (
"github.com/stretchr/testify/require"
)
type frameOracleStub uint64
func (f frameOracleStub) CurrentFrame() uint64 { return uint64(f) }
func TestRebaseFrameAcrossWraps(t *testing.T) {
tests := []struct {
current uint64
low8 uint8
want uint64
}{
{0, 0, 0},
{0, 5, 5},
{255, 0, 256},
{255, 255, 255},
{300, 0, 256},
{300, 44, 300},
{300, 100, 356},
{300, 200, 200},
{700, 10, 778},
{700, 188, 700},
{1<<32 - 1, 0, 1 << 32},
{128, 0, 256},
{127, 0, 0},
{1024, 254, 1022},
func TestRebaseFrameIsLeastUpperBound(t *testing.T) {
samples := []uint64{
0, 1, 127, 128, 255, 256, 300, 1024,
1<<31 - 1, 1 << 31, 1<<32 - 1, 1 << 32, 1 << 63,
}
for _, tt := range tests {
got := RebaseFrame(tt.current, tt.low8)
require.Equalf(t, tt.want, got, "RebaseFrame(%d, %d)", tt.current, tt.low8)
for _, current := range samples {
for low := range 256 {
got := RebaseFrame(current, uint8(low))
require.GreaterOrEqualf(t, got, current, "current=%d low=%d", current, low)
require.Lessf(t, got-current, uint64(256), "current=%d low=%d got=%d", current, low, got)
require.Equalf(t, uint64(low), got&0xff, "current=%d low=%d got=%d", current, low, got)
}
}
}
func TestIsoSchedulerEncodeSubmitASAP(t *testing.T) {
scheduler := NewIsoScheduler(frameOracleStub(500))
command := scheduler.EncodeSubmit(SubmitCommand{
func TestEncodeIsoSubmitASAP(t *testing.T) {
command := EncodeIsoSubmit(500, SubmitCommand{
Header: DataHeader{Command: CmdSubmit, Direction: USBIPDirOut, Endpoint: 1},
TransferBufferLength: 16,
}, 99, true)
@@ -47,14 +30,13 @@ func TestIsoSchedulerEncodeSubmitASAP(t *testing.T) {
require.Equal(t, int32(0), command.StartFrame)
}
func TestIsoSchedulerEncodeSubmitScheduled(t *testing.T) {
scheduler := NewIsoScheduler(frameOracleStub(700))
command := scheduler.EncodeSubmit(SubmitCommand{
func TestEncodeIsoSubmitScheduled(t *testing.T) {
command := EncodeIsoSubmit(300, SubmitCommand{
Header: DataHeader{Command: CmdSubmit, Direction: USBIPDirIn, Endpoint: 0x82},
TransferBufferLength: 64,
}, 10, false)
}, 200, false)
require.Equal(t, int32(0), command.TransferFlags&usbipTransferFlagIsoASAP)
require.Equal(t, int32(778), command.StartFrame)
require.Equal(t, int32(456), command.StartFrame)
}
func TestScatterIsoResponseHonorsPacketOffsets(t *testing.T) {
+25
View File
@@ -1,6 +1,8 @@
package usbip
import (
"slices"
"github.com/sagernet/sing-box/option"
)
@@ -29,3 +31,26 @@ func matches(m option.USBIPDeviceMatch, d DeviceKey) bool {
}
return true
}
// SelectMatches returns the indexes of keys that match at least one
// non-zero pattern. Indexes are deduplicated and returned in ascending
// order so callers iterate devices in a stable sequence.
func SelectMatches(patterns []option.USBIPDeviceMatch, keys []DeviceKey) []int {
if len(patterns) == 0 || len(keys) == 0 {
return nil
}
hit := make(map[int]struct{})
for _, pattern := range patterns {
for i := range keys {
if matches(pattern, keys[i]) {
hit[i] = struct{}{}
}
}
}
out := make([]int, 0, len(hit))
for i := range hit {
out = append(out, i)
}
slices.Sort(out)
return out
}
+4 -1
View File
@@ -39,6 +39,9 @@ type ServerService struct {
}
func NewServerService(ctx context.Context, logger log.ContextLogger, tag string, options option.USBIPServerServiceOptions) (adapter.Service, error) {
if len(options.Devices) == 0 {
return nil, E.New("devices: at least one match is required")
}
for i, m := range options.Devices {
if m.IsZero() {
return nil, E.New("devices[", i, "]: at least one of busid/vendor_id/product_id/serial is required")
@@ -167,7 +170,7 @@ func (s *ServerService) reconcileAndBroadcast(notify bool) error {
if s.ctx != nil && s.ctx.Err() != nil {
return nil
}
snapshot, released, err := s.host.Reconcile(s.ctx, s.ledger.IsBusy)
snapshot, released, err := s.host.Reconcile(s.ctx, s.ledger.IsReserved)
s.ledger.ApplyHostSnapshot(snapshot, released)
if notify {
s.ledger.BroadcastIfChanged(s.ctx)
+4 -5
View File
@@ -689,11 +689,10 @@ bool box_usbhost_device_iso(box_usbhost_device_t *device, uint8_t endpoint, uint
}
NSError *error = nil;
// Apple's IOUSBHostPipe iso API uses firstFrameNumber=0 as the ASAP
// signal; there is no separate options bit. Honor the wire-level ASAP
// flag explicitly so a caller that genuinely wants "scheduled at
// frame 0" (rare) cannot be mistaken for ASAP via the old start_frame>0
// sentinel.
uint64_t firstFrameNumber = asap ? 0 : (start_frame > 0 ? (uint64_t)start_frame : 0);
// signal. Trust the wire-level ASAP flag end-to-end; round-trip the
// int32 bit pattern through uint32 so a Go-side wraparound stays
// non-negative when widened to uint64.
uint64_t firstFrameNumber = asap ? 0 : (uint64_t)(uint32_t)start_frame;
BOOL ok = [pipe sendIORequestWithData:payload
transactionList:transactions
transactionListCount:packet_count