Adapt network interfaces sorting #6456
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`Network_utils.is_sorted_by_script` checks interface-rename-data dir. When true: Follow the legacy behavior, the new added interface_order field is always None. When false: Use `Network_device_order.sort` to sort the interfaces, store the result in config.interface_order. Signed-off-by: Changlei Li <[email protected]>
Signed-off-by: Changlei Li <[email protected]>
Xapi get interface position from `get_interface_positions`, instead of getting position from "ethx" name. Signed-off-by: Changlei Li <[email protected]>
Some devices like ibft may not be in networkd sort result but need to build pif, network for it. So the devices need be got by `get_all`. Signed-off-by: Changlei Li <[email protected]>
minglumlu
reviewed
May 7, 2025
minglumlu
reviewed
May 7, 2025
minglumlu
reviewed
May 7, 2025
minglumlu
reviewed
May 7, 2025
minglumlu
reviewed
May 7, 2025
| @@ -420,6 +420,18 @@ module Interface_API (R : RPC) = struct | |||
| ["Get list of all interface names"] | |||
| (debug_info_p @-> unit_p @-> returning iface_list_p err) | |||
|
|
|||
| let get_interface_positions = | |||
There was a problem hiding this comment.
Could this be done within Interface.get_all?
There was a problem hiding this comment.
I prefer to add a new interface to keep the interface function simple and not modify the old one for potential break.
There was a problem hiding this comment.
OK. I thought it's would be simpler to include this in the Interface.get_all from the interface design's point of view. But I'm not sure about this.
Hi @robhoes @psafont @lindig
May I please have your thoughts on this?
BTW, a minor comment on the name: Interface.get_interface_positions seems a bit redundant. If we are going to use this, how about Interface.get_order?
There was a problem hiding this comment.
I do prefer the name Interface.get_{positions,order,slots}, But I would keep the function separate, because they have different semantic meaning. was there only a single user of get_all?
minglumlu
reviewed
May 7, 2025
psafont
reviewed
May 12, 2025
psafont
reviewed
May 12, 2025
changlei-li
commented
May 13, 2025
Pif device name maybe change. Look up device_to_position table to get the new device name in pif refresh. Then update to db. This function is called by pif.scan and resynchronise_pif_params. Signed-off-by: Changlei Li <[email protected]>
When xapi start, networkd config will be reset, see Xapi_pif.start_of_day_best_effort_bring_up. In fact, the interface_order is only maintained by networkd, it shouldn't be cleared. Signed-off-by: Changlei Li <[email protected]>
Signed-off-by: Changlei Li <[email protected]>
- function is_sorted_by_script to val device_renamed_already - reduce nested structure in update config - handle port name update - bring back read inventory to get bridge name Signed-off-by: Changlei Li <[email protected]>
Signed-off-by: Changlei Li <[email protected]>
Signed-off-by: Changlei Li <[email protected]>
changlei-li
force-pushed
the
private/changleli/rework-network
branch
from
c455eb0 to
3b21c30
Compare
minglumlu
reviewed
May 15, 2025
minglumlu
reviewed
May 15, 2025
minglumlu
reviewed
May 15, 2025
minglumlu
reviewed
May 15, 2025
minglumlu
reviewed
May 15, 2025
- refine n_of_xenbrn_opt by add %! - change back to get_mac when update db feild MAC The mac_table is filtered by physical but pif device may be non physical Signed-off-by: Changlei Li <[email protected]>
This reverts commit 402671f. Signed-off-by: Changlei Li <[email protected]>
minglumlu
reviewed
May 15, 2025
- combine sorting on first boot and non first boot in one function - reduce the use of device_already_renamed and make it clear that interface_order is None when device_already_renamed Signed-off-by: Changlei Li <[email protected]>
For the situation if a db pif device's position is not in the position list but can get mac for networkd. There is a mismatch between networkd sort function and dev filesys method. Signed-off-by: Changlei Li <[email protected]>
changlei-li
force-pushed
the
private/changleli/rework-network
branch
from
b694365 to
d95662a
Compare
minglumlu
approved these changes
May 16, 2025
psafont
reviewed
May 16, 2025
Comment on lines
+132
to
+136
| let choose_network_name_for_pif device pos_opt = | ||
| let pos_str = | ||
| match pos_opt with None -> "" | Some pos -> Printf.sprintf "[%d]" pos | ||
| in | ||
| Printf.sprintf "Pool-wide network associated with %s%s" device pos_str |
There was a problem hiding this comment.
This will be user visible, might be worth explaining what that number is:
Suggested change
| let choose_network_name_for_pif device pos_opt = | |
| let pos_str = | |
| match pos_opt with None -> "" | Some pos -> Printf.sprintf "[%d]" pos | |
| in | |
| Printf.sprintf "Pool-wide network associated with %s%s" device pos_str | |
| let choose_network_name_for_pif device pos_opt = | |
| let pos_str = | |
| match pos_opt with None -> "" | Some pos -> Printf.sprintf "(slot %d)" pos | |
| in | |
| Printf.sprintf "Pool-wide network associated with %s %s" device pos_str |
Does this rename the existing PIFs? we probably should start picking words for the user-visible concepts, so "slot" might not be the right one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is the adaption of #6381 in networkd and xapi.
XS8: Keep the legacy behaviour, use host-installer, sort-script to sort and rename the network interfaces to
ethx.XS9: Use
Network_device_order.sortto sort the interfaces, store the result in networkdconfig.interface_order.Compatibility is offered by check the sort-script
interface-rename-datadir.Add new interface Interface.get_interface_positions to pass interfaces and positions from networkd to xapi.