Skip to content

Comments

link: add Detach()#1830

Merged
lmb merged 4 commits intomainfrom
link-detach
Aug 11, 2025
Merged

link: add Detach()#1830
lmb merged 4 commits intomainfrom
link-detach

Conversation

@florianl
Copy link
Contributor

Add bpf_link_detach() functionality.

Signed-off-by: Florian Lehner <[email protected]>
@florianl florianl requested review from a team and mmat11 as code owners July 26, 2025 11:19
@mmat11
Copy link
Contributor

mmat11 commented Jul 26, 2025

I see that iter links (and probably other) don't support Detach(); should we add a test for one of these cases? also, should we explicitly return ErrNotSupported?

florianl added 2 commits July 28, 2025 19:10
Signed-off-by: Florian Lehner <[email protected]>
@florianl
Copy link
Contributor Author

sorry for the delayed response 🙏

I see that iter links (and probably other) don't support Detach(); should we add a test for one of these cases?

With 2520bb8 I have added such a test.

[..] should we explicitly return ErrNotSupported?

With c92c648 I have added an error wrapper. Hope this is fine for you.

Signed-off-by: Florian Lehner <[email protected]>
@mmat11
Copy link
Contributor

mmat11 commented Jul 28, 2025

lgtm! there are some failing CI jobs, not sure they are related

@florianl
Copy link
Contributor Author

there are some failing CI jobs, not sure they are related

They don't seem to be related:

=== FAIL: asm  (0.00s)
panic: test timed out after 10m0s
	running tests:
		TestGetSetJumpOp (10m0s)

This is also reported in #1734

Copy link
Contributor

@mmat11 mmat11 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I thought I approved already 🙈

@lmb lmb merged commit e82f639 into main Aug 11, 2025
32 of 35 checks passed
@lmb lmb deleted the link-detach branch August 11, 2025 12:10
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.

3 participants