search

florianl / go-tc

traffic control in pure go - it allows to read and alter queues, filters and classes
MIT License
414 stars 44 forks source link

RFC: Support for `tc actions` like functionality #145

Closed daniel-noland closed 2 weeks ago

daniel-noland commented 2 months ago

I would like to add support for functionality equivalent to the tc actions commands to this library, but I wanted to make sure that this is desirable from your point of view before I spend the time.

Summary of functionality I would like to add

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

  1. be associated with one 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

If you are interested I will work on it and submit a patch.

daniel-noland commented 2 months ago

Sorry, I didn't mean to label this as a bug.

florianl commented 2 months ago

Adding this functionallity and being able to independently define actions in tc sounds like a good addition. From a first glance, it looks to me, as tc action is similar to tc qdisc or tc filter and so implementing it should be similar to Qdisc or Filter.

Thanks for suggesting this feature and volunteering to add it. I'm happy to accept a PR.

florianl commented 2 weeks ago

Not sure about your progress and status of work at @daniel-noland. As there was no updated so far, I went ahead and started working on it with https://github.com/florianl/go-tc/pull/150.

florianl commented 2 weeks ago

Closing with the merge of https://github.com/florianl/go-tc/pull/150.

daniel-noland commented 2 weeks ago

Oh, you are ahead of me and more :smile:

Glad you got this done.

I had to learn more than I expected to about tc and netlink and the arch of this library so I was quite slow.

I did manage to implement it over here rust-netlink/netlink-packet-route#122 since I am notably more fluent in rust than go. I will compare our implementations and see if I can learn some stuff :smiley:

florianl commented 1 week ago

No worry :) I'm kind of familiar with this code base and netlink. As I'm less familiar with Rust, I will look at https://github.com/rust-netlink/netlink-packet-route/pull/122 to learn more.