usbip: address review findings

This commit is contained in:
世界 2026-04-24 08:52:43 +08:00
parent a9057b386f
commit 794e1bcf15
No known key found for this signature in database
GPG key ID: CD109927C34A63C4
13 changed files with 80 additions and 56 deletions

View file

@ -48,9 +48,12 @@ jobs:
echo "LDFLAGS_SHARED=$(cat release/LDFLAGS)" >> "$GITHUB_ENV"
- name: Install usbip tools
if: matrix.os == 'ubuntu-latest'
shell: bash
run: |
sudo apt-get update
sudo apt-get install -y linux-tools-common
sudo apt-get install -y linux-tools-common "linux-tools-$(uname -r)"
usbip version
usbipd --version
- name: Test (unix)
if: matrix.os != 'windows-latest'
run: go test -v -exec sudo -tags "$BUILD_TAGS" -ldflags "$LDFLAGS_SHARED" ./...

View file

@ -795,12 +795,7 @@ func assignMatchedBusIDs(targets []clientTarget, current []string, entries []Dev
if busid == "" {
continue
}
keysByBusID[busid] = DeviceKey{
BusID: busid,
VendorID: entries[i].Info.IDVendor,
ProductID: entries[i].Info.IDProduct,
Serial: entries[i].Info.SerialString(),
}
keysByBusID[busid] = entryDeviceKey(entries[i])
}
nextAssigned := make([]string, len(targets))
@ -846,12 +841,7 @@ func assignMatchedBusIDs(targets []clientTarget, current []string, entries []Dev
func firstMatchingUnclaimedBusID(match option.USBIPDeviceMatch, entries []DeviceEntry, reserved map[string]struct{}) string {
for i := range entries {
key := DeviceKey{
BusID: entries[i].Info.BusIDString(),
VendorID: entries[i].Info.IDVendor,
ProductID: entries[i].Info.IDProduct,
Serial: entries[i].Info.SerialString(),
}
key := entryDeviceKey(entries[i])
if _, claimed := reserved[key.BusID]; claimed {
continue
}

View file

@ -293,7 +293,7 @@ func deviceInfoV2FromEntry(entry DeviceEntry, backend string, stableID string, s
StableID: stableID,
Backend: backend,
Path: entry.Info.PathString(),
Serial: entry.Info.SerialString(),
Serial: entrySerial(entry),
VendorID: entry.Info.IDVendor,
ProductID: entry.Info.IDProduct,
BCDDevice: entry.Info.BCDDevice,
@ -313,7 +313,7 @@ func deviceInfoV2FromEntry(entry DeviceEntry, backend string, stableID string, s
func (d DeviceInfoV2) toDeviceEntry() DeviceEntry {
var info DeviceInfoTruncated
encodePathField(&info.Path, d.Path, d.Serial)
encodePathField(&info.Path, d.Path)
copy(info.BusID[:], d.BusID)
info.Speed = d.Speed
info.IDVendor = d.VendorID
@ -333,7 +333,7 @@ func (d DeviceInfoV2) toDeviceEntry() DeviceEntry {
BInterfaceProtocol: d.Interfaces[i].Protocol,
}
}
return DeviceEntry{Info: info, Interfaces: interfaces}
return DeviceEntry{Info: info, Interfaces: interfaces, Serial: d.Serial}
}
func (d DeviceInfoV2) key() DeviceKey {

View file

@ -674,7 +674,7 @@ func truncateDarwinFakeDescriptor(data []byte, length int) []byte {
func darwinFakeDeviceEntry() DeviceEntry {
var info DeviceInfoTruncated
encodePathField(&info.Path, "fake-darwin-usbip", "codex-usbip-fake")
encodePathField(&info.Path, "fake-darwin-usbip")
copy(info.BusID[:], darwinFakeBusID)
info.BusNum = 1
info.DevNum = 1
@ -686,7 +686,8 @@ func darwinFakeDeviceEntry() DeviceEntry {
info.BNumConfigurations = 1
info.BNumInterfaces = 1
return DeviceEntry{
Info: info,
Info: info,
Serial: "codex-usbip-fake",
Interfaces: []DeviceInterface{{
BInterfaceClass: 0xff,
BInterfaceSubClass: 0x42,

View file

@ -108,12 +108,30 @@ func requireUSBIPTools(t *testing.T) testUSBIPTools {
if usbipErr != nil || usbipdErr != nil {
t.Skip("usbip and usbipd are required")
}
requireRunnableUSBIPTool(t, usbipPath, "version")
requireRunnableUSBIPTool(t, usbipdPath, "--version")
return testUSBIPTools{
usbip: usbipPath,
usbipd: usbipdPath,
}
}
func requireRunnableUSBIPTool(t *testing.T, path string, args ...string) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
command := exec.CommandContext(ctx, path, args...)
command.Env = os.Environ()
output, err := command.CombinedOutput()
if ctx.Err() != nil {
t.Skipf("%s %s timed out", path, strings.Join(args, " "))
}
if err != nil {
t.Skipf("%s is unavailable: %v\n%s", path, err, strings.TrimSpace(string(output)))
}
}
func currentUDCNames() []string {
entries, err := os.ReadDir("/sys/class/udc")
if err != nil {

View file

@ -600,9 +600,9 @@ func TestAssignMatchedBusIDs(t *testing.T) {
first := newTestDevice("1-2", 0x1d6b, 0x0002, "first", SpeedHigh)
second := newTestDevice("1-3", 0x1d6b, 0x0002, "second", SpeedHigh)
entries := []DeviceEntry{
{Info: fixed.toProtocol()},
{Info: first.toProtocol()},
{Info: second.toProtocol()},
fixed.toDeviceEntry(),
first.toDeviceEntry(),
second.toDeviceEntry(),
}
require.Equal(t, []string{"1-1", "1-3", "1-2"}, assignMatchedBusIDs(
@ -969,7 +969,7 @@ func TestServerBuildDevListEntriesFiltersUnavailableAndRefreshFailures(t *testin
entries := server.buildDevListEntries()
require.Len(t, entries, 1)
require.Equal(t, "1-1", entries[0].Info.BusIDString())
require.Equal(t, "ok", entries[0].Info.SerialString())
require.Equal(t, "ok", entries[0].Serial)
}
func TestServerHandleImportWithOpaqueConnRelay(t *testing.T) {
@ -2036,7 +2036,7 @@ func TestClientSyncRemoteStateAndResetControlStateRebuildsV2Map(t *testing.T) {
defer cancel()
device := newTestDevice("1-1", 0x1d6b, 0x0002, "serial-1", SpeedHigh)
entry := DeviceEntry{Info: device.toProtocol(), Interfaces: device.Interfaces}
entry := device.toDeviceEntry()
listener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
defer listener.Close()

View file

@ -80,6 +80,7 @@ type DeviceInterface struct {
type DeviceEntry struct {
Info DeviceInfoTruncated
Interfaces []DeviceInterface
Serial string // internal metadata carried by control V2, not OP_REP_* wire data
}
type ImportExtRequest struct {
@ -279,12 +280,8 @@ func (d *DeviceInfoTruncated) DevID() uint32 {
return (d.BusNum << 16) | (d.DevNum & 0xffff)
}
func encodePathField(dst *[256]byte, path, serial string) {
func encodePathField(dst *[256]byte, path string) {
copy(dst[:], path)
if serial == "" || len(path) >= len(dst)-1 {
return
}
copy(dst[len(path)+1:], "serial="+serial)
}
func cstring(b []byte) string {
@ -308,3 +305,19 @@ func trailingCString(b []byte) string {
}
return ""
}
func entrySerial(entry DeviceEntry) string {
if entry.Serial != "" {
return entry.Serial
}
return entry.Info.SerialString()
}
func entryDeviceKey(entry DeviceEntry) DeviceKey {
return DeviceKey{
BusID: entry.Info.BusIDString(),
VendorID: entry.Info.IDVendor,
ProductID: entry.Info.IDProduct,
Serial: entrySerial(entry),
}
}

View file

@ -247,7 +247,7 @@ func TestOpRepDevListRoundTrip(t *testing.T) {
t.Parallel()
var path [256]byte
encodePathField(&path, "/sys/bus/usb/devices/1-2", "serial-1")
encodePathField(&path, "/sys/bus/usb/devices/1-2")
entries := []DeviceEntry{{
Info: DeviceInfoTruncated{
@ -293,13 +293,13 @@ func TestDeviceInfoHelpers(t *testing.T) {
t.Parallel()
var info DeviceInfoTruncated
encodePathField(&info.Path, "/sys/bus/usb/devices/1-2", "serial-1")
encodePathField(&info.Path, "/sys/bus/usb/devices/1-2")
copy(info.BusID[:], "1-2")
info.BusNum = 3
info.DevNum = 9
require.Equal(t, "/sys/bus/usb/devices/1-2", info.PathString())
require.Equal(t, "serial-1", info.SerialString())
require.Empty(t, info.SerialString())
require.Equal(t, "1-2", info.BusIDString())
require.Equal(t, uint32(0x00030009), info.DevID())
}
@ -308,7 +308,7 @@ func TestDeviceInfoV2RoundTrip(t *testing.T) {
t.Parallel()
var path [256]byte
encodePathField(&path, "/sys/bus/usb/devices/1-2", "serial-1")
encodePathField(&path, "/sys/bus/usb/devices/1-2")
entry := DeviceEntry{
Info: DeviceInfoTruncated{
Path: path,
@ -326,6 +326,7 @@ func TestDeviceInfoV2RoundTrip(t *testing.T) {
BNumInterfaces: 1,
},
Interfaces: []DeviceInterface{{BInterfaceClass: 0xff, BInterfaceSubClass: 1, BInterfaceProtocol: 2}},
Serial: "serial-1",
}
copy(entry.Info.BusID[:], "1-2")
@ -336,18 +337,22 @@ func TestDeviceInfoV2RoundTrip(t *testing.T) {
require.Equal(t, DeviceKey{BusID: "1-2", VendorID: 0x1d6b, ProductID: 0x0002, Serial: "serial-1"}, info.key())
roundTrip := info.toDeviceEntry()
require.Equal(t, "1-2", roundTrip.Info.BusIDString())
require.Equal(t, "serial-1", roundTrip.Info.SerialString())
require.Equal(t, "serial-1", roundTrip.Serial)
require.Empty(t, roundTrip.Info.SerialString())
require.Equal(t, uint16(0x1d6b), roundTrip.Info.IDVendor)
require.Equal(t, uint16(0x0002), roundTrip.Info.IDProduct)
require.Equal(t, entry.Interfaces, roundTrip.Interfaces)
}
func TestEncodePathFieldSkipsSerialWithoutRoom(t *testing.T) {
func TestEncodePathFieldZeroPadsPath(t *testing.T) {
t.Parallel()
var info DeviceInfoTruncated
encodePathField(&info.Path, strings.Repeat("a", len(info.Path)-1), "serial-1")
encodePathField(&info.Path, "/sys/bus/usb/devices/1-2")
pathLen := len("/sys/bus/usb/devices/1-2")
require.Equal(t, byte(0), info.Path[pathLen])
require.Equal(t, make([]byte, len(info.Path)-pathLen-1), info.Path[pathLen+1:])
require.Empty(t, info.SerialString())
}

View file

@ -479,10 +479,7 @@ func (s *ServerService) buildDevListEntries() []DeviceEntry {
s.logger.Debug("refresh ", busid, ": ", err)
continue
}
entries = append(entries, DeviceEntry{
Info: d.toProtocol(),
Interfaces: d.Interfaces,
})
entries = append(entries, d.toDeviceEntry())
}
return entries
}
@ -773,7 +770,7 @@ func (s *ServerService) buildDeviceStateV2() []DeviceInfoV2 {
state = deviceStateUnavailable
reason = statusErr.Error()
}
entry := DeviceEntry{Info: dev.toProtocol(), Interfaces: dev.Interfaces}
entry := dev.toDeviceEntry()
devices = append(devices, deviceInfoV2FromEntry(entry, "linux-sysfs", linuxStableID(dev), state, status, reason))
}
return devices

View file

@ -36,12 +36,7 @@ func assignMatchedBusIDs(targets []clientTarget, current []string, entries []Dev
if busid == "" {
continue
}
keysByBusID[busid] = DeviceKey{
BusID: busid,
VendorID: entries[i].Info.IDVendor,
ProductID: entries[i].Info.IDProduct,
Serial: entries[i].Info.SerialString(),
}
keysByBusID[busid] = entryDeviceKey(entries[i])
}
nextAssigned := make([]string, len(targets))
reserved := make(map[string]struct{}, len(targets))
@ -83,12 +78,7 @@ func assignMatchedBusIDs(targets []clientTarget, current []string, entries []Dev
func firstMatchingUnclaimedBusID(match option.USBIPDeviceMatch, entries []DeviceEntry, reserved map[string]struct{}) string {
for i := range entries {
key := DeviceKey{
BusID: entries[i].Info.BusIDString(),
VendorID: entries[i].Info.IDVendor,
ProductID: entries[i].Info.IDProduct,
Serial: entries[i].Info.SerialString(),
}
key := entryDeviceKey(entries[i])
if _, claimed := reserved[key.BusID]; claimed {
continue
}

View file

@ -57,7 +57,7 @@ func (d *sysfsDevice) key() DeviceKey {
func (d *sysfsDevice) toProtocol() DeviceInfoTruncated {
var info DeviceInfoTruncated
encodePathField(&info.Path, d.Path, d.Serial)
encodePathField(&info.Path, d.Path)
copy(info.BusID[:], d.BusID)
info.BusNum = d.BusNum
info.DevNum = d.DevNum
@ -74,6 +74,14 @@ func (d *sysfsDevice) toProtocol() DeviceInfoTruncated {
return info
}
func (d *sysfsDevice) toDeviceEntry() DeviceEntry {
return DeviceEntry{
Info: d.toProtocol(),
Interfaces: d.Interfaces,
Serial: d.Serial,
}
}
type vhciStatusRecord struct {
hub string
port int

View file

@ -4,7 +4,6 @@ package usbip
import (
"bytes"
"os"
"golang.org/x/sys/unix"
)
@ -20,7 +19,6 @@ func newUEventListener() (*ueventListener, error) {
}
addr := &unix.SockaddrNetlink{
Family: unix.AF_NETLINK,
Pid: uint32(os.Getpid()),
Groups: 1,
}
if err := unix.Bind(fd, addr); err != nil {

View file

@ -451,9 +451,10 @@ func darwinDeviceInfoFromC(info *C.box_usbhost_device_info_t) darwinUSBHostDevic
BNumConfigurations: uint8(info.num_configurations),
BNumInterfaces: uint8(info.interface_count),
},
Serial: serial,
}
copy(entry.Info.BusID[:], busid)
encodePathField(&entry.Info.Path, path, serial)
encodePathField(&entry.Info.Path, path)
interfaceCount := int(info.interface_count)
if interfaceCount > C.BOX_USBHOST_MAX_INTERFACES {
interfaceCount = C.BOX_USBHOST_MAX_INTERFACES