fix(netwatch): a long list of small issues

[dignifiedquire]

Details can be found in the individual commits

EDIT: final changes revert some of the commits as they were controversial

[github-actions]

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/net-tools/pr/109/docs/net_tools/

Last updated: 2026-03-03T14:00:30Z

[divagant-martian]

• 2e1f120
  From what I can tell, it would have been advisable from the beginning to go
  with 4096 for the read buffer. This, to avoid fragmentation since the kernel
  usually works on the size of a memory page. (I can verify the size by doing
  getconf PAGESIZE). 8192 seems a bit on the "too safe" side, but this is
  still small, so it's fine.
• 7e110ee
  routing can be in different tables, so if the objective is to generally
  ignore multicast and link-local address updates then the previous code might
  miss it. The original code from tailscale seemed to have tried to target the
  kernel auto-added interface routes (we are already in the match of this
  message tells us it a NewRoute). If that's the case, the previous code is
  probably not the best way either. What should be checked here from what my
  internet-search foo tells me is to check the RTPROT and filter our the
  kernel ones. It seems to be in some extremely specific container scenarios,
  the routes might always be link-local, but might also be part of the tables
  previously checked (?) so this change (or rather, this code in general) might
  miss scenarios, but I don't think we should catter to these without a proper
  use case. There's no authoritative bit to tell us exactly how to deal with
  that, and without an use case we don't have evidence to base a better code.
  TLDR: commit looks fine to me.
• 44cdaf3
  OK this is an improvement wrt the 8192 update. The start might still be
  "narrow" but this takes into account some systems with larget page sizes (it
  seems to be macOS uses 16KB for example).
• 800e3c6
  fmt
• 634a16f
  If the call to send NetworkChanged does effectively block, there is no
  capacity. Since all notifications are the same (NetworkChanged does not
  carry data) the reader is already lagged and a "missed" notification won't
  change much (probably anything). Blocking while sending just blocks the
  monitor so this seems like a very good tradeoff.
• 8f23863
  The wasm warn seems useful, not sure how often it will trigger.
• 39e65ec
  unchecked_into -> dyn_into seems safer
  There is one unchecked_into left, is it intentional?
• acaeca5
  Good change to add a timeout. The fn creates a socket, does a kernel call and
  waits for a response, so everythong local.. 5s seems a bit too long for this
  but better than nothing. Why just in non-android linux?
• fc58eae
  Adding more paths to find ip seems fine, tho I'd like to know what's the
  motivation here? Have we seen this before? The alternative paths seem to be
  rarely used in general (from what I could find)
• cbe9cc6
  This makes the code more resilient but might make it harder to debug if
  something goes wrong (?) not sure it's great to work with data behind a
  poisoned state. I'd like to get some extra thoughts on this, not a fan, but
  also not completely against I guess. Also, poll_read_socket still panics.
  Is this intentional?
• a62aa07
  Fixes a TOCTOU, lgtm
• 339d77c
  Seems to be fmt only, lgtm

[matheus23]

39e65ec
unchecked_into -> dyn_into seems safer

I dislike that it would swallow errors.
Using unchecked_into isn't dangerous - it just causes runtime exceptions in the host.
I prefer the previous version, but if people would like the dyn_into, then I'd prefer logging a warning at least.

[dignifiedquire]

I dislike that it would swallow errors.
Using unchecked_into isn't dangerous - it just causes runtime exceptions in the host.
I prefer the previous version, but if people would like the dyn_into, then I'd prefer logging a warning at least.

I don't have a strong preference in either way, happy to revert it

[dignifiedquire]

• This makes the code more resilient but might make it harder to debug if
  something goes wrong (?) not sure it's great to work with data behind a
  poisoned state. I'd like to get some extra thoughts on this, not a fan, but
  also not completely against I guess. Also, poll_read_socket still panics.
  Is this intentional?

I am not super convinced either, will revert

[dignifiedquire]

• Adding more paths to find ip seems fine, tho I'd like to know what's the
  motivation here? Have we seen this before? The alternative paths seem to be
  rarely used in general (from what I could find)

no concrete usage, I didn't know there are alternative paths before doing this, so I think its cool to also check those, but not critical

[dignifiedquire]

• Good change to add a timeout. The fn creates a socket, does a kernel call and
  waits for a response, so everythong local.. 5s seems a bit too long for this
  but better than nothing. Why just in non-android linux?

we should do it on all platforms likely, no idea if 5s is a good or bad timeout, I think it should be quite large, to allow for slow cases to be handled

[matheus23]

There has been another wasm-bindgen release 😅

Need a cargo update -p wasm-bindgen/cargo update -p wasm-bindgen-test

[divagant-martian]

lgtm!