luci-mod-network: IP sets "Save & Apply" works for adding IPs but not removing IPs

[Skwidder]

Is there an existing issue for this?

• I have searched among all existing issues (including closed issues)

screenshots or captures

Screencast_20251118_205218.mp4

Actual behaviour

When modifying IP sets through Network → Firewall → IP Sets, clicking "Save & Apply" correctly applies added IPs to nftables, but does NOT remove deleted IPs from the running firewall.

Testing results:

• Adding IPs: Works correctly ✓ (new IP appears in nft list set)
• Removing IPs: Saved to /etc/config/firewall but not removed from running nftables set ✗

Note: This occurs when removing ANY IP from the set, not just the last one. Even when multiple IPs remain in the set after removal, the deleted IP persists in the running nftables configuration until service firewall restart is executed.

Workaround: Run service firewall restart after removing IPs.

Expected behaviour

Both adding and removing IPs should be immediately reflected in the running nftables IP set after clicking "Save & Apply". Users should not need to manually restart the firewall service.

Steps to reproduce

1. Navigate to Network → Firewall → IP Sets → (any IP set)
2. Add a new IP address → Save & Apply
3. Run nft list set inet fw4 <ipset_name> - verify new IP is present ✓
4. Remove an IP address from the same set → Save & Apply
5. Run nft list set inet fw4 <ipset_name> - removed IP still present ✗
6. Run service firewall restart
7. Run nft list set inet fw4 <ipset_name> - now removed IP is gone

The issue is that step 4-5 should work without requiring step 6.

Additional Information

- Model: Netgear Nighthawk X4S R7800
- Target: ipq806x/generic
- Architecture: arm_cortex-a15_neon-vfpv4
- Firmware Version	OpenWrt 24.10.2 r28739-d9340319c6 / LuCI openwrt-24.10 branch 25.168.50434~d6b13f6

What browsers do you see the problem on?

No response

Relevant log output

[efahl]

6. Run service firewall restart

What happens if you run fw4 reload-sets instead of the firewall restart for step 6?

[Skwidder]

6. Run service firewall restart

What happens if you run fw4 reload-sets instead of the firewall restart for step 6?

it appers that is dependent on if you remove the last ip or not.

Remove any but the last ip in the set and it has the same affect as restarting the firewall:

But if its the last IP in the set that dose not fix the issue unlike restarting the firewall:

[efahl]

Hah, looks like you've found two bugs, not just one...

[efahl]

Second one appears to be this:

https://github.com/openwrt/firewall4/blob/b6e5157527d361f99ad52eaa6da273cb0f2dfd59/root/usr/share/firewall4/main.uc#L21

Since you are loading from config (i.e., !set.loadfile) and the list is now empty (i.e., !length(set.entries)) it just bails out without doing a flush. That condition looks completely redundant, I see no reason not to just remove it.

Still looking at the LuCI one...

[Ramon00]

Without actually looking into the code this maybe means it needs an o.write = o.remove =.... somewhere?

[systemcrash]

Would that actually trigger a flush though?

[Ramon00]

So if you remove completely the o.write does not get triggered. It triggers o.remove. If you redefine the o.write to trigger something and did not do the same with o.remove then the default method will just be used. But I'm just guessing a bit, I did not actually check if there is a custom o.write defined.

[systemcrash]

The problem here is but does NOT remove deleted IPs from the running firewall.

[systemcrash]

@efahl could you propose a fix to fw4? Then at least @jow- could commit.

[Ramon00]

The problem here is but does NOT remove deleted IPs from the running firewall.

I know, if the reload is triggered with the o.write then it makes sense as that is not called anymore, but like I said I'm guessing a bit. (I will shut up now)

[systemcrash]

I'm not sure what conditions fw4 might reload by itself, but a save could chain trigger a reload of fw4...

[dave14305]

Just adding my notes:

• LuCI updates trigger a fw4 reload
• reload removes the state file and re-adds the ruleset
• The ruleset includes a flush table inet fw4, but flushing a table does not flush the sets in the table.
• Flushing sets requires knowing the name of each set (no broad nft flush sets command).
• Replacing the flush table inet fw4 with destroy table inet fw4 nukes the sets, but could have other issues, such as the next line in ruleset.uc deleting the flowtable.

[jow-]

At some point it was a design decision to not clear sets on reload but to only ever append to them. This was done to not accidentally destroy externally populated sets which are filled by e.g. dnsmasq while modifying unrelated firewall settings.

[efahl]

My fix would be to do fw4 reload-sets when the rule change involved ipsets, rather than do fw4 reload...

[efahl]

I see three different set use-case/classifications:

1. ipset rule with list of values
2. ipset rule with file
3. no ipset rule in config: user created/managed set

For the latter, I've been using a script (say, /etc/firewall.user or whatever) to create the set, completely outside of fw4's reach.

@jow- Is that line I flagged as a "bug" (https://github.com/openwrt/firewall4/blob/b6e5157527d361f99ad52eaa6da273cb0f2dfd59/root/usr/share/firewall4/main.uc#L21) intended to use an empty list on case 1 as the basis for user-managed sets? I assumed it was just trying to optimize away the "do nothing" case, but your comment makes me think this was the intent.

So, maybe we need a user_managed option on the ipset config that makes that case explicit?

(In any case, I'll stick with my above comment on the LuCI side of things: the action for ipsets should be a reload-set and not a reload...)