Repository navigation
IPv4 routing table: add() ignores netmask and ifconfig leaves stale connected routes #774
Copy link
Copy link
Closed
Labels
agent/doneThe agent finished successfully; PR opened or comment posted.The agent finished successfully; PR opened or comment posted.area/netnet/ — network stack, sockets, ARP, IPv4, DNS, Ethernet.net/ — network stack, sockets, ARP, IPv4, DNS, Ethernet.kind/bugConfirmed bug report with a repro or stack trace.Confirmed bug report with a repro or stack trace.
Description
Summary
Two related routing-table hygiene defects that became easy to hit once ifconfig started installing a connected route:
IPv4RoutingTable.add()de-duplicates on destination + gateway + device and ignores the netmask, so adding a route with a different prefix silently replaces the existing one.Ifconfig.setDefault()never removes the previous connected route, so re-addressing an interface leaves the old subnet route up.
Steps to Reproduce
1. add() ignores the netmask
# ifconfig eth-pci(0,3,0) 10.0.2.15 255.255.255.0
# route
10.0.2.0 - 255.255.255.0 - null - U - 0 - eth-pci(0,3,0)
# route -a 10.0.2.0/16 eth-pci(0,3,0)
# route
10.0.2.0 - 255.255.0.0 - null - U - 0 - eth-pci(0,3,0)
The /16 replaced the /24 connected route. The table now has no route for the rest of the /24 subnet.
2. ifconfig leaves the old connected route up
# ifconfig eth-pci(0,3,0) 10.0.2.15 255.255.255.0
# ifconfig eth-pci(0,3,0) 10.0.3.15 255.255.255.0
# route
10.0.2.0 - 255.255.255.0 - null - U - 0 - eth-pci(0,3,0)
10.0.3.0 - 255.255.255.0 - null - U - 0 - eth-pci(0,3,0)
Both connected routes are present. IPv4RoutingTable.search() now uses longest-prefix match with a gateway tie-break at equal prefix length, so with two /24s present the winner between them is decided by insertion order rather than by anything meaningful.
Expected Behavior
- Adding a route with an explicit prefix creates a second entry rather than replacing one with a different prefix.
- Re-addressing an interface removes the connected route for its previous address/mask.
Actual Behavior
Both routes are silently clobbered as described above.
Root Cause
net/src/net/org/jnode/net/ipv4/IPv4RoutingTable.java-add()compares destination, gateway and device but notgetSubnetmask().net/src/net/org/jnode/net/ipv4/config/impl/Ifconfig.java-addConnectedRoute()only ever adds.
Notes
- Defect 1 is pre-existing, but before IPv4: no connected routes after ifconfig, classful-only shell routes, gateway preferred over on-link direct routes #768 on-link routes were rare enough that it was hard to hit.
- Defect 2 is new with IPv4: no connected routes after ifconfig, classful-only shell routes, gateway preferred over on-link direct routes #768.
- Both should have unit tests in
IPv4RoutingTableTest/ anIfconfigtest once fixed. Do not cement the current behaviour with tests.
Found while reviewing #770.
Environment
- Commit:
0b1055d(PR IPv4 routing fixes: connected routes, LPM, route prefix support #770 branchopencode/issue768-20261016180433) - Host JDK:
openjdk version "1.8.0_504" - Build target:
cd-x86-lite - Bitness: 32-bit
- Run:
qemu-system-x86_64 -cdrom all/build/cdroms/jnode-x86-lite.iso -m 1G -nographic - ISO:
all/build/cdroms/jnode-x86-lite.iso - Boot reached
System has finished: yes
Triage Addendum (auto, 2026-10-16)
- Area / Kind: area/net + kind/bug
- Blast radius: L2 package (net/src/net/org/jnode/net/ipv4 + .../ipv4/config/impl, 2 classes; IPv4RoutingTable is reachable via IPv4Service.getRoutingTable() but no public API signature change; not on the boot path)
- Expected scope: IPv4RoutingTable.add() dedup key must include getSubnetmask(); Ifconfig.setDefault() must remove the connected route of the previous address/mask before installing the new one. No ASM, no boot-path, no exported-API changes. Fix defect 1 on main; defect 2 on top of the IPv4: no connected routes after ifconfig, classful-only shell routes, gateway preferred over on-link direct routes #768/IPv4 routing fixes: connected routes, LPM, route prefix support #770 branch (addConnectedRoute does not exist on main yet).
- Test path:
cd net && ant test(new net/src/test/org/jnode/net/ipv4/IPv4RoutingTableTest.java, register in net/src/test/org/jnode/net/AllTests.java) + QEMU repro of the two ifconfig/route sequences on cd-x86-lite - Merge signal: human-review (bug kind; routing-table behaviour change, needs maintainer eyes)
- Gaps: non-blocking - defect 2 cannot be built/tested on main until IPv4: no connected routes after ifconfig, classful-only shell routes, gateway preferred over on-link direct routes #768/IPv4 routing fixes: connected routes, LPM, route prefix support #770 merges; both defects currently have no test coverage
- Full report:
## Triagecomment below.
✅ Ticket Runner Status
| Field | Value |
|---|---|
| Phase | DONE |
| Turn | 1/3 |
| Retries | 0/3 |
| PR | #776 |
| Review in progress | no |
| Started | 2026-10-16T18:42:20.717Z |
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
agent/doneThe agent finished successfully; PR opened or comment posted.The agent finished successfully; PR opened or comment posted.area/netnet/ — network stack, sockets, ARP, IPv4, DNS, Ethernet.net/ — network stack, sockets, ARP, IPv4, DNS, Ethernet.kind/bugConfirmed bug report with a repro or stack trace.Confirmed bug report with a repro or stack trace.
Activity
LSantha commentedon Oct 16, 2026
/oc triage issue #774
Triage run ONLY. Load the jnode-triage-issue skill and obey it. Output is exactly one ## Triage comment plus kind/area label edits. FORBIDDEN in this run: git checkout -b, git push, gh pr create, any branch, any PR. Audit kind/area labels (correct mislabels), assess repro-sufficiency, estimate blast radius, post one ## Triage comment (clear shape when actionable, vague shape with the needs-info literals only when human input is truly required).
Validate route prefixes and cover mask handling with tests (#768)
LSantha commentedon Oct 16, 2026
Triage
ifconfig eth-pci(0,3,0) 10.0.2.15 255.255.255.0thenroute -a 10.0.2.0/16 eth-pci(0,3,0)-> /16 replaces the /24 entry; re-addressing with10.0.3.15leaves both /24 connected routes present. Actual vs expected stated, env complete (commit0b1055d,cd-x86-lite, qemu command, boot reachedSystem has finished).org.jnode.net.ipv4andorg.jnode.net.ipv4.config.impl;IPv4RoutingTableis reachable throughIPv4Service.getRoutingTable()but no signature change; not boot-critical, no data-loss/hang axis)IPv4RoutingTable.add()- includegetSubnetmask()in the dedup key so a different prefix is a second entry, not a replace;Ifconfig.setDefault()- remove the previous address/mask's connected route before installing the new one. No-go: ASM, boot path, exported API. Defect 1 is self-contained onmain; defect 2 depends onaddConnectedRoute(), which does not exist onmainyet and only lands with IPv4: no connected routes after ifconfig, classful-only shell routes, gateway preferred over on-link direct routes #768/IPv4 routing fixes: connected routes, LPM, route prefix support #770 - land it on top of that branch.cd net && ant testwith a newnet/src/test/org/jnode/net/ipv4/IPv4RoutingTableTest.java(registered innet/src/test/org/jnode/net/AllTests.java) covering same-dest-different-mask add, identical-route add, and longest-prefixsearch()after a /16+/24 mix; plus QEMU replay of both ifconfig/route sequences oncd-x86-lite. Existing search() tie-break behaviour must stay untouched - cementing today's clobbering in a test is a regression, not coverage.ant test)mainuntil IPv4: no connected routes after ifconfig, classful-only shell routes, gateway preferred over on-link direct routes #768/IPv4 routing fixes: connected routes, LPM, route prefix support #770 merges, so the two halves land in different order; neither defect has existing coverage.kind/bug,area/net(both already correct from filing, verified - no additions, no removals)github run
LSantha commentedon Oct 16, 2026
/oc Please proceed with this task.
LSantha commentedon Oct 16, 2026
🤖 Partially fixed. Commit
16fb65274onopencode/issue774-20261016184429.Defect 1 — fixed.
IPv4RoutingTable.add()now includesgetSubnetmask()in the dedup key (net/src/net/org/jnode/net/ipv4/IPv4RoutingTable.java:69), so adding10.0.2.0/16next to an existing10.0.2.0/24on the same device creates a second entry instead of clobbering the first. Newnet/src/test/org/jnode/net/ipv4/IPv4RoutingTableTest.java(registered inAllTests) covers same-dest-different-mask, same-dest-same-mask-different-device, same-dest-same-mask-different-gateway, replacement of an identical route, and idempotent re-add.sh build.sh -f net/build-tests.xml all-junit→Tests run: 28, Failures: 0, Errors: 0. Verified non-vacuous: reverting only theadd()change makestestAddKeepsRouteWithSameDestinationAndDifferentMaskfail.Defect 2 — not in this change.
Ifconfig.addConnectedRoute()does not exist onmaster; it arrives with #768/#770, so there is no stale-connected-route behaviour to fix here and nothing to boot-test (theroute -a 10.0.2.0/16repro syntax is also #768'sIPv4NetworkArgument). This matches the triage split: defect 1 lands standalone, defect 2 follows #770.Notes for whoever picks up defect 2 after #770 merges:
addrInfo.getSubnetMask(previousAddress)beforesetDefaultAddress()gives the mask needed to build the old connected route.IPv4RoutingTable.remove()isVector.remove(Object)andIPv4Routehas noequals(), soremove(new IPv4Route(...))is a no-op.removeConnectedRoute()has to locate the existing instance (iterateentries()and match on destination/mask/device) orIPv4Routeneedsequals()/hashCode().Related: #768, #770
LSantha commentedon Oct 16, 2026
Created PR #776
github run
#774 - net: net test file trailing newline
LSantha commentedon Oct 20, 2026
/run
Fixed add() netmask dedup + tests; defect 2 blocked on #770. (#776)