The need for a separate map for TSO and non-TSO seems to have arisen from
a misunderstanding about the DMA requirements for Intel drivers like
ice/ixl/iavf. Specifically, this Intel hardware has a quirk where no
more than N (=8) segs can be DMA'ed per packet on the wire. This limit
was enforced using a DMA map with a low segment limit for non-TSO and a
driver specific check for TSO. This led to a *lot* of complexity in
iflib (~80 extra lines of code that this change removes) when the
limit can simply be enforced via the existing EFBIG mechanisms.
This change adds a new flag, IFLIB_SPARSE_TX, that drivers for hardware
with this quirk must set. When set, it causes non-TSO sends that
exceed isc_tx_nsegment to be treated as if bus_dmamap_load_mbuf_sg()
had returned EFBIG.
Due to the API/KABI change, I've bumped the FreeBSD version as well.
Note kgalazka suggested the flag, rather than a more awkward
mechanism that I had in my initial patch.
Reviewed by: kbowling
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D60175
When 5e6e4f7528 landed, it inadvertently allowed unsupported / unknown
phys with a valid link_info->sig_mode to use media_type uninitialized.
Fix this by returning from those cases that do not assign a media_type
so as to avoid using it uninitialized.
Reported by: imp (forwarding an llm's post-commit review of previous
changes to this file)
Fixes: 5e6e4f7528
Reviewed by: imp, kbowling, sumit.saxena_broadcom.com
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D61164
In preparation for removing mp_ring, enable simple_tx by default.
Users encountering bugs are still able to set the tunable:
net.iflib.prefer_mpring="1"
at boot, but are encouraged to report any regressions so they
can be fixed.
My plan is to remove mp_ring assuming no major bugs are found in
simple_tx after a month or two.
Reviewed by: kbowling, netchild, rcm
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D61166
- Support 400G-DR4 optics
- Support 400G-CR8 and -SR8 with their types, rather than generic 400G-AUI8
- Use the generic 400G-AUI8 for 400G-SR4, rather than mislabeling it -DR4
(We probably should add a 400G-SR media type, but I don't have any to test ATM)
- Add missing legacy PAM4 100G LR entry and matching media-selection cases.
Reviewed by: sumit.saxena_broadcom.com
Differential Revision: https://reviews.freebsd.org/D61162
Sponsored by: Netflix
Fall back to if_baudrate if ifmedia_baudrate() returns 0 in
lacp_aggregator_bandwidth().
When lacp_aggregator_bandwidth() returns 0, lacp will
never come up, due to aggregator selection starting with a 0.
This is why its important for interfaces with unknown media
(meaning ifmedia_baudrate() returns 0) to fall back to
using if_baudrate in order to avoid this state.
Differential Revision: https://reviews.freebsd.org/D61084
Reviewed by: kbowling
Sponsored by: Netflix
Connect the baseline OpenSSL ARMv8 fused AES-GCM kernels to the OCF
ossl driver. Advertise the algorithm only when the system-wide capability set
includes both AES and PMULL.
Note that get_kernel_reg() is used to detect features such as PMULL
because the ossl device attaches before arm64 populates elf_hwcap when
ossl is built into the kernel or pre-loaded at boot.
Prefer OSSL over armv8crypto for AES-GCM sessions while retaining
armv8crypto as a fallback.
This provides about a 50% speedup for a Netflix ktls workload on a small
neoverse N1 board.
Reviewed by: markj
Constructive review feedback from: andrew, ngie
Differential Revision: https://reviews.freebsd.org/D60097
Sponsored by: Netflix
Map complete EFI descriptor ranges at the kernel page granularity
and merge compatible entries when 4K EFI regions share a kernel page.
Preserve execution when any sharing region requires it and reject
conflicting mapping attributes without panicing.
This gets my small N1 booting with 16K pages.
Reviewed by: chs, imp
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D60749
Changing the layout of the pkthdr requires external drivers
to be rebuilt, lest they access the wrong fields.
Sponored by: Netflix
Differential Revision: https://reviews.freebsd.org/D60691
When the leaf_rcv union was introduced to support ktls RX offload in
4d88d81c31 it was put at the front of the pkthhdr, and pushed the
more commonly accessed csum_flags and rsstype into the next cacheline.
Given that csum_flags is accessed on every pkt sent and every pkt freed,
but the leaf_rcv fields are only used for ktls rx offload, it makes more
sense to have csum_flags in a cacheline that is already likely to be hot.
Reviewed by: kib
Differential Revision: https://reviews.freebsd.org/D60691
Sponsored by: Netflix
In '398254f9999e mbuf: simplify m_free' we removed tag processing
on m_free() of M_NOFREE mbufs. It is possible that a packet filter
could attach a tag to this stack mbuf and call m_free()
on it, meaning that 398254f999 may have introduced a tag leak.
However,if a tag is attached without anyone calling m_free(), then
this leak was pre-existing.
Reviewed by: glebius
Differential Revision: https://reviews.freebsd.org/D60686
Fixes: 398254f999
Sponsored by: Netflix
The check for (M_PKTHDR|M_NOFREE) was added by np@ in
"91d1b03d3617 Merge r254736 from user/np/cxl_tuning."
in order to support a clever rx mode optimization used
by cxgbe at the time. That optimization was removed in
46e1e307ed several years ago. This check is otherwise
unneeded and adds an extraneous test to every mbuf free,
so let's remove it.
Reviewed by: glebius, np
Differential Revision: https://reviews.freebsd.org/D60686
Sponsored by: Netflix
This change makes all prefetching use the iflib prefetch wrappers, rather
than using prefetch() in some places and __builtin_prefetch() in others.
Note that __builtin_prefetch() does not work in the kernel on amd64 as clang
disables it when -mnosse is specified, so we must use the asm directives there,
and builtin_prefetch() elsewhere.
This change enables prefetching on non-x86 (notably arm64), and it enables
prefetching in netmap that has been there since day one, but which has never worked.
This is a no-op on non-netmap amd64 and i386
Differential Revision: https://reviews.freebsd.org/D60602
Reviewed by: glebius, kbowling
Sponsored by: Netflix
Avoid allocating per-transfer maps for coherent tags that cannot
bounce. Retain maps for cache synchronization, CCA realms, and KMSAN.
These un-used maps carry with them memory and cache miss overheads.
Reviewed by: andrew
Differential Revision: https://reviews.freebsd.org/D60098
Sponsored by: Netflix
This fixes a bug where a binary linked using max-page-size=0x200000
can result in a bogus relocation offset when running on a system
with a smaller page size. This causes samples to fall outside
the image mapping or be translated to the wrong address (resulting
in symbol resolution, or incorrect symbol resolution). We noticed
this at Netflix because we run a patchset enabling 16k pages on
amd64 and have been compiling userspace with a 2MB page size.
Since we started doing this profiling userspace binaries has been
wonky.
Reviewed by: ali_mashtizadeh.com
Differential Revision: https://reviews.freebsd.org/D59771
Sponsored by: Netflix
net.iflib.prefer_mpring can be used to control whether or
not all iflib driver instances default to mp_ring or simple_tx.
This is intended to be temporary, to allow easy testing (now) of
simple_tx, and to allow an easy fallback to the legacy path
(later) once the default is switched to simple_tx
Until now, simple tx has used its own queue selector, and has
ignored isc_txq_select and isc_txq_select_v2 (not this only
seems to matter for ice(4) with dcb enabled). This change
makes simple-tx use isc_txq_select* when present.
The implementation is defined to be efficient, with a
transmit routine chosen up-front that hard-codes the
queue selection and calls an always-inlined body. This
avoids a useless test per packet in the hotpath, and
also may avoid speculation into header parsing.
Note that this was designed for readability and efficiency
in the common case (interface up, not ALTQ). That's why
we do queue selection without duplicating nic-running and
altq checks, leaving them to the common implmentation.
Sponsored by: Netflix
Reviewed by: kbowling
Differential Revision: https://reviews.freebsd.org/D59712
Implement buf_ring/drbr deferred transmit in iflib. This is intended
to allow the new simpler code path to replace mp_ring. This patch
makes the simple_tx outperform mp_ring by a wide margin when CPU is
the bottleneck (eg, cannot fill the NIC). See graphs at:
https://people.freebsd.org/~gallatin/mpring_vs_simple_tx
Note that the buf ring is used for contention, not capacity. Eg,
it is used as a place for contending threads to put packets without
waiting for a mutex. It is not designed to act as a software ring
on top of the hardware descriptors provided by the underlying NIC
driver. "stranded packets" are exceedingly rare due to the fact that
if there is enough load to use the buf_ring, there will probably be
more load coming that can be a drainer. Not scheduling a gtask to
drain is intentional, and we really on the timer as a fallback.
One thing I noticed while developing this patch is that a simple mutex
with no deferral generally outperformed both mp_ring and drbr at high
levels of contention for the same queue. This is inherent in a
bounded MPSC queue where multiple producers are contending on claiming
ring entries. So I came up with the idea of bounding the number of
producers such that the deferral ring would devolve to a mutex when
contention was high.
Identifying the crossover point in a general way was hard. On
different machines, the point between a mutex and a deferral ring was
very different and also depended on the placement of the producers. I
eventually realized that on the large AMD EPYC servers that I was
testing on, the crossover point generally coincided with the work
spilling into another CCX. So I developed an approach where we limit
the number of producers by limiting simultanious producers to the same
L3 AND by bounding the number of simultanious producers.
This limit can be adjusted via the sysctl net.iflib.max_producers.
The number of packets drained from the deferral ring is limited
by net.iflib.simple_drain_quota. The intent is to process just
enough packets in the gtaskq context so as to make space to
allow threads to make progress. The task also uses a trylock
so as to avoid blocking, waiting for a thread to drain.
This change also enables ALTQ support for simple tx.
Reviewed by: kbowling
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D58901
The new watchdog code triggers spurious watchdog resets on
NICs doing KTLS offload. Fix this by using the actual segments
consumed by the NIC driver's isc_txd_encap.
The issue is that rs_pending is updated using an estimate of
the descriptors that will be used for the current packet,
based on what bus_dma produced. However, NICs which support
ktls offload may do extra DMAs (and consume extra
descriptors) to derive crypto state when re-transmitting
TLS segments. This is the reason for allowing drivers to control
ift_pad. When this happens, the estimated rs_pending may undercount.
This may also happen if NIC drivers consume extra descriptors for
other reasons. (eg, hw errata handling on e1000)
Reviewed by: kbowling
Differential Revision: https://reviews.freebsd.org/D59321
Sponsored-by: Netflix
When _task_fn_admin() is active, it will regularly call
IFDI_UPDATE_ADMIN_STATUS(). So there is no need to do it in
iflib_media_status. This can be fairly expensive on some drivers (long
DELAY busywait loops waiting for a NIC command), and there is no need
to pause a userspace app in this DELAY() if it is happening
asynchronously anyway.
Note the logic to detect if _task_fn_admin() is regularly calling
IFDI_UPDATE_ADMIN_STATUS() was copied from that function.
Reviewed by: erj, kbowling
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D54096
When getting some baseline ALTQ numbers, I noticed that
if simlple_tx is enabled in kenv, we wind up re-setting the transmit
routine, but I neglected to actually clear ctx->ifc_sysctl_simple_tx.
That leads to many different panics as we run a mixture of mp_ring
and simple_tx.
Pointy-hat to: gallatin
Sponsored by: Netflix
- Fixed memory leaks around m_dup() not freeing the original chain on
failure. If we return ENOMEM, we are expected to have freed the
chain, else the mbuf would be leaked. Also updated iflib_ether_pad()
to follow the same structure.
- In iflib_parse_header()
o Fixed a bug where the ip/ip6 and th pointers may point into a
freed chain after m_pullup. Those pointers must be reset to
point into the new chain.
o Eliminate ENXIO returns for non-TCP TSO sends (which would violate
the mbuf ownership contract if they could happen). Since they
cannot happen, I made them assertions instead.
- in iflib_ether_pad(), return ENOMEM after freeing mbuf, so that
mp_ring knows it is free. An ENOBUFS error will cause the mp_ring
path to retain the mbuf and retry
- in iflib_encap():
o Fix a leak when bus_dmamap_load_mbuf_sg() returns ENOMEM
o Fix a use-after-free in the mp_ring path when a driver using
ktls frees an mbuf and returns ENOBUFS via iflib_encap()
After this change the expection from iflib_encap is that:
mp_ring: ENOBUFS can be returned only when we run out of descriptors
(ENOBUFS causes mp_ring to retain the mbuf).
simple_tx: iflib_encap() always consumes the mbuf, regardless of the
return
Note that iflib_debugnet_transmit(), like simple_tx, expects that
iflib_encap() always consumes mbufs. This will be true after mp_ring
is removed, and its such a rare special case (overrunning the ring
during panic dumps) that I don't think its worth fixing in the
meantime.
Sponsored by: Netflix
Reviewed by: kbowling, sumit.saxena_broadcom.com
Differential Revision: https://reviews.freebsd.org/D58843
Fixes: 074ff87463
This fixes a bug where we do not report all lanes when
a NIC configures a breakout. Eg, we reported all 4
lanes when a NIC configured the optics as 1x400g, but
only printed the first lane's strength when configured as
4x100g.
Fix this by actually parsing the active lane count, rather
than pulling it from the default descriptor.
While here, optionally print page 10h when -vvvv is
specified. This aids in determining how a breakout is configured.
I put it under an extra level of verbosity, as I don't want
to let things get out of hand printing CMIS pages.
Sponsored by: Netflix
Reviewed by: kib, sumit.saxena_broadcom.com
Differential Revision: https://reviews.freebsd.org/D58263
Some nic drivers (including iflib) do not initialize if_hwassist
until after the interface is brought up. If a lagg member is included
in a lagg when its not yet been brought up, that will cause lagg to see
if_hwassist=0 and will disable all checksum offload, etc, on the interface.
This is almost impossible to debug without kgdb or dtrace, as ifconfig does
not surface if_hwassist.
Fix this by re-calculating lagg caps (including if_hwassist) after adding a port.
I encountered this problem when I had a commented-out if_foo1=up entry in
my rc.conf that i neglected to uncomment when I was re-configuring a lagg.
Sponsored by: Netflix
Reviewed by: markj, zlei
Differential Revision: https://reviews.freebsd.org/D58062
TLS receive offload is really only beneficial for in-kernel use cases
(such as NFS over TLS) or when using a hardware offload. In addition,
several recent SAs have involved the TLS receive path, but the only
current mitigation for those is to disable TLS offload entirely.
Reviewed by: ziaee, gallatin, markj
Relnotes: yes
Sponsored by: Netflix
Sponsored by: Chelsio Communications
Co-authored-by: John Baldwin <[email protected]>
Differential Revision: https://reviews.freebsd.org/D57974
We need to report the actual link speed, rather than a hard-coded
100Gbs for a variety of reasons, but most importantly, as reporting
100Gbs on a 10g link breaks lacp when this NIC is in an LACP bundle
with other vendors' NICs after e98ed8d99f (lacp:
Simplify lacp_compose_key()), as the fake 100g puts this nic
into a different aggregation group.
Sponsored by: Netflix
Reviewed by: glebius
Tested by: glebius
Until we introduced support for nic ktls offload, all error returns
from isc_txd_encap() indicated a permanent failure. Iflib remapped all
those failures to ENOMEM, which was treated by the tcp stack as a
permanent error and passed back to the caller. This was done to avoid
creating "infinite loops" where a packet couldn't be mapped for
transmit, and kept being sent over and over.
Now that we have support for nic ktls offload, some ktls offload
drivers may return ENOBUFS from their encap function to indicate that,
for example, the ktls context may not yet be fully initialized. This
needs to be treated as a transient error so that the TCP stack may
re-try at a later time. To achieve this, pass the raw error back to
the caller when the encap routine returns an error aside from EFBIG.
Note that I audited all in-tree iflib drivers. Only ice and ixl ran
return anything other than 0 from their encap, which is EFBIG. both of
which are still treated as they were before.
Testing with an out-of-tree ktls offload nic using iflib showed ENOMEM
errors from sendmsg() before this change, and no more errors passed
back to userspace after this change.
Reviewed by: kp, sumit.saxena_broadcom.com
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D57550
This change refactors fatal error delivery via APEI and prints more info:
- Makes the NMI handler call into the ge handler to establish a common
code flow, no matter how the error is delivered
- Adds the FRU to the panic string so as to provide more information than
just "APEI Fatal Hardware Error!" such as
"APEI Fatal Hardware Error: PcieError"
- Prints more details about fatal pcie errors. Note that we skip acquiring
Giant on fatal errors
- Hexdumps the full GED data on fatal errors, so as to facilitate
offline data analysis
Reviewed by: imp
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D57417
lacp uses interface speed to separate lacp members into different
aggregation groups. It wants to use the lower 4 bits of the key
to represent the speed. This change fixes a few bugs around that:
1) Actually use the baud rate reported by the interface as the speed
(and fall back to use the baudrate associated with the media if
the interface somehow doesn't support if_baudrate)
2) Compressess the baud rates down to the 4 bits reserved for them.
Using things like FM_400G_FR8 does not fit in 4 bits (its value
is 0x1811) . In fact, interfaces faster than 1Gb/s don't fit in
4 bits using the old scheme
3) Emits a warning on the console once per boot if it encounters a
NIC with an unsupported speed to make it slightly more obvious
why LACP might not behave as expected.
This was prompted by a 400g nic with a broken media detection
reporting "unknown" as its speed, and being unable to be in the
same aggregation group with a different 400g nic that was properly
reporting its speed.
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D57026
Reviewed by: slavash
This change removes the hard-forcing of PCI_IOV and adds shims to
allow the driver to compile and work when the kernel is missing
PCI_IOV support.
Fixes: 7c450d1127
Reviewed by: sumit.saxena_broadcom.com
Differential Revision: https://reviews.freebsd.org/D57300
Sponsored by: Netflix
The recent IBS work intruduced a bug on older CPUs where the
IBS handler will be called on NMIs even when IBS is not initialized.
Work around this in the IBS handler by checking to see if ibs_pcpu
is NULL before accessing it.
When using SW KTLS, we must account for the headers in sf_iodone()
in terms of either freeing or enqueuing them for TLS work.
Not doing so can lead to a situation where we enqueue only
the payload, and not the header, for encryption. Rather than
leaking the header, the socket is left "hung" with the header marked
M_NOTREADY.
Sponsored by: Netflix
Reviewed by: glebius, kib
Differential Revision: https://reviews.freebsd.org/D57134
MFC After: 14 days
This reverts commit 2fe37927d4.
This turns out to have been misguided. First, clearing the
hash results in all loopback ip/ip6 traffic being hashed
to the netisr queue associated with the if_index of the loopback
interface. Eg, it bottlenecks loopback traffic. When the
hash is kept, traffic is spread evenly among netisrs.
Also, it is safe to keep the hash here. The clearing was only
needed when RSS core selection is enabled; we only enabled the
consistent hashing parts of RSS globally, not the cpuid mapping
stuff. So there is no need to clear it.
Reviewed by: glebius
Sponsored by: Netflix
This reverts commit ce33f96fcf.
It turns out that doing it this way did indeed prevent backwards
movement of timestamps, however it also lead to an ever increasing
error, eventually yielding timestamps hundreds or thousands of
seconds in the future.
Back this out until we can come up with a solution that prevents
backards timestamps and also avoids accumulating error.
Sponsored by: Netflix
When we have multiple aggregators, the link state should reflect the
state of the active aggregator.
This change was prompted by a script pruning 10GbE interfaces from an
lacp bundle with 100GbE interfaces. Mixing speeds like this creates multiple
aggregators. When the last 10GbE interface was removed, lagg0 would loose
link because the current aggregator's port count would drop to 0, even
though the 100GbE aggregator had active ports. This left the system in a
hard to diagnose state where lagg0 reported "active", but all outgoing
IP traffic was dropped, due to the RT_LINK_IS_UP() check noticing lagg0's
if_link_state was marked as down.
Reviewed by: zlei
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D56579
When under heavy load or churn, inline ktls offload NICs may run out
of hardware resources described by ktls send tags. Rather than
waiting for connections to pass through the time_wait state, reclaim
the ktls send tags early, at entry to time_wait. By preventing
potentially tens or hundreds of thousands of sessions from holding
send tags in time_wait, this allows more ktls sessions to be offloaded
to hardware.
Reviewed by: glebius, kib, nickbanks_netflix.com, rrs, tuexen
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D56610
It turns out interface ioctls are defined not just in sockio.h, but are
spread among many files. When I added SIOCGI2CPB at the bottom of the
file, the next number (160) collided with an ioctl (IPSECGREQID) that
I was unaware of in another file. Fix this by moving to a number that
is unclaimed.
Fixes: cf1f215728 (net: Add SIOCGI2CPB ioctl & add page/bank fields to ifi2creq)
Reported by: dhw
Reviewed by: imp
- kaudit_to_bsm: Log IPv6 as well as IPv4 and unix addrs
- au_to_sock_inet128: Treat ports the same way as au_to_sock_inet32() as
just pushing a uint16 causes byte ordering problems on little endian.
Differential Revision: https://reviews.freebsd.org/D39633
Sponsored by: Netflix
Reviewed by: csjp
When TCP_REQUEST_TRK is enabled, the tcb grows by 600 bytes
to accommodate the t_tcpreq_info[MAX_TCP_TRK_REQ] array.
Even when the option is enabled, not every connection is using
this feature. So let's allocate it on-demand, and save 600
bytes in the common case.
Sponsored by: Netflix
Reviewed by: rrs, tuexen
Differential Revision: https://reviews.freebsd.org/D56484
When using software based ifnet counters, iflib has not factored
TSO into account when reporting the segments and bytes sent.
So it will underreport NIC bandwidth by a small percent,
and will undercount sent segments by a large factor.
Fix this by calculating the number of added segments the NIC
will send, and add header size multiplied by that number
to arrive at a correct accounting of segments and bytes sent.
This makes these software counters directly comparable to
hardware counters.
Doing this requires moving the calculation into iflib_encap() where
we have already parsed the packet and know the header size, MSS, etc.
Differential Revision: https://reviews.freebsd.org/D56338
Sponsored by: Netflix
If we are low on TX descriptors, bypass iflib_txq_can_reclaim()
and force a reclaim. This is intended to reduce the number of
output drops under heavy load when using simple transmit.
Differential Revision: https://reviews.freebsd.org/D56339
Sponsored by: Netflix
The clock calibration routine currently can result in rx timestamps
jumping backwards, which can confuse the TCP stack.
Ensure they are monotonically increasing by estimating what
we'd calculate as the next timestamp and clamp the calibration
so new timestamps are no earlier in time.
Reviewed by: kib, nickbanks_netflix.com
Tested by: nickbanks_netflix.com
Differential Revision: https://reviews.freebsd.org/D56427
Sponsored by: Netflix
Make loopback packet drops more obvious by reporting them
in interface stats visable via netstat -ni
Since loopback uses netisr, packets can be dropped if the
netisr queue overflows. These drops are visible via
netisr -Q, but its not an obvious place to look.
Differential Revision: https://reviews.freebsd.org/D56356
Reviewed by: glebius, tuexen
Sponsored by: Netflix
When lro mbuf queuing is enabled, we should not queue easily
reject-able packets. Queuing them does a bit of extra work (sorting,
timestamps) and can potentially delay urgent packets such as LACP
PDUs. This change moves simple rejection tests from lro_rx_common()
into lro_rx and (more importantly) into tcp_lro_queue_mbuf().
Note this change only moves the easy checks on forwarding and packet
metadata, where the rejection criteria is already hot in cache. It
does not move parsing and looking inside the packet to verify the
ether protocol, ip protocol, etc. This could be done, but we risk
essentially doubling the cache misses per-packet by doing so.
Differential Revision: https://reviews.freebsd.org/D56337
Reviewed by: rrs, tuexen
Sponsored by: Netflix
Add support for CMIS based optics, typically used by 400GbE
and faster ethernet optics. The CMIS standard requires paged
support for i2c ioctls.
This has been tested on an Nvidia ConnectX-7 and Broadcom
Thor2 400GbE NIC, and I have verified that optics vendor information,
light levels, and temperatures match the information provided by
various vendor tools.
Differential Revision: https://reviews.freebsd.org/D56265
Reviewed by: kbowling, sumit.saxena_broadcom.com
Sponsored by: Netflix
This commit adds page & bank fields to ifi2creq in preparation
for adding CMIS support for 400g optics to ifconfig.
The new ioctl SIOCGI2CPB is added, so that drivers can distinguish
between callers asking for page/bank selection and legacy callers
that simply failed to zero out all ifi2creq fields.
The mlx5en(4) driver and iflib(4) driver frameork have been updated
to use this new SIOCGI2CPB ioctl and support page/bank operations.
A follow-on patchset will add support to ifconfig for reporting
data from CMIS optics.
This has been tested on Nvidia ConnectX-7 and Broadcom Thor2 (using
out of tree driver) based NICs.
Differential Revision: https://reviews.freebsd.org/D55912
Sponsored by: Netflix Inc.
Reviewed by: kib
The recent FIBify commits deref struct thread without including
sys/proc.h, which can result in a compiler error. This becomes
apparent when building with LINT-NOVIMAGE, as net/vnet.h includes
sys/proc.h. Fix this by directly including sys/proc.h
Fixes: 4c486fe402 ("ip_mroute: FIBify"), 0bb9c2b665 ("ip6_mroute: FIBify")
Add a new tunable/sysctl (kern.ipc.splice.num_wq) which can be used
to limit the number of splice worker queues as a way to limit
splice cpu use.
The default (-1) keeps the current behavior of running one worker
for each core in the system. An administrator can set it to 0 (either
via tunable, or before the first splice call via sysctl) to
effectively disable splice, or some number smaller than the number of
cores to limit splice thread use.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D55579
Sponsored by: Netflix
When adding the IFLIB_GET_MBUF/FLAGS, I neglected to NULL out the
mbuf in the descriptor ring. I didn't think this should matter as
the I thought this code was only used when the ring was about
to be freed. But I was wrong, and leaving a stale mbuf in there can
cause panics.
Reported by: Marek Zarychta (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=292547)
Fixes: 14d93f612f
Sponsored by: Netflix
Allow reading of i2c addresses other than A0. A0 does provide most
information, but doesn't provide things like module temps, and optical
signal levels.
Sponsored by: Netflix
Reviewed by: sumit.saxena_broadcom.com
Differential Revision: https://reviews.freebsd.org/D54590
MFC after: 3 days
Freeing completed transmit mbufs can be time consuming (due to them
being cold in cache, and due to ext free routines taking locks),
especially when we batch tx completions. If we do this when holding
the tx ring mutex, this can cause lock contention on the tx ring mutex
when using iflib_simple_transmit.
To resolve this, this patch opportunistically copies completed mbuf
pointers into a new array (ifsd_m_defer) so they can be freed after
dropping the transmit mutex. The ifsd_m_defer array is
opportunistically used, and may be NULL. If its NULL, then we free
mbufs in the old way. The ifsd_m_defer array is atomically nulled
when a thread is using it, and atomically restored when the freeing
thread is done with it. The use of atomics here avoids
acquire/release of the tx lock to restore the array after freeing
mbufs.
Since we're no longer always freeing mbufs inline, peeking into them to see if a
transmit used TSO or not will cause a useless cache miss, as nothing
else in the mbuf is likely to be accessed soon. To avoid that cache
miss, we encode a TSO or not TSO flag in the lower bits of the mbuf
pointer stored in the ifsd_m array. Note that the IFLIB_NO_TSO flag
exists primarily for sanity/debugging.
iflib_completed_tx_reclaim() was refactored to break out
iflib_txq_can_reclaim() and _iflib_completed_tx_reclaim()
so the that the tx routine can call iflib_tx_credits_update()
just once, rather than twice.
Note that deferred mbuf freeing is not enabled by default, and can be
enabled using the dev.$DEV.$UNIT.iflib.tx_defer_mfree sysctl.
Differential Revision: https://reviews.freebsd.org/D54356
Sponsored by: Netflix
Reviewed by: markj, kbowling, ziaee
Document the new tx related sysctls I've added to iflib in the
last few months: tx_reclaim_ticks & tx_reclaim_thresh
Sponsored by: Netflix
Reviewed by: ziaee
Differential Revision: https://reviews.freebsd.org/D54564
MFC after: 3 days
Ever since "d9c55b2e8cd6 rss: Enable portions of RSS globally.."
exposed the RSS software hashing functions, it has been possible
to use them without "ifdef RSS". Do so now in the syncache
so as to get flowids recorded.
Note that the use of the rss hash functions is conditional on IP versions,
so we must ifdef INET to ensure rss_proto_software_hash_v4() is available.
Fixes 73fe85e486
Sponsored by: Netflix
Reviewed by: glebius, p.mousavizadeh_protonmail.com, nickbanks_netflix.com, tuexen
Differential Revision: https://reviews.freebsd.org/D54534
According to section 8.3.2.2.1 of the XL710 datasheet,
FLTSTAT indicates if RSS hashing was used (or flow director
or nothing) to arrive at the flowid it is reporting.
We need to check this before reporting the flowid and setting the
rss type. Not checking this allows invalid flowids (0s) to be
reported for some TCP traffic (it seems to mostly impact incoming
connections). This leads to the inp flowid being 0, and egress
traffic being unbalanced (going to tx queue 0, and NIC 0 of an
lacp bundle).
Differential Revision: https://reviews.freebsd.org/D54442
Reviewed by: adrian, kbowling
Sponsored by: Netflix
MFC after: 2 weeks
Some drivers want the TLS seqno when offload starts. Capture this
for them by adding a union for initial_seqno, sharing space with
the TLS 1.0 next_seqno.
Reviewed by: jhb
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D54275
MFC After: 1 month
This change adds support to iflib for drivers that want to do
transmit-side NIC ktls offload. This change does 2 things:
1) Extends the pkt info to include an optional mbuf pointer.
This gives drivers the ability to find the start of a TLS record if
they need to re-DMA part of the record to re-construct TLS state on
the NIC. This mbuf pointer is only passed when CSUM_SND_TAG is
present on the pkthdr. Note that I don't bother to inspect the send
tag on purpose; this will only be present for TLS offloaded or paced
connections
2) Allows the driver to specify how much ring padding is needed
before the ring is considered to be full using the new isc_tx_pad
field in if_softc_ctx.
This re-uses a field that was marked spare in 2019 via d49e83eac3.
Iflib initializes this to the previous value of 2 slots and allows the
driver to override it. The TXQ_AVAIL() macro has been adjusted to
subtract this padding, and uses of the macro have removed +2 from the
other side of the comparison. To avoid potential cache misses from
looking at the ifc_softc_ctx in TXQ_AVAIL(), the value is mirrored in
the txq (in an alignment hole).
Reviewed by: kbowling, kgalazka, sumit.saxena_broadcom.com, shurd
Sponsored by: Netflix
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D54274
This makes it easier to reason about system topology, and to
potentially map applications to NIC queues by (ab)using the
mbuf flowid to select egress NIC and queue in a predictable fashion.
Differential Revision: https://reviews.freebsd.org/D54053
Reviewed by: glebius, kbowling
Sponsored by: Netflix
- change the state to SPLICE_EXCEPTION to allow so_unsplice() to work
to cleanup failed splices (fixes socket reference leak)
- NULL out sp->dst when unsplicing from so_splice() before so2 has been
been referenced.
- Deal with a null sp->dst / so2 in so_unsplice
- Fix asserts that talked about sp->state == SPLICE_INIT; that state
is not possible here.
Differential Revision: https://reviews.freebsd.org/D54157
Reviewed by: markj
Sponsored by: Netflix
Fixes: c0c5d01e53 ("so_splice: Synchronize so_unsplice() with so_splice()")
MFC after: 3 days
Clear the RSS hash on transmit, now that RSS hashing is enabled
unconditionally, and the network stack may want to trust that
it is getting the correct hash on input.
Differential Revision: https://reviews.freebsd.org/D53090
Reviewed by: zlei
Sponsored by: Netflix
Now that we can trust NICs to supply an identical hash result
to software, we can setup the inpcb hash on outgoing connections.
This gives us symmetric hashing, meaning packets should enter
and leave on the same NIC queue.
Differential Revision: https://reviews.freebsd.org/D53104
Reviewed by: adrian, cc, kbowling, tuexen, zlei
Sponsored by: Netflix
We use the fact that all NICs that support hashing are using the
same hash algorithm and hash key to enable symmetic hashing in
TCP, where a software version of the same hash is used to
establish hashes on outgoing connections.
Sponsored by: Netflix
Reviewed by: adrian, zlei (both early version)
Differential Revision: https://reviews.freebsd.org/D53089
Use the same check as iflib_if_transmit() to detect when the
interface is down and return the proper error code, and also
free the mbuf.
This fixes an mbuf leak when a member of a lagg is brought
down (and probably many other scenarios).
Sponsored by: Netflix
Remove prefetching from the transmit path of iflib in the interest of
increased performance and reduced complexity. Details regarding the
performance penalties of prefetching can be found in the differential
review.
Note this prefetching was only done on link speeds of 10Gb/s and
above, so the change is a no-op (or perhaps slight performance
improvement simply due to the code simplification) for slower
interfaces.
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D53674
Reviewed by: kbowling, markj, mjg
Change tag preallocation to happen asynchronously when an interface is
brought up, so as to reduce boot times when preallocating tags.
- A new mlx5-tls-prealloc_wq is allocated when preallocation is
desired, and started when an interface is opened
- The bulk of the prealloc code remains the same, except the
allocations are now M_NOWAIT. M_NOWAIT is needed because, since the
preallocation is done asynchronously, and since tag allocation is
not instant, we could race with a real TLS session trying to
allocate a tag. Note that in this case, we take allocation failure
as a sign that we were unable to obtain the entire zone due to there
being other consumers. This was suggested by @markj as a way to
keep things simple, after discussing why uma_zone_get_cur() didn't
immediately report a fully allocated zone. If this turns out to be
problematic, we could use uma_zone_set_maxaction() to stop
pre-allocations (also suggested by Mark)
Reviewed by: glebius, kib, markj
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D53570
When system call auditing is enabled, every audited call
does a lookup in the evclass hash table. This table
appears to be insert only (eg, nothing can be removed)
and protecting it with an rwlock is overkill. Using
an rwlock causes just the atomic operations to maintain
uncontended rwlock state to be responsible for measurable
overhead on high core count servers making lots of system calls.
Given that the evclass hash table can never have items removed,
only added, using a mutex to serialize additions and converting
to ck_list allows sufficient protection for lockless lookups.
In a contrived example of 64 cores, all reading 1 byte from their
own file, this change increases performance from 5M reads/sec
to 70M reads/sec on an AMD 7502P.
Reviewed by: markj, mjg, glebius (privately)
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D53176
On some iflib drivers, the txd reclaim routine can be fairly expensive
at high packet rates. Iflib was designed with the intent of only
reclaiming tx descriptors above a configurable threshold, but this
logic was left unimplemented.
This change:
- implements 2 new knobs, iflib.tx_reclaim_thresh and
iflib.tx_reclaim_ticks.
- moves tx reclaim thresh from the if_shared_ctx and into the
iflib_ctx as drivers don't need to see it, and it needs to be
changed, so it can't be const
- tx_reclaim_thresh and ticks are replicated into the txq to
improve cache locality of data accessed in the hot path
- ticks is used rather than more expensive timekeeping mechanism so
as to keep things simple and cheap
This change substantially improves packet rates on bnxt. It has been
tested on bxnt and ixl
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D52561
Reviewed by: markj (initial version)
- Fix an mbuf leak with iflib.simple_tx=1 when we run out of tx descs
in iflib_encap(). It seems odd to free the mbuf in iflib_encap(),
but that routine consumes mbufs for other reasons, and it seemed
safest to free there rather than have the simple tx routine parse
return values to determine what needed to be freed.
- Increment counters for output drops when ENOBUFS is encountered
and output errors when other transmit errors are encountered for both
the simple and normal tx routines.
- Performed driver changes so that iflib drivers now add the generic
output drop and output error counters to their private counters in their
ifdi_get_counter routines.
Reviewed by: kbowling, markj
Differential Revision: https://reviews.freebsd.org/D52369
Sponsored by: Netflix
While mp_ring can provide amazing scalability in scenarios
where the number of cores exceeds the number of NIC tx
rings, it can also lead to greatly reduced performance in simpler,
high packet rate scenarios due to extra CPU cycles and cache
misses stemming from its complexity.
This change implements a simple if_transmit routine, selected
at driver load. This routine does not queue anything, and uses
a simple queue selection and ends up being far more cache
friendly.
In testing on a 400GbE NIC in an AMD 7502P EPYC server, this
simple tx routine is roughly 2.5 times as fast as mp_ring
(8Gbs -> 20Gb/s). and 5x as fast as mp_ring with tx_abdicate=1
(4Gbs -> 20Gb/s) for a simple in-kernel packet generator, which
is closed source currently. It also shows a 50% speedup for
a simple netperf -tUDP_STREAM test (5Gb/s -> 8Gbs).
This change is mostly a noop, as it not enabled by default.
The one exception is the change to iflib_encap() to immediately
reclaim completed tx descriptors, and only failing the transmit
and scheduling a later reclaim if iflib_completed_tx_reclaim()
didn't free enough descriptors.
Reviewed by: kbowling, sumit.saxena_broadcom.com, vmaffione
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D51905
Simplify LRO handling by:
- removing fowarding checks... LRO will already do this
- using the rss sorted LRO (tcp_lro_queue_mbuf()) API, which
allows LRO to accumulated large numbers of packets
and sort them by LRO hash and arrival time, leading to
far higher LRO aggregation rates on servers with lots
of connections (esp. internet facing servers).
- eliminate cache busing loop over chained packets
Note that this change may slightly pessimize a silly case
(LRO enabled + IP forwarding enabled) wth the benefit of giving
a small speedup for forwarding with LRO disabled, and a very
large speedup for LRO w/o forwarding disabled. And the large
benefit of simplifying the code.
Reviewed by: kbowling, markj
Tested by: olivier
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D51772
The IPv4 packet parsing logic in iflib is incredibly complex,
prematurely optimized, and believes all the world is TCP.
This causes it to pullup part of the UDP payload into
the packet header, causing unneeded memory copies.
This impacts a project I'm working on, and also impacts
nearly any kernel user of UDP, like NFS. Eg, NFS over UDP
will result in pullups for every datagram sent over an iflib NIC.
This patch:
- adds parsing for UDP to iflib
- attempts to pull up the correct header size, based on UDP or
TCP protocol type.
- simplifies packet parsing in iflib by
- no longer special casing having an ethernet header in a packet by
itself
- no longer checking that we're trying to pullup something beyond the
end of the packet. Since we're no longer trying to pull up a
larger TCP header, attempting to pullup something larger than
the packet should no longer happen. If it does, the packet is
malformed and m_pullup will return an error when it runs out of
data in the mbuf chain
Reviewed by: erj, glebius, kbowling
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D51748
In a mixed lagg, its likely that ifcaps or hwassist may not
match between members. If this is true, the logical OR will
be short-circuited and if_hw_tsomax_update() will not be called.
Fix this by calling it inside the body of the if as well
Sponsored by: Netflix
MFC after: 2 weeks
When encountering a failed NIC, the mlx5 driver will wait up to 120
secs for the firmware to respond. This timeout is absurdly huge, and
leads to boot times of 40 minutes to over an hour on our servers when a
NIC fails. This is because the driver will attempt to attach to the
failed NIC multiple times (once for each driver loaded after mlx5),
and wait 2 minutes on each attempt. This happens because the mlx5
driver is still the best match for the device. This delay then
triggers watchdog timeouts in our environment, rendering servers
with a failed NIC entirely unbootable without manual intervention.
Note that FW_INIT_WARN_MESSAGE_INTERVAL must also be decreased, as
it must be less than the init timeout.
Reviewed by: kib (initial version, before reducing warn interval)
Sponsored by: Netflix
UDP over IPv6 was not leaving space for link headers,
resulting in the ethernet header being placed in its own mbuf
at the front of the mbuf chain sent down to the NIC driver.
This is inefficient, in terms of allocating 2x as many
header mbufs as needed, and its also confusing for drivers
which may expect to find ether/ip/l4 headers together in the same
mbuf.
Reviewed by: glebius, rrs, tuexen
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D49840
This is a port of e6ccd70936, which was done by Robert
Watson in 2004 for IP4
This fixes a bug where optics on 100G and faster NICs
were not properly reported.
The problem is that we pull the string from the correct
table in ifconfig_sfp_physical_spec only when sfp_eth_1040g
contains the SFP_ETH_1040G_EXTENDED bit. However, we were
never saving that bit when it was encountered. This change
records that bit into sfp_eth_1040g, allowing us to later
select the appropriate ID string.
This should cause most 100G interfaces to stop being identified
as "unknown" in the "plugged" output of ifconfig -v, and to
start being identified as what they really are.
Example output from a Chelsio T6 with SR4 optics in one port
and DR1 optics in another:
Before:
plugged: QSFP28 Unknown (MPO 1x12 Parallel Optic)
plugged: QSFP28 Unknown (LC)
After:
plugged: QSFP28 100GBASE-SR4 or 25GBASE-SR (MPO 1x12 Parallel Optic)
plugged: QSFP28 100GBASE-DR (LC)
Reviewed by: kbowling, np
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D49127
MFC after: 7 days
- Fix a bug where the semantics of refcount_release() were
reversed. This would lead to the nmi interrupt being prematurely
masked in the local apic, leading to an out-of-tree profiling
tool only getting results the first time it was run.
- Stop executing nmi handlers after one claims the interrupt.
The core2 hwpmc handler seems to be especially heavy, and running it
in addition to vtune's handler caused roughly 50% of the nmi interrupts
to be lost (and caused vtune to give worse results).
Reviewed by: bojan
Sponsored by: Netflix
In 4cc5d081d8, a change was introduced that manipulated
drv_ioctl_data->reqcap using IFCAP2 bits. This was noticed
when creating a mixed lagg with mce0 and ixl0 caused the
interfaces' txcsum caps to be disabled.
Fixes: 4cc5d081d8
Reviewed by: glebius
Sponsored by: Netflix
MFC After: 7 days
The ktls buffer zone allocates 16k contiguous buffers, and often needs
to call vm_page_reclaim_contig_domain_ext() to free up contiguous
memory, which can be expensive. Web servers which have a daily
pattern of peaks and troughs end up having UMA trim the
ktls_buffer_zone when they are in their trough, and end up re-building
it on the way to their peak.
Rather than calling vm_page_reclaim_contig_domain_ext() multiple times
on a daily basis, lets mark the ktls_buffer_zone with a new UMA flag,
UMA_ZONE_NOTRIM. This disables UMA_RECLAIM_TRIM on the zone, but
allows UMA_RECLAIM_DRAIN* operations, so that if we become extremely
short of memory (vm_page_count_severe()), the uma reclaim worker can
still free up memory.
Note that UMA_ZONE_UNMANAGED already exists, but can never be drained
or trimmed, so it may hold on to memory during times of severe memory
pressure. Using UMA_ZONE_NOTRIM rather than UMA_ZONE_UNMANAGED is an
attempt to keep this zone more reactive in the face of severe memory
pressure.
Sponsored by: Netflix
Reviewed by: jhb, kib, markj (via slack)
Differential Revision: https://reviews.freebsd.org/D48451
Ensure all allocated tags have a hardware context associated.
The hardware context allocation is moved into the zone import
routine, as suggested by kib. This is safe because these zone
allocations are always done in a sleepable context.
I have removed the now pointless num_resources tracking,
and added sysctls / tunables to control UMA zone limits
for these tls tags, as well as a tunable to let the
driver pre-allocate tags at boot.
MFC after: 2 weeks
The jobid (which was stored in kernelinfo) was used to look up
jobs until 1ce9182407, where it became essentially write only.
Remove it to simplify the code and pave the way for future work
to make aio scale better.
Note this has been slated for removal "soon" for 18 years.
Suggested by: jhb
Reviewed by: kib
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D47583
We have been incorrectly choosing the "hlt" idle method on modern AMD
EPYC servers for C1 idle. This is because AMD also uses the Functional
Fixed Hardware interface. Due to not parsing the table properly for
AMD, and due to a weird quirk where the mwait latency for C1 is
mis-interpreted as the latency for hlt, we wind up choosing hlt for
c1, which has a far higher wake up latency (similar to IO) of roughly
400us on my test system (AMD 7502P).
This patch fixes this by:
- Looking for AMD in addition to Intel in the FFH
(Note the vendor id of "2" for AMD is not publically documented, but
AMD has confirmed they are using "2" and has promised to document it.)
- Using mwait on AMD when specified in the table, and when CPUid says
its supported
- Fixing a weird issue where we copy the contents of cx_ptr for C1 and
when moving to C2, we do not reinitialize cx_ptr. This leads to
mwait being selected, and ignoring the specified i/o halt method
unless we clear mwait before looking at the table for C2.
Differential Revision: https://reviews.freebsd.org/D47444
Reviewed by: dab, kib, vangyzen
Sponsored by: Netflix
Under massive connection thrashing (web server restarting), we see
long periods where the web server blocks when enabling ktls offload
when NIC ktls offload is enabled.
It turns out the driver uses a single-threaded linux work queue to
serialize the commands that must be sent to the nic to allocate and
free tls resources. When freeing sessions, this work is handled
asynchronously. However, when allocating sessions, the work is handled
synchronously and the driver waits for the work to complete before
returning. When under massive connection thrashing, the work queue is
first filled by TLS sessions closing. Then when new sessions arrive,
the web server enables kTLS and blocks while the tens or hundreds of
thousands of sessions closes queued up are processed by the NIC.
Rather than using the work queue to open a TLS session on the NIC,
switch to doing the open directly. This allows use to cut in front of
all those sessions that are waiting to close, and minimize the amount
of time the web server blocks. The risk is that the NIC may be out of
resources because it has not processed all of those session frees. So
if we fail to open a session directly, we fall back to using the work
queue.
Differential Revision: https://reviews.freebsd.org/D47260
Sponsored by: Netflix
Reviewed by: kib
When the kernel is compiled with options RATELIMIT, the
mlx5en driver cannot detach. It gets stuck waiting for all
kernel users of its rates to drop to zero before finally calling
ether_ifdetach.
The tcp ratelimit code has an eventhandler for ifnet departure
which causes rates to be released. However, this is called as an
ifnet departure eventhandler, which is invoked as part of
ifdetach(), via either_ifdetach(). This means that the tcp
ratelimit code holds down many hw rates when the mlx5en driver
is waiting for the rate count to go to 0. Thus devctl detach
will deadlock on mlx5 with this stack:
mi_switch+0xcf sleepq_timedwait+0x2f _sleep+0x1a3 pause_sbt+0x77 mlx5e_destroy_ifp+0xaf mlx5_remove_device+0xa7 mlx5_unregister_device+0x78 mlx5_unload_one+0x10a remove_one+0x1e linux_pci_detach_device+0x36 linux_pci_detach+0x24 device_detach+0x180 devctl2_ioctl+0x3dc devfs_ioctl+0xbb vn_ioctl+0xca devfs_ioctl_f+0x1e kern_ioctl+0x1c3 sys_ioctl+0x10a
To fix this, provide an explicit API for a driver to call the tcp
ratelimit code telling it to detach itself from an ifnet. This
allows the mlx5 driver to unload cleanly. I considered adding an
ifnet pre-departure eventhandler. However, that would need to be
invoked by the driver, so a simple function call seemed better.
The mlx5en driver has been updated to call this function.
Reviewed by: kib, rrs
Differential Revision: https://reviews.freebsd.org/D46221
Sponsored by: Netflix
In cd85379104, kib made maxphys a load-time tunable. This made
the #define MAXPHYS in sys/param.h almost entirely obsolete, as
it could now be overridden by kern.maxphys at boot time, or by
opt_maxphys.h.
However, decades of tradition have led to several new, incorrect, uses
of MAXPHYS in other parts of the kernel, mostly by seasoned
developers. I've corrected those uses here in a mechanical fashion,
and verified that it fixes a bug in the md driver that I was
experiencing.
Since using MAXPHYS is such an easy mistake to make, it is best to
hide it from the kernel namespace. So I've moved its definition to
_maxphys.h, which is now included in param.h only for userspace.
That brings up the fact that lots of userspace programs use MAXPHYS
for different reasons, most of them probably wrong. Userspace consumers
that really need to know the value of maxphys should probably be
changed to use the kern.maxphys sysctl. But that's outside the scope
of this change.
Reviewed by: imp, jkim, kib, markj
Fixes: 30038a8b4e ("md: Get rid of the pbuf zone")
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D44986
When profiling an IP6 heavy workload, I noticed that we were
getting a lot of cache misses in ip6_output() around
ip6_pktopts. This was happening because the TCP stack passes
inp->in6p_outputopts even if all options are unused. So in the
common case of no options present, pkt_opts is not null, and is
checked repeatedly for different options. Since ip6_pktopts is
large (4 cachelines), and every field is checked, we take 4
cache misses (2 of which tend to be hidden by the adjacent line
prefetcher).
To fix this common case, I introduced a new flag in ip6_pktopts
(ip6po_valid) which tracks which options have been set. In the
common case where nothing is set, this causes just a single
cache miss to load. It also eliminates a test for some options
(if (opt != NULL && opt->val >= const) vs if ((optvalid & flag) !=0 )
To keep the struct the same size in 64-bit kernels, and to keep
the integer values (like ip6po_hlim, ip6po_tclass, etc) on the
same cacheline, I moved them to the top.
As suggested by zlei, the null check in MAKE_EXTHDR() becomes
redundant, and can be removed.
For our web server workload (with the ip6po_tclass option set),
this drops the CPI from 2.9 to 2.4 for ip6_output
Differential Revision: https://reviews.freebsd.org/D44204
Reviewed by: bz, glebius, zlei
No Objection from: melifaro
Sponsored by: Netflix Inc.
Andrew Gallatin