Skip to content

ipset: fix bitmap:port entry add failing with invalid protocol#1177

Open
chent1996 wants to merge 1 commit intovishvananda:mainfrom
chent1996:fix/ipset-bitmap-port-protocol
Open

ipset: fix bitmap:port entry add failing with invalid protocol#1177
chent1996 wants to merge 1 commit intovishvananda:mainfrom
chent1996:fix/ipset-bitmap-port-protocol

Conversation

@chent1996
Copy link
Contributor

@chent1996 chent1996 commented Mar 25, 2026

Summary

  • Decouple Protocol and Port attributes in buildEntryData() so they are sent independently
  • Only send IPSET_ATTR_PROTO when entry.Protocol is explicitly set (non-nil)
  • Remove the implicit TCP default that was unconditionally added whenever Port was set
  • Add bitmap:port test case for entry add/del
  • Fix port/protocol assertion in tests to handle nil Protocol

Context

bitmap:port ipsets use only port numbers without a protocol dimension. The old code unconditionally sent IPSET_ATTR_PROTO (defaulting to TCP) whenever Port was set, which newer kernels reject with "invalid protocol" for bitmap:port sets.

This matches the behavior of the ipset CLI tool, which does not send the protocol attribute for bitmap:port operations.

Note: Callers using hash:ip,port or hash:net,port,net must now explicitly set entry.Protocol, as the implicit TCP default is removed. The kernel requires protocol for these set types and will return an error if omitted.

Test plan

  • Existing hash:ip,port and hash:net,port,net tests pass (they already set Protocol explicitly)
  • New bitmap:port test case verifies entry add without Protocol
  • go build ./... compiles cleanly

Fixes #1054

Summary by CodeRabbit

  • Bug Fixes
    • Corrected ipset entry creation behavior to remove automatic protocol assignment when specifying only a port value. Protocol and port attributes are now independently configured based on what is explicitly provided, eliminating implicit defaulting behavior. This change enables more precise and predictable control when creating ipset entries with specific attribute requirements.

buildEntryData() unconditionally sends IPSET_ATTR_PROTO (defaulting
to TCP) whenever Port is set. For bitmap:port ipsets, this protocol
attribute is not expected and newer kernels reject it with "invalid
protocol".

Decouple Protocol and Port attributes: only send IPSET_ATTR_PROTO
when entry.Protocol is explicitly set. This matches the behavior of
the ipset CLI tool, which does not send protocol for bitmap:port.

Note: callers using hash:ip,port or hash:net,port,net must now
explicitly set entry.Protocol, as the implicit TCP default is removed.

Also add a bitmap:port test case to TestIpsetCreateListAddDelDestroy
and fix the port/protocol assertion to handle nil Protocol.

Fixes vishvananda#1054
@coderabbitai
Copy link

coderabbitai bot commented Mar 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c27727ca-a0fc-4c9d-9362-159e8d6c50c1

📥 Commits

Reviewing files that changed from the base of the PR and between 72a8cd7 and 9871a53.

📒 Files selected for processing (2)
  • ipset_linux.go
  • ipset_linux_test.go

📝 Walkthrough

Walkthrough

This change modifies ipset attribute encoding in buildEntryData to remove automatic protocol defaulting. Protocol and port attributes are now only emitted when explicitly provided, resolving compatibility issues with bitmap:port ipset type where protocol specification is unnecessary and causes errors.

Changes

Cohort / File(s) Summary
Protocol/Port Attribute Encoding
ipset_linux.go
Removed implicit defaulting where entry.Port without entry.Protocol would auto-populate protocol to TCP (IPPROTO_TCP). Now only emits IPSET_ATTR_PROTO and IPSET_ATTR_PORT attributes when explicitly provided, eliminating invalid protocol errors.
Test Coverage
ipset_linux_test.go
Added test case for bitmap:port ipset type with port range configuration, validating entries containing only Port field. Updated protocol assertions to conditionally check only when entry.Protocol is non-nil.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 No more protocol tricks up our sleeves,
Bitmap ports now work as intended, I believe!
TCP won't sneak in when it needn't appear,
Clean attributes flow—no errors to fear! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main fix: addressing a protocol-related failure when adding entries to bitmap:port ipsets, which aligns with the primary change.
Linked Issues check ✅ Passed The pull request successfully addresses issue #1054 by decoupling Protocol and Port attributes in buildEntryData(), only emitting IPSET_ATTR_PROTO when entry.Protocol is non-nil, and removing the implicit TCP default that caused the invalid protocol error.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing bitmap:port entry add failures by modifying buildEntryData() logic and adding corresponding test coverage; no unrelated modifications were introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

ipset v7 bitmap:port add failed: invalid protocol

1 participant