tcp_chg_pacing_rate() is expected to release the hw rate limit table,
but failed to do so in several error cases, leading to ever
increasing counts of flows using the rate.
This patch was mostly done by rrs
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D34058
Reviewed by: hselasky, rrs, jhb (inital version, outside of Differential)
When ip_output_send() returns EAGAIN due to issues with send tags (route
change, lagg failover, etc), it must free the mbuf. This is because
ip_output_send() was written as a wrapper/replacement for a direct
call to if_output(), and the contract with if_output() has
historically been that it owns the mbufs once called. When
ip_output_send() failed to free mbufs, it violated this assumption
and lead to leaked mbufs.
This was noticed when using NIC TLS in combination with hardware
rate-limited connections. When seeing lots of NIC output drops
triggered ratelimit send tag changes, we noticed we were leaking
ktls_sessions, send tags and mbufs. This was due ip_output_send()
leaking mbufs which held references to ktls_sessions, which in
turn held references to send tags.
Many thanks to jbh, rrs, hselasky and markj for their help in
debugging this.
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D34054
Reviewed by: hselasky, jhb, rrs
MFC after: 2 weeks
TCP_BBR:
- Fix a typo introducted in 1b90dfa5d2, which was reported by tuexen@
TCP_RACK:
- Correct two sysctl descriptions: s/corret/correct/
tcp_bbr(4): Also fix s/measurment/measurement/ in the man page
MFC after: 1 week
X_ip_mrouter_done might sleep, which triggers INVARIANTS to
print additional errors on the screen.
Move it outside the lock, but provide some basic synchronization
to avoid race condition during module uninit/unload.
Obtained from: Semihalf
Sponsored by: Stormshield
The clang compiler recently got an update that generates warnings of unused
variables where they were set, and then never used. This revision goes through
the tcp stack and cleans all of those up.
Reviewed by: Michael Tuexen, Gleb Smirnoff
Sponsored by: Netflix Inc.
Differential Revision:
Since trie configuration remains invariant during each DXR instance
lifetime, instead of shifting and masking lookup keys by values
computed at runtime, compile upfront several dxr_lookup()
configurations with hardcoded shift / mask constants, and choose the
apropriate lookup function version after each DXR instance rebuild.
In synthetic tests this yields small but measurable (5-10%) lookup
throughput improvement, depending on FIB size and prefix patterns.
MFC after: 3 days
LRO was willing to merge ACK and non-ACK packets together. This
can cause incorrect th_ack values to be reported up the stack.
While non-ACKs are quite unlikely to appear in practice, LRO's
behaviour is against the spec. Make LRO unwilling to merge
packets with different TH_ACK flag values in order to fix the
issue.
Found by: Sysunit test
Differential Revision: https://reviews.freebsd.org/D33775
Reviewed by: rrs
To check if it needed to regenerate a packet's header before
sending it up the stack, LRO was checking if more than one payload
had been merged into the packet. This failed in the case where
a single payload was merged with one or more pure ACKs. This
results in lost ACKs.
Fix this by precisely tracking whether header regeneration is
required instead of using an incorrect heuristic.
Found with: Sysunit test
Differential Revision: https://reviews.freebsd.org/D33774
Reviewed by: rrs
mrouter_done is called with RAW IP lock taken. Some annoying
printfs are visible on the console if INVARIANTS option is enabled.
Provide atomic-based mechanism which counts enters and exits from/to
critical section in ip_input and ip_output.
Before de-initialization of function pointers ensure (with busy-wait)
that mrouter de-initialization is visible to all readers and that we don't
remove pointers (like ip_mforward etc.) in the middle of packet processing.
When TCP_MD5SIG is set on a socket, all packets are dropped that don't
contain an MD5 signature. Relax this behavior to accept a non-signed
packet when a security association doesn't exist with the peer.
This is useful when a listen socket set with TCP_MD5SIG wants to handle
connections protected with and without MD5 signatures.
Reviewed by: bz (previous version)
Sponsored by: nepustil.net
Sponsored by: Klara Inc.
Differential Revision: https://reviews.freebsd.org/D33227
Provide structure inpcbstorage, that holds zones and lock names for
a protocol. Initialize it with global protocol init using macro
INPCBSTORAGE_DEFINE(). Then, at VNET protocol init supply it as
the main argument to the in_pcbinfo_init(). Each VNET pcbinfo uses
its private hash, but they all use same zone to allocate and SMR
section to synchronize.
Note: there is kern.ipc.maxsockets sysctl, which controls UMA limit
on the socket zone, which was always global. Historically same
maxsockets value is applied also to every PCB zone. Important fact:
you can't create a pcb without a socket! A pcb may outlive its socket,
however. Given that there are multiple protocols, and only one socket
zone, the per pcb zone limits seem to have little value. Under very
special conditions it may trigger a little bit earlier than socket zone
limit, but in most setups the socket zone limit will be triggered
earlier. When VIMAGE was added to the kernel PCB zones became per-VNET.
This magnified existing disbalance further: now we have multiple pcb
zones in multiple vnets limited to maxsockets, but every pcb requires a
socket allocated from the global zone also limited by maxsockets.
IMHO, this per pcb zone limit doesn't bring any value, so this patch
drops it. If anybody explains value of this limit, it can be restored
very easy - just 2 lines change to in_pcbstorage_init().
Differential revision: https://reviews.freebsd.org/D33542
Now that each module handles its global and VNET initialization
itself, there is no VNET related stuff left to do in domain_init().
Differential revision: https://reviews.freebsd.org/D33541
The historical BSD network stack loop that rolls over domains and
over protocols has no advantages over more modern SYSINIT(9).
While doing the sweep, split global and per-VNET initializers.
Getting rid of pr_init allows to achieve several things:
o Get rid of ifdef's that protect against double foo_init() when
both INET and INET6 are compiled in.
o Isolate initializers statically to the module they init.
o Makes code easier to understand and maintain.
Reviewed by: melifaro
Differential revision: https://reviews.freebsd.org/D33537
Allow the resending of DATA chunks to be controlled by the caller,
which allows retiring sctp_mtu_size_reset() in a separate commit.
Also improve the computaion of the overhead and use 32-bit integers
consistently.
Thanks to Timo Voelker for pointing me to the code.
MFC after: 3 days
Introduce a new function, lltable_get(), to retrieve lltable pointer
for the specified interface and family.
Use it to avoid all-iftable list traversal when adding or deleting
ARP/ND records.
Differential Revision: https://reviews.freebsd.org/D33660
MFC after: 2 weeks
While here move out one more erroneous condition out of the epoch and
common return. The only functional change is that if we send control
on a shut down socket we would get EINVAL instead of ECONNRESET.
Reviewed by: tuexen
Reported by: syzbot+8388cf7f401a7b6bece6@syzkaller.appspotmail.com
Fixes: f64dc2ab5b
This patch makes the handling of the SCTP_MAXSEG socket option
compliant with RFC 6458 (SCTP socket API) and fixes an issue
found by syzkaller.
Reported by: syzbot+a2791b89ab99121e3333@syzkaller.appspotmail.com
MFC after: 3 days
The intent is to provide more entropy than can be provided
by just the 32-bits of the IPv6 address which overlaps with
6to4 tunnels. This is needed to mitigate potential algorithmic
complexity attacks from attackers who can control large
numbers of IPv6 addresses.
Together with: gallatin
Reviewed by: dwmalone, rscheff
Differential revision: https://reviews.freebsd.org/D33254
Now struct prison has two pointers (IPv4 and IPv6) of struct
prison_ip type. Each points into epoch context, address count
and variable size array of addresses. These structures are
freed with network epoch deferred free and are not edited in
place, instead a new structure is allocated and set.
While here, the change also generalizes a lot (but not enough)
of IPv4 and IPv6 processing. E.g. address family agnostic helpers
for kern_jail_set() are provided, that reduce v4-v6 copy-paste.
The fast-path prison_check_ip[46]_locked() is also generalized
into prison_ip_check() that can be executed with network epoch
protection only.
Reviewed by: jamie
Differential revision: https://reviews.freebsd.org/D33339
The advanced TCP stacks (bbr, rack) may decide to drop a TCP connection
when they do output on it. The default stack never does this, thus
existing framework expects tcp_output() always to return locked and
valid tcpcb.
Provide KPI extension to satisfy demands of advanced stacks. If the
output method returns negative error code, it means that caller must
call tcp_drop().
In tcp_var() provide three inline methods to call tcp_output():
- tcp_output() is a drop-in replacement for the default stack, so that
default stack can continue using it internally without modifications.
For advanced stacks it would perform tcp_drop() and unlock and report
that with negative error code.
- tcp_output_unlock() handles the negative code and always converts
it to positive and always unlocks.
- tcp_output_nodrop() just calls the method and leaves the responsibility
to drop on the caller.
Sweep over the advanced stacks and use new KPI instead of using HPTS
delayed drop queue for that.
Reviewed by: rrs, tuexen
Differential revision: https://reviews.freebsd.org/D33370
For all functions that are leaves of tcp_input() call
ctf_do_dropwithreset_conn() instead of ctf_do_dropwithreset(), cause
we always got tp and we want it to be dropped.
Reviewed by: rrs, tuexen
Differential revision: https://reviews.freebsd.org/D33368
This function is always called from tcp_do_segment() method, that
can drop tcpcb and return unlocked.
Reviewed by: rrs, tuexen
Differential revision: https://reviews.freebsd.org/D33367
RFC792,1009,1122 state the original conditions for sending a redirect.
RFC1812 further refine these.
ip_forward() still sepcifies the checks originally implemented for these
(we do slightly more/different than suggested as makes sense).
The implementation added in 8ad114c082
to ip_tryforward() however is flawed and may send a "multi-hop"
redirects (to a host not on the directly connected network).
Do proper checks in ip_tryforward() to stop us from sending redirects
in situations we may not. Keep as much logic out of ip_tryforward()
and in ip_redir_alloc() and only do the mbuf copy once we are sure we
will send a redirect.
While here enhance and fix comments as to which conditions are handled
for sending redirects in various places.
Reported by: pi (on net@ 2021-12-04)
MFC after: 3 days
Sponsored by: Dr.-Ing. Nepustil & Co. GmbH
Reviewed by: cy, others (earlier versions)
Differential Revision: https://reviews.freebsd.org/D33274
VNET teardown waits 2*MSL (60 seconds by default) before expiring
tcp PCBs. These PCBs holds references to nexthops, which, in turn,
reference ifnets. This chain results in VNET interfaces being destroyed
and moved to default VNET only after 60 seconds.
Allow tcp_msl to be set in jail by virtualising net.inet.tcp.msl sysctl,
permitting more predictable VNET tests outcomes.
MFC after: 1 week
Reviewed by: glebius
Differential Revision: https://reviews.freebsd.org/D33270
