Driver requests are done with stack allocated request. The request is
put on the tailq and then we msleep(9) until kernel process processes it.
If we timeout from this sleep, the kernel process may still read the
request from our stack, which may already be reused by some other code.
Make this sleep unbound and rely on the kernel process that does all its
I/O with timouts and will eventually wake us up.
Reviewed by: jhb
Differential Revision: https://reviews.freebsd.org/D47179
Both conditions are the same, so the second one is unreachable.
PR: 229550
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: andreast, markj
Differential Revision: https://reviews.freebsd.org/D47167
No functional change intended.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: markj, emaste
Differential Revision: https://reviews.freebsd.org/D47152
These malloc(9) calls are in contexts where sleeping is permitted.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, zlei, markj, emaste
Differential Revision: https://reviews.freebsd.org/D46845
The current channel naming convention is:
pcmX:[virtual_]play|record:dspX.[v]p|rY
- pcmX and dspX share the same unit numbers
- "[v]p|r" gives us the same information as "[virtual_]play|record"
Remove the redundant information, so that the channel names become more
readable: dspX.[virtual_]play|record.Y
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D46836
Currently we create and destroy channels with the following consistent
pattern:
- chn_init() -> pcm_chn_add()
- pcm_chn_remove() -> chn_kill()
Instead of calling two separate functions, merge pcm_chn_add() with
chn_init(), and pcm_chn_remove() with chn_kill().
Another benefit of this change is that we avoid the confusion caused by
having pcm_chn_add(), as well as pcm_addchan().
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj
Differential Revision: https://reviews.freebsd.org/D46835
feeder_rate_min functions as the lower boundary.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj, emaste
Differential Revision: https://reviews.freebsd.org/D46834
Instead of checking the value of "ret" multiple times, just set a goto
label and jump there immediately in case of an error.
While here, remove a redundant assignment to "d".
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj, emaste
Differential Revision: https://reviews.freebsd.org/D46833
No longer used.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D46522
If we do not enter dummy_chan_trigger() before detaching, we'll get a
use-after-free since the callout(9) callback might be called after
having been detached.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj, emaste
Differential Revision: https://reviews.freebsd.org/D46715
feeder_register() is currently a SYSINIT in order to create the root
feeder, which happens only when feedercnt is 0. Separating the root
feeder registration makes the code more readable.
No functional change intended.
While here, fix some style errors.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, zlei, markj
Differential Revision: https://reviews.freebsd.org/D46821
There is no reason to initialize global variables in feeder_register(),
as these variables are unrelated to what the function does. Instead,
initialize them during sound(4) load.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: zlei
Differential Revision: https://reviews.freebsd.org/D46749
Currently it is possible to assign a unit number that already exists.
Suppose the following channel list:
[vp2]
If we create 3 channels, we'll end up with the following list:
[vp0, vp1, vp2, vp2]
This happens because chn_init() does not check if the unit number we are
assigning already exists. While fixing this is trivial when the channel
list is sorted in ascending order, it is way more involved when sorted
in descending order. Even though sorting the list in descending order
would require deliberately modifying pcm_chn_add(), and is most likely
not going to happen, make the mechanism more robust by using a unr(9)
allocator for each channel type.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D46680
Refactor CHN_INSERT_SORT() to also take into account the unit number of
each channel, while still prioritizing the type, to make the channel
list easier to work with.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D46549
Simplify logic to execute the following algorithm:
1. Search for a free (i.e not busy) channel and return successfully.
2. If no channel was found, either return an ENOTSUP if no VCHAN can be
created (disabled or we reached the limit), or create one more VCHAN
and scan for it again.
3. If the second scan failed again, return EBUSY.
This patch also solves a bug where we'd end up in an infinite loop,
calling vchan_setnew() with the same `newcnt` value indefinitely, after
the following scenario:
1. Plug device.
2. Spawn X channels.
3. Kill all channels except _last_ opened.
4. Clean up all unused VCHANs.
5. Spawn X+1 channels.
6. Infinite loop in pcm_chnalloc().
I am not exactly sure which part of pcm_chnalloc() caused this bug, but
the patch fixes it at least...
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D46548
The VCHAN count is checked in vchan_setnew(), and there is no reason to
cap the hardware channels in the first place.
This is part of a series of follow-up patches.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D46521
This check is not really useful, and can in fact break things, if
sysctl_dev_pcm_vchans() calls vchan_setnew() with a value that will not
satisfy the KASSERT condition.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, emaste
Differential Revision: https://reviews.freebsd.org/D46545
A hardware IPv6 server needs 2 consecutive stids (server tids) starting
from a 2-aligned stid whereas an IPv4 server needs only 1 stid without
any constraint. The allocator used to grab the first free stid(s) for
both but this can fragment the stid space leaving nothing suitable for
IPv6 even when lots of stids are available. Change the allocator to
prefer stids for IPv4 from the ones that cannot be used for IPv6.
Reviewed by: jhb
MFC after: 1 week
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D47042
Previously this just dereferenced NULL qp pointers and panicked.
Instead, use a shared lock on the connection lock to protect access to
the qp pointers and allocate a request. If the controller is not
associated, fail the request with ECONNABORTED.
Possibly this should be honoring kern.nvmf.fail_on_disconnection and
block waiting for a reconnect request while disconnected if that
tunable is false.
Reported by: Suhas Lokesha <suhas@chelsio.com>
Sponsored by: Chelsio Communications
The hash filter table order in the GMAC matches the order of the top
bit of the hashed destination address. See the description of
GMAC_MAC_Hash_table_Reg0 in RK3568 TRM part 2, section 20.4.2.
PR: 282074
Reviewed by: manu
MFC after: 7 days
Differential Revision: https://reviews.freebsd.org/D47115
Add a sysctl to artificially fail the reset to test the failure to reset
hardware code path. While there are many ways that reset can fail, this
provides an adequate way that similates enough of the failures well
enough to shake out this failure path.
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D28584
ifmedia_add() allocates an ifmedia_entry during ena_attach.
Current code doesn't release this memory during ena_detach()
This commit calls ifmedia_removeall() to properly free the
allocated memory during ena_detach().
Also, in case ena_attach fails, we need to detach ifmedia
which was allocated within ena_setup_ifnet().
This bug was first described in:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278100
Reviewed by: zlei
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Large LLQ depth size is currently calculated by dividing the maximum
possible size of LLQ by 2.
In newer paltforms, starting from r8g the size of BAR2,
which contains LLQ, will be increased, and the maximum depth of
wide LLQ will be set according to a value set by the device, instead of
hardcoded division by 2.
The new value will be stored by the device in max_wide_llq_depth field
for drivers that expose ENA_ADMIN_LLQ_FEATURE_VERSION_1 or higher to
the device.
There is an assumption that max_llq_depth >= max_wide_llq_depth, since
they both use the same bar, and if it is possible to have a wide LLQ
of size max_wide_llq_depth, it is possible to have a normal LLQ of the
same size, since it will occupy half of the space.
Also moved the large LLQ case calculation of max_tx_queue_size
before its rounddown.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit adds support for receiving LLQ entry size recommendation
from the device. The driver will use the recommended entry size, unless
the user specifically chooses to use regular or large LLQ entry.
Also added enum ena_llq_header_size_policy_t and llq_plociy field in
order to support the new feature.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit adds a handler for the new aenq message
ENA_ADMIN_DEVICE_REQUEST_RESET,
which in turn causes the driver to trigger reset of a new type:
ENA_REGS_RESET_DEVICE_REQUEST. Also adds counting of such occurrences in
a new statistic for it.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
When attaching ENA driver, ena_netmap_attach() is invoked which, in turn
calls netmap_attach which, initializes a struct netmap_adapter,
allocating the struct's netmap_ring and the struct selinfo.
When we change the interface number of queues we need to reinit the
netmap adapter struct as well, so we need to detach it in order to free
the memory allocated by netmap_attach and allocate new memory based on
the new parameters like number of rings, ring size etc...
Without detaching and attaching the netmap interface, if we're to change
the number of queues from 8 to 2 for example and try to enable netmap,
the kernel will panic since the original netmap struct within the
kernel's possession still thinks that the driver has 8 queues which will
eventually cause a non-allocated virtual address access fault.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
When processing packets within the rx-flow
ena_netmap_rx_load_desc doesn't know the number of descriptors, so it
sets NS_MOREFRAG to all the slots to indicate that there are more
fragments for this packet.
The code calls ena_netmap_rx_load_desc() for every descriptor in
this packet to map the relevant buffer into the netmap shared memory.
After ena_netmap_rx_load_desc() calls, we need to unset the NS_MOREFRAG
for the last fragment to indicate that this is the last fragment,
so we explicitly turn off NS_MOREFRAG flag.
Current code overrides all other flags and sets NS_BUF_CHANGED.
This patch unsets the relevant flag only.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Netmap index wraps around based on the number of netmap kernel ring
slots.
Currently the driver prefetches the next slot using nm_i + 1 which may
be wrong since it does not handle wrap around.
This patch fixes that by using the kernel API for fetching the next
netmap index.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
In case ena_com_prepare_tx() fails within the netmap tx flow,
the driver will unmap the last socket chain.
Currently, the driver unmaps the wrong socket within
ena_netmap_unmap_last_socket_chain().
Illustration of the flow:
1- ena_netmap_tx_frames()
2- ena_netmap_tx_frame()
3- ena_netmap_tx_map_slots()
3.1- Map slot
3.2- Advance to the next socket
4- ena_com_prepare_tx()
4.1- ena_com_prepare_tx() fails
5- ena_netmap_unmap_last_socket_chain()
In step 5, where the driver unmaps the socket, the netmap
index already points at the next entry, meaning we're unmapping the
wrong socket in case ena_com_prepare_tx() fails.
In order to fix that, the driver should first update the netmap index to
point at the previous entry and only then update the socket parameters.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit changes the code so all global counters will have the
same line break.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
The mbuf is NULL issue happens when the device sends the driver
a completion with a wrong request id.
Trigger a reset whenever this happens.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit adds differentiation for a reset caused by missing tx
completions, by verifying if the driver didn't receive tx
completions caused by missing interrupts.
The cleanup_running field was added to ena_ring because
cleanup_task.ta_pending is zeroed before ena_cleanup() runs.
Also ena_increment_reset_counter() API was added in order to support
only incrementing the reset counter.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit sets the default value for ena_min_poll_delay_us to 100.
This commit does not change the behavior of the driver, the delay is
calculated as MAX(ENA_MIN_ADMIN_POLL_US, delay_us), where the first
field is already defined as 100.
The second parameter, delay_us is taken from ena_min_poll_delay_us
which is currently unset - 0.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
There can be cases when we trigger reset if an admin interrupt
is missing.
In order to identify this use-case specifically,
this commit adds a new reset reason.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
RX completion descriptors may sometimes contain errors due
to corruption. Upon identifying such a case, the driver will
trigger a reset with an explicit reset reason
ENA_REGS_RESET_RX_DESCRIPTOR_MALFORMED.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
TX completion descriptors may sometimes contain errors due
to corruption. Upon identifying such a case, the driver will
trigger a reset with an explicit reset reason
ENA_REGS_RESET_TX_DESCRIPTOR_MALFORMED.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
The driver uses different reset reasons.
Some of them are counted and presented in the driver statistics.
There are cases where statistics are counted on a ring level,
but these are zeroed after a reset procedure takes place.
This commit makes the following changes:
1. Add statistics for the unrepresented reset reasons.
2. Add reset reasons which are counted on a ring level,
to be also global for better tracking.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit is part of the effort of notifying the user of non-optimal
or performance impacting practices.
A new interface is serving as a communication channel
between the device and the driver. One of the goals of this channel is
to create a new mechanism of notifying the driver and user in case of
sub-optimal configuration using a bitmap.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Currently we count all of the newly added and already existing
missing tx completions in each iteration of
check_missing_comp_in_tx_queue() causing duplicate counts
to missing_tx_comp stat.
This commit adds a new counter new_missed_tx within the relevant
function which only counts the newly added missing tx completions
in each iteration of check_missing_comp_in_tx_queue().
This will allow us to update missing_tx_comp stat accurately without
counting duplicates.
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
Upstream commit [1] made if_alloc_domain() never fail, then also do the
wrappers if_alloc(), if_alloc_dev(), and if_gethandle().
Upstream commit [2] removed the NULL check conducted by the driver.
This commit also removes err_customer_metrics_alloc goto label.
Commit [2] leaves behind a floating free() statement that
deallocates customer_metrics_array. This commit places the
deallocation statement where it belongs.
[1] commit 4787572d05 ("ifnet: make if_alloc_domain() never fail")
[2] commit aa3860851b ("net: Remove unneeded NULL check for the allocated ifnet")
Approved by: cperciva (mentor)
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
The Arm True Random Number Generator Firmware Interface provides a way
to query the SMCCC firmware for up to 192 bits of entropy. Use it to
provide another source of randomness to the kernel.
Reviewed by: cem, markm
Sponsored by: Arm Ltd
Differential Revision: https://reviews.freebsd.org/D46989
