Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for alternative names #862

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Conversation

SchSeba
Copy link
Contributor

@SchSeba SchSeba commented Apr 3, 2023

This commit add the support for alternative names.

exposing the attributes in the link object.
also adding the add and delete functions

@aboch
Copy link
Collaborator

aboch commented Sep 27, 2023

Thank you.
Can you pelase add some UT as well?

@hellt
Copy link

hellt commented Oct 4, 2023

I would love to see this merged. Need it to set user-friendly names for otherwise restricted link length

@hellt
Copy link

hellt commented Oct 4, 2023

@SchSeba I wonder, which func would one use to get the interface by its altname?

@SchSeba
Copy link
Contributor Author

SchSeba commented Oct 5, 2023

Hi @hellt,
it will be just as part of the link attrs you will have a list of them https://github.com/vishvananda/netlink/pull/862/files#diff-ff692b0e80c3a46490bdaf004f6628c003a4736c549aa8050a430246e70d6703R36

@hellt
Copy link

hellt commented Oct 5, 2023

Hi @SchSeba
So to get a link by an alias one would need to list all links and then do a loop to the desired altname to be present in []AltNames?

I wonder, since we have LinkByAlias, wouldn't it be nice to have LinkByAltName?

@SchSeba
Copy link
Contributor Author

SchSeba commented Oct 5, 2023

The problem is that LinkByAltName will have to do "list all links and then do a loop to the desired altname to be present in []AltNames" because there is no API right now in the kernel.

btw I didn't try that but I think you can just use the LinkByName with the Alt name and it should work can you give it a try?

@hellt
Copy link

hellt commented Oct 5, 2023

The problem is that LinkByAltName will have to do "list all links and then do a loop to the desired altname to be present in []AltNames" because there is no API right now in the kernel.

yes, this is also how LinkByAlias works as well for older kernels -

if err == unix.EINVAL {

I did try to use LinkByName and providing an alias, but I got a netlink error, smth about slice bounds out of range... So I had to revert back to using aliases, though AltNames is what I really want.

@SchSeba
Copy link
Contributor Author

SchSeba commented Feb 12, 2024

@aboch @vishvananda could you take a look at this PR ? :)

@SchSeba
Copy link
Contributor Author

SchSeba commented Feb 12, 2024

Hi @hellt be aware that now it works link netlink does you can do LinkByName and it will find the device base on the name or the altName

# ip link show enp8s0u2u1u2
26: ens1u2u1u2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
    link/ether 08:3a:88:61:c6:bf brd ff:ff:ff:ff:ff:ff
    altname enp8s0u2u1u2

link.go Outdated Show resolved Hide resolved
link_linux.go Outdated Show resolved Hide resolved
link_linux.go Outdated Show resolved Hide resolved
link_linux.go Show resolved Hide resolved
link_linux.go Outdated Show resolved Hide resolved
link_test.go Show resolved Hide resolved
@SchSeba
Copy link
Contributor Author

SchSeba commented Feb 13, 2024

@adrianchiris please take another look when you have time

@adrianchiris
Copy link
Collaborator

@SchSeba code changes look OK. thx for addressing my comments.

could you please update commit message to mention the changes in LinkByName()

This commit add the support for alternative names.

* exposing the attributes in the link object.
* adding the add and delete functions
* allow LinkByName() to also find devices by altname like `ip link`

Signed-off-by: Sebastian Sch <[email protected]>
@SchSeba
Copy link
Contributor Author

SchSeba commented Feb 20, 2024

Done thanks @adrianchiris !

Copy link
Collaborator

@adrianchiris adrianchiris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@aboch aboch merged commit a008cbd into vishvananda:main Feb 20, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants