-
Notifications
You must be signed in to change notification settings - Fork 40
Add support for zone filtering for Dump and Flush #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for zone filtering for Dump and Flush #48
Conversation
Support for the CTA_ZONE attribute for dump and flush requests was added to the kernel in 6.8: torvalds/linux@eff3c55 We add an optional `Zone` field to the `Filter` struct in order to expose this support. In the case of Dump, for clients who are only interested in flows from a specific zone, this can be much more efficient than dumping all flows, unmarshalling them, and filtering based on the unmarshalled zone value, especially considering that the library has no support for partial unmarshalling. For older kernels, the attribute will be ignored. While we do not include a version check in the implementation of the Flush / Dump methods, we do make this requirement clear in their documentation. Fixes ti-mo#23 Signed-off-by: Antonin Bas <[email protected]>
634c05f to
cf2a41b
Compare
Pull Request Test Coverage Report for Build 18477283390Details
💛 - Coveralls |
ti-mo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch! Left a few comments.
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
9e2ef0f to
fd33425
Compare
Of course, makes sense, sorry. I've done some reading and it looks like the kernel supports a bunch more dump filters these days, so I feel like the existing I suggest a builder pattern to deal with the optionality problem: type Filter struct {
f map[string][]netfilter.Attribute
}
func (f *Filter) NewFilter() *Filter
func (f *Filter) Family(family uint8) *Filter
func (f *Filter) Zone(zone uint16) *Filter
func (f *Filter) OrigFlags(flags uint32) *Filter
func (f *Filter) ReplyFlags(flags uint32) *Filter
func (f *Filter) MarkMask(mark, mask uint32) *Filter
func (f *Filter) Status(status Status) *FilterRepeated calls to the same method would overwrite the same map key. I'm debating whether we should add a What do you think? |
|
@ti-mo I can commit to working on this (or finding someone that will work for this) some time in the next few weeks. However, would you be willing to merge this change and make a release first, before we look into changing the |
|
@antoninbas Done for now, mind giving current master a spin in your project and report back? (another breaking change in #53) I'll cut a release if all is well. |
Done: antrea-io/antrea#7504 |
|
Thanks! I'll cut a release today. |
Support for the CTA_ZONE attribute for dump and flush requests was added to the kernel in 6.8:
torvalds/linux@eff3c55
We add an optional
Zonefield to theFilterstruct in order to expose this support. In the case of Dump, for clients who are only interested in flows from a specific zone, this can be much more efficient than dumping all flows, unmarshalling them, and filtering based on the unmarshalled zone value, especially considering that the library has no support for partial unmarshalling.For older kernels, the attribute will be ignored. While we do not include a version check in the implementation of the Flush / Dump methods, we do make this requirement clear in their documentation.
Fixes #23