Page MenuHomeFreeBSD

tcp_hostcache: refactor the hostcache update function
Needs ReviewPublic

Authored by rscheff on Mon, Jul 20, 12:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 27, 10:16 PM
Unknown Object (File)
Mon, Jul 27, 7:52 AM
Unknown Object (File)
Mon, Jul 27, 1:02 AM
Unknown Object (File)
Mon, Jul 27, 12:31 AM
Unknown Object (File)
Mon, Jul 27, 12:15 AM
Unknown Object (File)
Sun, Jul 26, 5:06 PM
Unknown Object (File)
Sun, Jul 26, 4:01 PM
Unknown Object (File)
Sun, Jul 26, 12:41 PM
Subscribers

Details

Reviewers
tuexen
glebius
Group Reviewers
transport
Summary

Refactor the update function to support a callback once the correct entry (or new entry in the correct place) has been determined, to subsequently allow testing of the hostcache, and prime new tcp sessions with uncommon parameters.

No functional change intended.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 74952
Build 71835: arc lint + arc unit

Event Timeline

  • just do the refactory here
rscheff retitled this revision from tcp_hostcache: add sysctl function for updating the hostcache to tcp_hostcache: refactor the hostcache update function.Mon, Jul 20, 4:01 PM
rscheff edited the summary of this revision. (Show Details)

This revision is good one independently of how the API to delete entries is going to look like. To be fair I'm not very happy with new function names, but can't quickly come with a good alternatives.

  • provide a way to check for the mere existance, and contents of an entry for a given IP address.
  • provide a way to check for the mere existance, and contents of an entry for a given IP address.

This is why callback now can be NULL? What's the practical use?

This is why callback now can be NULL? What's the practical use?

Userspace sysctl actually makes 5 calls:

  1. NULL, 0 -> gets returned the buffer size, allocates that buffer
  2. NULL, size -> gets returned the old value
  3. newval, size -> sets the new value
  4. NULL, 0 -> gets returned the (new) buffer size
  5. NULL, newsize -> retrieves the values after setting

By allowing a NULL for callback, (and a NULL for the passed-on argument), I can check for the mere existance of an entry (return 0 / ENOENT), without touching the expiry timer.
With callback NULL but the pass-one argument a pointer to hc_entries, I can additionally get the actual values of that entry if it exists.

So, in the custom sysctl handler can statically return sizeof(struct hc_entry) in (1) and (4).
In (2) and (5), the current values can be retrieved (NULL, &(struct hc_entry))
This allow then the userspace tool to modify the values from (2) as necessary (leave some unmodified, change others...) before writing the value back. And also validate the set values finally match what got set.

The sysctl utility (userspace) already has a few special handlers for some S,xxx struct / CTLTYPE_OPAQUE/CTLTYPE_STRUCT binary blob exchanges - but i think those are so far mostly read-only.

Likely I can also change from a (void *) arg to a (struct hc_entry *)arg, with proper typechecking - and no typecasting.

See the companion D58358 over the next couple days when i get to completing this properly. Ultimately this facility will allow proper unit testing / priming of the tcp hostcache and thus start the tcp stack in various states not reachable by purgeall of the tcp hostcache.

sys/netinet/tcp_hostcache.c
458

May I suggest name hc_update_cb() for this internal function.

Your future callback can be called hc_purge_cb() in case if it purges an entry.

534

Maybe hc_apply() or hc_process()? Trying to cover all of the functionality of this combine would end in a name like find_or_create_and_callback_or_copyout :)

652