Skip to content

Conversation

@antoninbas
Copy link
Contributor

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 #23

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]>
@antoninbas antoninbas force-pushed the support-zone-filtering-for-dump-and-flush branch from 634c05f to cf2a41b Compare September 30, 2025 22:20
@coveralls
Copy link

coveralls commented Sep 30, 2025

Pull Request Test Coverage Report for Build 18477283390

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 91.593%

Totals Coverage Status
Change from base Build 16113237611: 0.04%
Covered Lines: 1340
Relevant Lines: 1463

💛 - Coveralls

Copy link
Owner

@ti-mo ti-mo left a 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.

@antoninbas antoninbas force-pushed the support-zone-filtering-for-dump-and-flush branch from 9e2ef0f to fd33425 Compare October 13, 2025 20:20
@ti-mo
Copy link
Owner

ti-mo commented Oct 14, 2025

The default behavior is not reading from the default zone (0), the default behavior is to read from all zones.

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 Filter implementation became somewhat inadequate. Would you be up for reworking it?

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) *Filter

Repeated calls to the same method would overwrite the same map key. Filter.marshal() would simply flatten the map into a []netfilter.Attribute.

I'm debating whether we should add a Mask member to Status (since it's already a struct, not really sure why), or if Status should be Status(status StatusFlags, mask uint32) to be consistent with MarkMask.

What do you think?

@antoninbas
Copy link
Contributor Author

@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 Filter API? We would really like to start using this in our project, and it is ready.

@ti-mo
Copy link
Owner

ti-mo commented Oct 15, 2025

Hm, I think you're overestimating how much work this really is. 🙂 I've ported mark/mask in #49. Superseding this PR, zone support is in #50. I'll play around with the other fields.

@ti-mo ti-mo closed this Oct 15, 2025
@antoninbas
Copy link
Contributor Author

Hm, I think you're overestimating how much work this really is. 🙂 I've ported mark/mask in #49. Superseding this PR, zone support is in #50. I'll play around with the other fields.

Thanks, do you think we can have a new release after you complete the Filter changes?

@ti-mo
Copy link
Owner

ti-mo commented Oct 16, 2025

@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.

@antoninbas
Copy link
Contributor Author

@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
The relevant tests are passing

@ti-mo
Copy link
Owner

ti-mo commented Oct 20, 2025

Thanks! I'll cut a release today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow zoneID to be netfilter attribute

3 participants