Bug Fixes:
* Initialize statistics before the interface is available
* Fix driver unload crash
Minor Changes:
* Mechanically convert ena(4) to DrvAPI
* Remove usage of IFF_KNOWSEPOCH
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
When ena_detach is called, we first call ether_ifdetach(),
which destroys internal addresses of ifp. One such address
is ifp->if_addr->ifa_addr. Then during ena_destroy_device(),
if_link_state_change() is called, eventually trying to access
ifp->if_addr->ifa_addr->sa_family. This causes an access
to garbage memory and crashes the kernel.
Ticket [1] was opened to the FreeBSD community to add null
check in the code of if_link_state_change().
A fix was submitted in commit [2], however it was noted
that it is our driver's responsibilty to not call
if_link_state_change() after calling ether_ifdetach().
This commit makes sure if_link_state_change() is not called
after ether_ifdetach().
[1]: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=270813
[2]: https://reviews.freebsd.org/D39614
Fixes: 32f63fa7f9 ("Split ENA reset routine into restore and destroy stages")
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
In [1], the FBSD community exposed a bug in the fbsd/ena driver.
Bug description:
----------------
Current function call order is as follows:
1. ena_attach()
1.1. ena_setup_ifnet()
1.1.1. Registration of ena_get_counter()
1.1.2. ether_ifattach(ifp, adapter->mac_addr);
1.2. Statistics allocation and initialization.
At point 1.1.2, when ether_ifattach() returns, the interface is available,
and stats can be read before they are allocated, leading to kernel panic.
Also fixed a potential memory leak by freeing the stats since they were
not freed in case the following calls failed.
Fix:
----
This commit moves the statistics allocation and initialization to happen
before ena_setup_ifnet()
[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=268934
Fixes: 9b8d05b8ac ("Add support for Amazon Elastic Network Adapter (ENA) NIC")
Fixes: 30217e2dff ("Rework counting of hardware statistics in ENA driver")
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Expect that drivers call into the network stack with the net epoch
entered. This has already been the fact since early 2020. The net
interrupts, that are marked with INTR_TYPE_NET, were entering epoch
since 511d1afb6b. For the taskqueues there is NET_TASK_INIT() and
all drivers that were known back in 2020 we marked with it in
6c3e93cb5a. However in e87c494015 we took conservative approach
and preferred to opt-in rather than opt-out for the epoch.
This change not only reverts e87c494015 but adds a safety belt to
avoid panicing with INVARIANTS if there is a missed driver. With
INVARIANTS we will run in_epoch() check, print a warning and enter
the net epoch. A driver that prints can be quickly fixed with the
IFF_NEEDSEPOCH flag, but better be augmented to properly enter the
epoch itself.
Note on TCP LRO: it is a backdoor to enter the TCP stack bypassing
some layers of net stack, ignoring either old IFF_KNOWSEPOCH or the
new IFF_NEEDSEPOCH. But the tcp_lro_flush_all() asserts the presence
of network epoch. Indeed, all NIC drivers that support LRO already
provide the epoch, either with help of INTR_TYPE_NET or just running
NET_EPOCH_ENTER() in their code.
Reviewed by: zlei, gallatin, erj
Differential Revision: https://reviews.freebsd.org/D39510
In case the reset sequence fails (ena_destroy_device() followed by
ena_restore_device() calls) during ena_restore_device(), the driver
resources are being freed. After the clean-up, the timer service is
re-armed in order to try and re-initialize the driver state.
But, such an attempt would fail given that the resources are freed.
Moreover, this would actually cause either the system to fail or a
panic.
When the driver fails in ena_restore_device() procedure, the only
recovery is either unloading and loading the driver or instance
reboot.
This change removes the timer service re-arm in case of failure
in ena_restore_device().
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Fixes: 78554d0c70 ("ena: start timer service on attach")
Commit [1] first added the ena_tx_buffer.print_once member,
so that a message about a missing tx completion is printed only
once per packet (and not every second when the watchdog runs).
In this commit print_once is initialized to true, and is set back
to false after detecting a missing tx completion and printing
a warning about it to dmesg.
Commit [2] incorrectly reverses the values assigned to print_once.
The variable is initialized to be true but is checked to be false
when a missing tx completion is detected. This is never true, and
therefore the warning print for each missing tx completion is never
printed since this commit.
Commit [3] added time passed since last TX cleanup to the missing
tx completions per-packet print. However, due to the issue in commit
[2], this time is never printed.
This commit reverses back the values assigned to ena_tx_buffer.print_once
erroneously by commit [2], bringing back to life the missing tx
completion per-packet print.
Also add a space after "." in the missing tx completion print.
[1] - 9b8d05b8ac ("Add support for Amazon Elastic Network Adapter (ENA) NIC")
[2] - 74dba3ad78 ("Split function checking for missing TX completion in ENA driver")
[3] - d8aba82b5c ("ena: Store ticks of last Tx cleanup")
Fixes: 74dba3ad78 ("Split function checking for missing TX completion in ENA driver")
Fixes: d8aba82b5c ("ena: Store ticks of last Tx cleanup")
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Minor version update which improves styling of a printouts, fixes
the KASAN and KMSAN kernel builds and LLQ reconfiguration after the
device reset.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
After the device reset, the LLQ configuration descriptor wasn't passed
to the hardware. On a 6-generation AWS instances (like C6gn), it is
required to pass the LLQ descriptor after the device reset, otherwise
the hardware will be missing the LLQ configuration resulting in
performance degradation.
This patch reconfigures the LLQ each time the ena_device_init() is
called. This means that the LLQ descriptor will be passed during the
initial configuration and after a reset.
The ena_map_llq_mem_bar() function call was moved before the
ena_device_init() call, to make sure that the mem bar is available.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
In most places, the req_id is printed first, and the qid is printed as a
second. To align the driver, one printout was reworked and the print
order of those variables was changed.
Suggested by: rpokala
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
We do not have atomic(9) routines for bools, and it is not guaranteed
that sizeof(bool) is 1.
This fixes the KASAN and KMSAN kernel builds, which fail because the
compiler refuses to silently cast a _Bool * to a uint8_t * when calling
the atomic(9) sanitizer interceptors.
Reviewed by: Dawid Górecki <dgr@semihalf.com>
MFC after: 2 weeks
Fixes: 0ac122c388 ("ena: Use atomic_load/store functions for first_interrupt variable")
Differential Revision: https://reviews.freebsd.org/D35683
Some of the changes in this release:
* Style fixes
* Fix ENI stats probing
* Add trace for the last Tx cleanup call
* Prevent LLQ initialization if member isn't exposed
* Improve logging
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
The KASSERT was originally added to ensure that the netmap Rx ring is
not NULL, however, it was checking for the opposite.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Most of the constants in ena.h file were prefixed with ENA_*, while
others did not have this prefix. Align the constants by prefixing the
remaining constants with ENA.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
The ena_qid variable value is never used. It can be safely removed.
That also silences the compilation warning.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
During probe the driver created a temporary buffer to which the value of
DEVICE_DESC constant was printed. This buffer was then copied to the
device structure using device_set_desc_copy. Since the value of this
string is exactly the same for every device using the ENA driver, using
sprintf is unnecessary, and device_set_desc can be used instead.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Copying ENI metrics was done in callout context, this caused the driver
to panic when sample_interval was set to a value other than 0, as the
admin queue call which was executed could sleep while waiting on
a condition variable. Taskqueue, unlike callout, allows for sleeping, so
moving the function to a separate taskqueue fixes the problem.
ena_timer_service is still responsible for scheduling the taskqueue.
Stop draining the callout during ena_up/ena_down. This was done to
prevent a race between ena_up/down and ena_copy_eni_metrics admin queue
calls. Since ena_metrics_task is protected by ENA_LOCK there is no
possibility of a race between ena_up/down and ena_metrics_task.
Remove a comment about locking in ena_timer_service. With ENI metrics
in a separate task this comment became obsolete.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Surround cases of possible simultaneous access to the first_interrupt
variable with atomic_load/store functions.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Store timestamp of last cleanup in Tx ring structure. This does not
change anything during normal operation of the driver but could be
useful when the device fails for some reason.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
The ena_com_config_dev_mode() function performs many LLQ related
calculations and sends an admin command to configure LLQ in the device.
All the LLQ related operations are unnecessary if the driver fails to
find LLQ memory bar.
Move LLQ memory bar allocation to separate helper function
ena_map_llq_mem_bar and execute this function before LLQ configuration.
If the LLQ memory bar cannot be allocated, then LLQ configuration is
skipped.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Print information about qid if req_id is invalid. Add information about
qid and req_id if mbuf is invalid.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
While ena_restore_device is called from the reset task, it can also be
called from other locations in the driver, for example in netmap
specific code. Move the reset completion logging to reset task, so it
better represents when the reset actually happened.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Some of the changes in this release:
- IPv6 L4 checksum offload fixes.
- Optimization of the Tx req_id validation.
- Timer service adjustments.
- NUMA awareness for the kernel RSS mode.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
If the device becomes unresponsive, the driver will not be able to
finish the reset process correctly. Timeout during version validation
indicates that the device is currently not responding. In that case
do not perform the reset and instead reschedule timer service. Because
of that the driver will continue trying to reset the device until it
succeeds or is detached.
Submitted by: Dawid Gorecki <dgr@semihalf.com>
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
The timer service was started when the interface was brought up and it
was stopped when it was brought down. Since ena_up requires the device
to be responsive, triggering the reset would become impossible if the
device became unresponsive with the interface down.
Since most of the functions in timer service already perform the check
to see if the device is running, this only requires starting the callout
in attach and stopping it when bringing the interface up or down to
avoid race between different admin queue calls.
Since callout functions for timer service are always called with the
same arguments, replace callout_{init,reset,drain} calls with
ENA_TIMER_{INIT,RESET,DRAIN} macros.
Submitted by: Dawid Gorecki <dgr@semihalf.com>
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Since `ena_com_tx_comp_req_id_get` already checks for `req_id` validity,
the logic was exiting early, never giving `validate_tx_req_id` a chance
to trigger device reset.
Rewrite the logic so that device reset is called based on return value
of `ena_com_tx_comp_req_id_get` instead.
Submitted by: Artur Rojek <ar@semihalf.com>
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
ena_tx_csum function did not check if IPv6 checksum offload was
requested it only checked checksum offloading flags for IPv4 packets.
Because of that, when encountering CSUM_IP6_* flags, the function simply
returned without actually setting checksum offloading in ena_ctx.
Check CUSM_IP6_* flags to enable IPv6 checksum offload.
Additionally, only IPv4 header was being parsed regardless of EtherType
field, because of that, value of L4 protocol read when actually trying
to send IPv6 packets was wrong. Use ip6_lasthdr function to get length
of all IPv6 headers and payload protocol.
Set the DF flag to 1 in order to allow the device to offload the IPv6
checksum calculation and achieve optimal performance.
Add CSUM6_OFFLOAD and CSUM_OFFLOAD definitions into ena_datapath.h.
Submitted by: Dawid Gorecki <dgr@semihalf.com>
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Merge commit '2530eb1fa01bf28fbcfcdda58bd41e055dcb2e4a'
Adjust the driver to the upgraded ena-com part twofold:
First update is related to the driver's NUMA awareness.
Allocate I/O queue memory in NUMA domain local to the CPU bound to the
given queue, improving data access time. Since this can result in
performance hit for unaware users, this is done only when RSS
option is enabled, for other cases the driver relies on kernel to
allocate memory by itself.
Information about first CPU bound is saved in adapter structure, so
the binding persists after bringing the interface down and up again.
If there are more buckets than interface queues, the driver will try to
bind different interfaces to different CPUs using round-robin algorithm
(but it will not bind queues to CPUs which do not have any RSS buckets
associated with them). This is done to better utilize hardware
resources by spreading the load.
Add (read-only) per-queue sysctls in order to provide the following
information:
- queueN.domain: NUMA domain associated with the queue
- queueN.cpu: CPU affinity of the queue
The second change is for the CSUM_OFFLOAD constant, as ENA platform
file has removed its definition. To align to that change, it has been
added to the ena_datapath.h file.
Submitted by: Artur Rojek <ar@semihalf.com>
Submitted by: Dawid Gorecki <dgr@semihalf.com>
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
When building ENA as compiled into the kernel, the driver would fail to
build. Resolve the problem by introducing the following changes:
1. Add missing `ena_rss.c` entry in `sys/conf/files`.
2. Prevent SYSCTL_ADD_INT from throwing an assert due to an extra
CTLTYPE_INT flag.
Fixes: 986e7b9227 ("ena: Move RSS logic into its own source files")
Fixes: 6d1ef2abd3 ("ena: Implement full RSS reconfiguration")
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
MFC after: 1 week
Some of the changes in this release:
* Hardware RSS hash key reconfiguration and indirection table
reconfiguration support.
* Full kernel RSS support.
* Extra statistic counters.
* Netmap support for ENAv3.
* Locking assertions.
* Extra log messages.
* Reset handling fixes.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Bind RX/TX queues and MSI-X vectors to matching CPUs based on the RSS
bucket entries.
Introduce sysctls for the following RSS functionality:
- rss.indir_table: indirection table mapping
- rss.indir_table_size: indirection table size
- rss.key: RSS hash key (if Toeplitz used)
Said sysctls are only available when compiled without `option RSS`, as
kernel-side RSS support currently doesn't offer RSS reconfiguration.
Migrate the hash algorithm from CRC32 to Toeplitz and change the initial
hash value to 0x0 in order to match the standard Toeplitz implementation.
Provide helpers for hash key inversion required for HW operations.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Provide the following sysctl statistics in order to stay aligned with
the Linux driver:
* rx_ring.csum_good
* tx_ring.unmask_interrupt_num
Also rename the 'bad_csum' statistic name to 'csum_bad' for alignment.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
In order to use `ena_global_lock` in sysctl context, it must be kept
outside the driver instance's software context, as sysctls can be called
before attach and after detach, leading to lock use before sx_init and
after sx_destroy otherwise.
Solve this issue by turning `ena_global_lock` into a file scope
variable, shared between all instances of the driver and associated
sysctl context, and in turn initialized/destroyed in dedicated
SYSINIT/SYSUNINIT functions.
As a side effect, this change also fixes existing race in the reset
routine, when simultaneously accessing sysctl exposed properties.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
If LLQ is being used, `ena_tx_ctx.meta_valid` must stay enabled. This
fixes netmap support on latest generation ENA HW and aligns it with the
core driver behavior.
As netmap doesn't support any csum offloads, the
`adapter->disable_meta_caching` value can be simply passed to the HW.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Delegate RSS related functionality into separate .c/.h files in
preparation for the full RSS support.
While at it, reorder functions and remove prototypes for ones with
internal linkage.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
ENA silently assumed that ena_up, ena_down and ena_start_xmit routines
should be called within locked context. Driver's logic heavily assumes
on concurrent access to those routines, so for safety and better
documentation about this assumption, the locking assertions were added
to the above functions.
The assertion was added only for the main steps (skipping the helper
functions) which can be called from multiple places including the kernel
and the driver itself.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Stay aligned with the Linux driver by adding the following logs:
* inform the user about retrying queue creation
* warn on non-empty ena_tx_buffer.mbuf prior to ena_tx_map_mbuf
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Check for ENA_FLAG_TRIGGER_RESET inside a locked context in order to
avoid potential race conditions with ena_destroy_device. This aligns the
reset task logic with the Linux driver.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
All ena_com_prepare_tx errors other than ENA_COM_NO_MEM are fatal and
require device reset.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
In case of Low-latency Queue, one small enough descriptor can be pushed
directly to the ENA hw, thus saving one fragment. Check for this
condition before performing collapse.
Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
