little-dude / netlink

netlink libraries for rust
Other
329 stars 89 forks source link

Add traffic control support #232

Closed wllenyj closed 2 years ago

wllenyj commented 2 years ago

Currently only u32 filter and qdisc ingress is supported.

wllenyj commented 2 years ago

@little-dude @cathay4t Do you have any comments on this pr? Thanks.

wllenyj commented 2 years ago

Will all tc Kind have the same name problem? If not, there is no need to do qdisc/filter/class distinction in TcMessage, which would simplify the implementation.

But I found that, for example, tc qdisc has htb, and tc class also has htb, but they can share the same attr without conflict.

So I will try to implement it by removing the TcMessage template class.

little-dude commented 2 years ago

Thanks a lot for the hard work @wllenyj. Generally speaking I like the approach you've taken. I need to spend a bit more time reviewing this though because I'm not familar with tc. I already left a couple questions for you.

wllenyj commented 2 years ago

Thanks a lot for the hard work @wllenyj. Generally speaking I like the approach you've taken. I need to spend a bit more time reviewing this though because I'm not familar with tc. I already left a couple questions for you.

@little-dude Thanks for you review, I have re-implemented a version and removed the template parameter of TcMessage. I'll get right back to finishing the patch.

wllenyj commented 2 years ago

The ci failure may be an environmental issue, it works when is running in my environment.

// TCA_OPTIONS                                 
let nla = iter.next().unwrap();                
let filter = if let tc::Nla::Options(f) = nla {
    // This assert is failed.
    //assert_eq!(f.len(), 5);                  
    f                                          
} else {                                       
    panic!("expect options nla");              
};                                             
---- traffic_control::add_filter::test::test_new_filter stdout ----
thread 'traffic_control::add_filter::test::test_new_filter' panicked at 'assertion failed: `(left == right)`
  left: `4`,
 right: `5`', rtnetlink/src/traffic_control/add_filter.rs:299:21
little-dude commented 2 years ago

Thanks @wllenyj

If you don't mind I'll wait for https://github.com/little-dude/netlink/pull/234 to land to make a release.

wllenyj commented 2 years ago

If you don't mind I'll wait for #234 to land to make a release.

@little-dude That's ok.