tc actions support

[daniel-noland]

Support for tc-actions

Add the building blocks for tc-actions support.

This PR is in the same spirit as my (in progress) work on florianl/go-tc#145

I broke out this functionality from my other PR #111 based on conversation and feedback from @cathay4t.

Summary of feature

This is easiest to explain by way of iproute2.

tc allows actions to be created independently of filters. Once created, these actions may then

1. be associated with zero or more filters,
2. live updated (and updates will be seen by all filters using that action),
3. be deleted (only after all filters using that action have been deleted).

For example, consider the following :

for i in x y z; do
  ip link add dev "$i" type dummy
  tc qdisc add dev "$i" clsact
done

tc actions add action mirred egress redirect dev y
tc actions add action gact drop 

At this point, we could

1. list the mirred actions

   $ tc actions list action mirred      
   total acts 1
   
          action order 0: mirred (Egress Redirect to device y) stolen
          index 1 ref 1 bind 0
          not_in_hw
          used_hw_stats disabled

2. list the gact actions

   $ tc actions list action gact
   total acts 1
   
           action order 0: gact action drop
            random type none pass val 0
            index 1 ref 1 bind 0
           not_in_hw
           used_hw_stats disabled

3. create any number of filters using either or both of these actions by index

   tc filter add dev x ingress pref 1000 proto ip flower dst_ip 8.8.8.8 action mirred index 1
   tc filter add dev z ingress pref 1000 proto ip flower dst_ip 8.8.8.8 action mirred index 1 action gact index 1   

4. display those filters as normal (with per-action statistics)

   $ tc -s filter show dev z ingress
   filter protocol ip pref 1000 flower chain 0
   filter protocol ip pref 1000 flower chain 0 handle 0x1
   eth_type ipv4
   dst_ip 8.8.8.8
   not_in_hw
         action order 1: mirred (Egress Redirect to device y) stolen
         index 1 ref 3 bind 2 installed 599 sec used 599 sec
         Action statistics:
         Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
         backlog 0b 0p requeues 0
   
         action order 2: gact action drop
          random type none pass val 0
          index 1 ref 2 bind 1 installed 599 sec used 599 sec
         Action statistics:
         Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) 
         backlog 0b 0p requeues 0

5. centrally update those actions (e.g., change drop to pass)

   $ tc actions change action gact pass index 1
   $ tc -s filter show dev z ingress 
   filter protocol ip pref 1000 flower chain 0
   filter protocol ip pref 1000 flower chain 0 handle 0x1
   eth_type ipv4
   dst_ip 8.8.8.8
   not_in_hw
   action order 1: mirred (Egress Redirect to device y) stolen
   index 1 ref 3 bind 2 installed 838 sec used 838 sec
   Action statistics:
   Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
   backlog 0b 0p requeues 0
   
          action order 2: gact action pass
           random type none pass val 0
           index 1 ref 2 bind 1 installed 838 sec used 838 sec
          Action statistics:
          Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) 
          backlog 0b 0p requeues 0

6. attempts to delete those actions while in use will be rejected (albeit with a buggy error message from iproute2/tc)

   $ tc actions delete action gact index 1
   Error: Failed to delete TC action.
   We have an error talking to the kernel
   Command "action" is unknown, try "tc actions help"

7. Removing all filters that use an action will allow the action to be deleted

   $ tc filter del dev z ingress pref 1000
   $ tc actions delete action gact index 1
   $ tc filter del dev x ingress pref 1000
   $ tc actions delete action mirred index 1

[daniel-noland]

I'm moving this out of draft state now because I think it can now be meaningfully reviewed.

I will work on squashing the remaining tiny TODO involving documentation, making sure all the doc links work correctly, and the other notes I left for myself. I will be very surprised if addressing any of those significantly change the code.

As things stand I

1. Wrote more documentation for all of the tc-actions functionality.
2. Added support for tc-tunnel_key EDIT: already broken out into another (future) pr. I will submit that after this is merged as it is a logically distinct unit of work.

in addition to what is described in the issue.

If you like I will break this into three commits to account for that (or even three PRs if you feel it is necessary). EDIT: I put the documentation for the existing functionality in a distinct commit from the rest of the PR in case the actions functionality needs to be reverted.

[daniel-noland]

I also made a slight amendment to the testing pattern by

1. pulling in the hex library as a dev dependency.
2. adding additional tests as per my thoughts in #121

I am happy to adapt that back into the existing testing pattern with hard coded vec![...] statements and or remove the "parse back" style tests if you don't think they belong here.

[daniel-noland]

Thanks for your documentation. The unit tests is nice too.

Thank you for the review :smile:

I will write fixups based on your comments.

If you think that flavor of unit tests is helpful (as I do, see #121) then we may also consider something like proptest or bolero to really expand on that "parse-back" strategy. We could even use bolero+kani if we wanna get really hardcore, but things like that are best done one step at a time.

It may also be worth looking at the rather unfortunate state of the higher level API this PR results in (see rust-netlink/rtnetlink#64) before we agree to merge this. If changes to this PR are needed to smooth out the rtnetlink api then that is best done before anybody (including rtnetlink) starts depending on this change set. Especially if we are going to try to 1.0 this library this year :smile: (something I really look forward to)

This segment of test code is especially unfortunate

This segment of test code is almost as sad

I have thoughts about what can be done to fix this. I will write them up shortly.

Thanks again :tada:

[cathay4t]

If you find it hard to capture netlink package as Vec, please let me know, I am happy to amend(and also validate) your test codes.

[daniel-noland]

If you find it hard to capture netlink package as Vec, please let me know, I am happy to amend(and also validate) your test codes.

I can capture as a Vec if that is better. It does remove the dev-dependency on hex. Will change

[daniel-noland]

Feedback responses posted.

Summary of changes:

1. adjust use of NLA_F_NESTED flag in both tests and emitted messages to reflect the kernel's stated usage (i.e. that NLA_F_NESTED be set for the tc-actions options).

   This required adjusting the tests which seem to have captured iproute2's incorrectly formed messages (iproute2 does not always set the NLA_F_NESTED flag)

2. Remove the use of hex as a dev dependency and use a const slice of u8 instead as per @cathay4t request

3. remove links to iproute2's github as per @cathay4t request

4. trivial documentation cleanup

5. restore deleted test

Will mark as ready for next review. Thanks for review.

I left the review response fixups as the last commit in this series to make review easier. I'm happy to squash that commit into the prior one before merge if you wish to keep the commit history clean

[daniel-noland]

Any update on this?