CP-54468 Handle USB network devices in network devices sort

[changlei-li]

The bus-info of a USB-based network device follows the format usb-<PCI address of USB controller>-<USB port path>. To determine the device's order when sorting, use the PCI address of the USB controller.

[psafont]

This function is meant to return a result, but List.nth can return an exception, it's quite a bad fit.

It would be better to use only string manipulation here:

Suggested change

	List.nth (String.split_on_char '-' v) 1
	String.sub v 4 (String.length v - 4)

[changlei-li]

The bus-info format is like usb-0000:44:00.3-1.2. There are three parts when split on '-'. We need the second part. While String.sub v 4 (String.length v - 4) is not correct.
Also needn't to worry about the nth exception. The string is verified starts_with usb-, so it is sure to have nth 1 in the split result.

[psafont]

The comments do not mantion that the string has 2 hyphens, could you add that to the comment?

[changlei-li]

In L249, it is "usb-<PCI address of USB controller>-<USB port path>" with two hyphens already 😄

[lindig]

I would suggest to use sscanf - it can do both checking for usb and extract whatever you need in one go.

let str = "usb-0000:44:00.3-1.2";;

top # Scanf.sscanf str "usb-%[:0-9A-F].%_s" Fun.id;;
- : string = "0000:44:00"

[changlei-li]

In fact, the Pciaddr.of_string in L255 uses sscanf to verify pci addr. See https://github.com/xapi-project/xen-api/blob/v25.23.0/ocaml/xapi-idl/lib/xcp_pci.ml#L6-L9. I would insist to preprocess the bus-info by this method here. While the difference is minimal, to be precise: USB network devices are uncommon on servers, so using starts_with to check is sufficient for most scenarios.

[changlei-li]

@psafont There's no changes needed as the comment is accurate already. Could you change your requested changes?

[minglumlu]

I think the first point is to eliminate any potential exception leaking from the function to make the type of the function clear.

[changlei-li]

OK. I will change the code to make the type of the function clear.

[changlei-li]

Done. The format of PCI addr string will be parsed and checked later in L255 by Pciaddr.of_string.

[changlei-li]

Self test:

Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [BIOS device]=[eth2]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [Kernel name]=[enp68s0f3u1u2c2]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [Permanent MAC]=[B0:3A:F2:B6:05:9F]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [Assigned MAC]=[B0:3A:F2:B6:05:9F]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [ifIndex]=[2]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [Driver]=[rndis_host]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [Driver version]=[6.6.22+0]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [Firmware version]=[RNDIS device]
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.Dev.parse.(fun): [Bus Info]=[usb-0000:44:00.3-1.2]
...
Jul  2 03:01:21 localhost xcp-networkd: [debug||0 |startup|Network_device_order] Network_device_order.sort.(fun) new order: position=2; name=enp68s0f3u1u2c2; MAC=b0:3a:f2:b6:05:9f; PCI=0000:44:00.3; present=true

[lindig]

match String.split_on_char '-' str with
| ["usb"; pci; _] -> pci
| _ -> str

[changlei-li]

Accept. Done.