openwrt / openwrt/netifd

Incorrect policy routing ip rule cleanup on reload with ip4table/ip6table change

Open
#61 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
37
Forks
62
PR merge metrics
No merged PRs in 30d

Description

interface_change_config() changes the ip4table and ip6table fields of if_old here: https://github.com/openwrt/netifd/blob/777f5942fa7d6245f6ad29daa1daecc400344d37/interface.c#L1369-L1370. After the update, interface_ip_set_enabled(..., false) and interface_set_prefix_address() will clean up a number of implicitly created ip rules (set_ip_source_policy() and set_ip_lo_policy()) before setting them up again, but the cleanup will attempt to delete rules with the new ip[46]table instead of its old value, so the deletion fails. Only a full network restart will clean up rules properly.

I don't know the code well enough to propose a proper fix; the following ideas did not work out so far:

  • Set interface_ip_set_enabled(..., false) before updating the fields (probably possible in the reload_ip == true case, unclear if the reload == true case can be made to work)
  • Delete rule with 'unspecified' table lookup (there may be multiple rules that only differ by the table number)
  • Store the table numbers used for the last rule creation in the interface_ip_settings, so the same table can be used for cleanup (possible for interface_ip_set_enabled(), looks more complicated for ``interface_set_prefix_address()`

It is unclear to me if a combination of the above ideas could be sufficient for a fix, or if more precise tracking of created rules would be required for cleanup.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in interface.c at interface_change_config(), especially the ip4table/ip6table updates around lines 1369-1370, and trace interface_ip_set_enabled(), interface_set_prefix_address(), set_ip_source_policy(), and set_ip_lo_policy(). Verify how reload cleanup identifies policy rules, then ensure rules created with the previous table values are removed before or during reconfiguration so a full network restart is not required.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.