Skip to content
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

set: Add set support for size specifier, add missing dynamic flag unmarshal #294

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

a-ilin
Copy link
Contributor

@a-ilin a-ilin commented Jan 11, 2025

Add support for set size specification: handle attribute NFTNL_SET_DESC_SIZE, as done in libnftnl: https://git.netfilter.org/libnftnl/tree/src/set.c#n424

Example:

nft add set ip filter myset { type ipv4_addr\; size 65535\; flags dynamic\; }

Also add missing unmarshal of Dynamic flag (NFT_SET_EVAL).

@stapelberg
Copy link
Collaborator

@turekt You contributed support for set comments in PR #290 — could you take a look at this PR and confirm everything looks good please?

Copy link
Contributor

@turekt turekt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would personally add an additional system test which checks if Size works ok in correlation with NFTA_SET_DESC_CONCAT. We work with this field when set uses a concatenation and this seems to be a part of the set element description https://git.netfilter.org/libnftnl/tree/include/linux/netfilter/nf_tables.h?id=be0bae0ad31b0adb506f96de083f52a2bd0d4fbf#n326.

I did a quick test on sample code and it seems to work ok. Here's sample code I've used

conn, err := nftables.New()
if err != nil {
	panic(err)
}
conn.FlushRuleset()
table := &nftables.Table{
	Family: nftables.TableFamilyIPv4,
	Name:   "filter",
}
conn.AddTable(table)

set := &nftables.Set{
	Name:    "test-set",
	Table:   table,
	KeyType: nftables.MustConcatSetType(nftables.TypeIP6Addr, nftables.TypeInetService, nftables.TypeIP6Addr),
	Dynamic: true,
	Concatenation: true,
	Size: 200,
}
if err := conn.AddSet(set, nil); err != nil {
	panic(err)
}
if err := conn.Flush(); err != nil {
	panic(err)
}

There is also an example test here

func TestSetElementsInterval(t *testing.T) {

Otherwise implementation looks ok.

@a-ilin
Copy link
Contributor Author

a-ilin commented Jan 18, 2025

Hi @turekt ,

Thanks for the review!

Please find the mentioned test added in the new commit: a6f09ce

nftables_test.go Outdated Show resolved Hide resolved
set_test.go Outdated
Size: 10,
Table: tbl,
KeyType: TypeIPAddr,
Timeout: time.Duration(30) * time.Second,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“Timeout: 30 * time.Second” please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle attribute NFTNL_SET_DESC_SIZE, as done in libnftnl:
https://git.netfilter.org/libnftnl/tree/src/set.c#n424

Example:
nft add set ip filter myset { type ipv4_addr\; size 65535\; flags dynamic\; }
@stapelberg stapelberg merged commit 69f487d into google:main Jan 24, 2025
2 checks passed
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.

3 participants