Support reconstruction of `SchedClassifierLink` #445

anfredette · 2022-11-18T23:39:27Z

This is the proposed solution for Step 2 of issue #414

For cases where you have done program.take_link() to manage ownership of TcLink we need an API similar to PinnedLink::from_pin that can reconstruct a TcLink

As long as a user application continues to run after executing take_link(), the SchedClassifierLink returned can be used to detach the program. However, if we want to handle cases where the application exits or crashes, we need a way to save and reconstruct the link, and to do that, we also need to know the information required for the reconstruction -- namely, the interface, attach_type, priority, and handle. The user knows the first two because they are required to execute attach() in the first place; however, the user will not know the others if they let the system choose them.

This pr solves the problems by adding an impl for SchedClassifierLink with accessors for priority and handle, and a new() function.

Signed-off-by: Andre Fredette afredette@redhat.com

netlify · 2022-11-18T23:39:40Z

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`7855a0e`
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/63d7e216eb019f000a4b0e08
😎 Deploy Preview	https://deploy-preview-445--aya-rs-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

aya/src/programs/tc.rs

dave-tucker

Looks good, but docs need work.

aya/src/programs/tc.rs

vadorovsky

LGTM, thanks!

dave-tucker

LGTM

dave-tucker · 2022-12-12T17:18:26Z

Ping @alessandrod - this one is good to go but requires a LGTM from you since API changes are involved.

alessandrod

Great work as always! See comments

aya/src/programs/tc.rs

alessandrod · 2022-12-12T19:58:55Z

aya/src/programs/tc.rs

+        handle: u32,
+    ) -> Result<SchedClassifierLink, ProgramError> {
+        let if_index =
+            ifindex_from_ifname(if_name).map_err(|io_error| TcError::NetlinkError { io_error })?;


This shoulld just return io::Error I think? if_nametoindex uses ioctls not netlink

That sounds fine. I copied the error mapping from the original attach() function, but I can change this, and I guess I should change the other instance as well.

aya/src/programs/tc.rs

It is based on aya with the following prs: aya-rs/aya#462 aya-rs/aya#445 (w/445 modified to work with 462) Signed-off-by: Andre Fredette <afredette@redhat.com>

aya/src/programs/tc.rs

anfredette · 2022-12-22T12:58:15Z

I think I've addressed the comments. Please take a look when you get a chance.

vadorovsky

Thanks!

This is the proposed solution for Step 2 of issue aya-rs#414 “For cases where you have done program.take_link() to manage ownership of TcLink we need an API similar to PinnedLink::from_pin that can reconstruct a TcLink” As long as a user application continues to run after executing `take_link()`, the `SchedClassifierLink` returned can be used to detach the program. However, if we want to handle cases where the application exits or crashes, we need a way to save and reconstruct the link, and to do that, we also need to know the information required for the reconstruction -- namely, the `interface`, `attach_type`, `priority`, and `handle`. The user knows the first two because they are required to execute `attach()` in the first place; however, the user will not know the others if they let the system choose them. This pr solves the problems by adding an `impl` for `SchedClassifierLink` with an accessor for `tc_options` and a `new()` function. Signed-off-by: Andre Fredette <afredette@redhat.com>

Also modified the impl a bit to work as described in the example. Signed-off-by: Andre Fredette <afredette@redhat.com>

Co-authored-by: Dave Tucker <dave@dtucker.co.uk> Signed-off-by: Andre Fredette <afredette@redhat.com>

Signed-off-by: Andre Fredette <afredette@redhat.com>

Avoids name confilct with pr aya-rs#462 Signed-off-by: Andre Fredette <afredette@redhat.com>

Signed-off-by: Andre Fredette <afredette@redhat.com>

- Rename `new_tc_link` to `attached` - Simplified example for `attached` The following was not requested in reviews: - Moved example from `SchedClassifierLink` tor `SchedClassifierLink::attached' since we're only showing an example of that one method Signed-off-by: Andre Fredette <afredette@redhat.com>

It was redundant with the one provided in define_link_wrapper Signed-off-by: Andre Fredette <afredette@redhat.com>

Signed-off-by: Andre Fredette <afredette@redhat.com>

anfredette · 2023-01-25T21:01:38Z

@alessandrod, I'd appreciate it if you could take a look when you get a chance.

alessandrod

Thanks! Just a couple of docs nits

aya/src/programs/tc.rs

Signed-off-by: Andre Fredette <afredette@redhat.com>

anfredette force-pushed the tc-link-recon branch from 6bd00bb to 08d9b86 Compare November 18, 2022 23:43

vadorovsky reviewed Nov 19, 2022

View reviewed changes

aya/src/programs/tc.rs Outdated Show resolved Hide resolved

anfredette force-pushed the tc-link-recon branch 3 times, most recently from bed7e8b to 0d8fd95 Compare November 21, 2022 22:12

dave-tucker requested changes Nov 22, 2022

View reviewed changes

anfredette force-pushed the tc-link-recon branch 3 times, most recently from d1622d2 to 449e342 Compare November 22, 2022 21:05

anfredette mentioned this pull request Nov 25, 2022

Rebuild TC dispatcher state bpfman/bpfman#217

Merged

1 task

vadorovsky approved these changes Nov 28, 2022

View reviewed changes

anfredette requested a review from dave-tucker November 28, 2022 15:45

dave-tucker approved these changes Dec 12, 2022

View reviewed changes

alessandrod requested changes Dec 12, 2022

View reviewed changes

anfredette force-pushed the tc-link-recon branch from 8439f02 to 3a031d5 Compare December 19, 2022 12:08

anfredette requested a review from alessandrod December 19, 2022 12:35

vadorovsky requested changes Dec 21, 2022

View reviewed changes

aya/src/programs/tc.rs Outdated Show resolved Hide resolved

aya/src/programs/tc.rs Outdated Show resolved Hide resolved

alessandrod reviewed Dec 21, 2022

View reviewed changes

aya/src/programs/tc.rs Outdated Show resolved Hide resolved

anfredette requested review from alessandrod and vadorovsky and removed request for alessandrod and vadorovsky December 22, 2022 12:58

vadorovsky approved these changes Dec 24, 2022

View reviewed changes

anfredette and others added 8 commits January 25, 2023 14:12

Add example for SchedClassifierLink::new()

c3a8400

Also modified the impl a bit to work as described in the example. Signed-off-by: Andre Fredette <afredette@redhat.com>

Combine updates to SchedClassifierLink example made by Dave Tucker

6563e6c

Co-authored-by: Dave Tucker <dave@dtucker.co.uk> Signed-off-by: Andre Fredette <afredette@redhat.com>

Additional edits to SchedClassifierLink documentation.

67efc33

Signed-off-by: Andre Fredette <afredette@redhat.com>

rename SchedClassifierLink:new() to new_tc_link()

849796c

Avoids name confilct with pr aya-rs#462 Signed-off-by: Andre Fredette <afredette@redhat.com>

Address review comments

65f5b76

Signed-off-by: Andre Fredette <afredette@redhat.com>

Remove SchedClassifierLink description

6766532

It was redundant with the one provided in define_link_wrapper Signed-off-by: Andre Fredette <afredette@redhat.com>

anfredette force-pushed the tc-link-recon branch from 6b70311 to 6766532 Compare January 25, 2023 19:12

Updates after rebase due to changes in define_link_wrapper

d43879d

Signed-off-by: Andre Fredette <afredette@redhat.com>

anfredette requested a review from alessandrod January 25, 2023 21:00

alessandrod reviewed Jan 26, 2023

View reviewed changes

aya/src/programs/tc.rs Outdated Show resolved Hide resolved

aya/src/programs/tc.rs Outdated Show resolved Hide resolved

anfredette added 2 commits January 30, 2023 10:03

Address review comments from @alessandrod

7c24296

Signed-off-by: Andre Fredette <afredette@redhat.com>

retrigger checks

7855a0e

Signed-off-by: Andre Fredette <afredette@redhat.com>

alessandrod approved these changes Jan 30, 2023

View reviewed changes

alessandrod merged commit 22d7931 into aya-rs:main Jan 30, 2023

dave-tucker added feature A PR that implements a new feature or enhancement aya This is about aya (userspace) labels Feb 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support reconstruction of `SchedClassifierLink` #445

Support reconstruction of `SchedClassifierLink` #445

anfredette commented Nov 18, 2022 •

edited

Loading

netlify bot commented Nov 18, 2022 •

edited

Loading

dave-tucker left a comment

vadorovsky left a comment

dave-tucker left a comment

dave-tucker commented Dec 12, 2022

alessandrod left a comment

alessandrod Dec 12, 2022

anfredette Dec 16, 2022

anfredette commented Dec 22, 2022

vadorovsky left a comment

anfredette commented Jan 25, 2023

alessandrod left a comment

Support reconstruction of SchedClassifierLink #445

Support reconstruction of SchedClassifierLink #445

Conversation

anfredette commented Nov 18, 2022 • edited Loading

netlify bot commented Nov 18, 2022 • edited Loading

✅ Deploy Preview for aya-rs-docs ready!

dave-tucker left a comment

Choose a reason for hiding this comment

vadorovsky left a comment

Choose a reason for hiding this comment

dave-tucker left a comment

Choose a reason for hiding this comment

dave-tucker commented Dec 12, 2022

alessandrod left a comment

Choose a reason for hiding this comment

alessandrod Dec 12, 2022

Choose a reason for hiding this comment

anfredette Dec 16, 2022

Choose a reason for hiding this comment

anfredette commented Dec 22, 2022

vadorovsky left a comment

Choose a reason for hiding this comment

anfredette commented Jan 25, 2023

alessandrod left a comment

Choose a reason for hiding this comment

Support reconstruction of `SchedClassifierLink` #445

Support reconstruction of `SchedClassifierLink` #445

anfredette commented Nov 18, 2022 •

edited

Loading

netlify bot commented Nov 18, 2022 •

edited

Loading