Remove unused and easy to misuse PNP macro parameter
Inspired by r338025, just remove the element size parameter to the
MODULE_PNP_INFO macro entirely. The 'table' parameter is now required to
have correct pointer (or array) type. Since all invocations of the macro
already had this property and the emitted PNP data continues to include the
element size, there is no functional change.
Mostly done with the coccinelle 'spatch' tool:
$ cat modpnpsize0.cocci
@normaltables@
identifier b,c;
expression a,d,e;
declarer MODULE_PNP_INFO;
@@
MODULE_PNP_INFO(a,b,c,d,
-sizeof(d[0]),
e);
@singletons@
identifier b,c,d;
expression a;
declarer MODULE_PNP_INFO;
@@
MODULE_PNP_INFO(a,b,c,&d,
-sizeof(d),
1);
$ rg -l MODULE_PNP_INFO -- sys | \
xargs spatch --in-place --sp-file modpnpsize0.cocci
(Note that coccinelle invokes diff(1) via a PATH search and expects diff to
tolerate the -B flag, which BSD diff does not. So I had to link gdiff into
PATH as diff to use spatch.)
Tinderbox'd (-DMAKE_JUST_KERNELS).
Approved by: re (glen)
I was not aware Warner was making or planning to make forward progress in
this area and have since been informed of that.
It's easy to apply/reapply when churn dies down.
Inspired by r338025, just remove the element size parameter to the
MODULE_PNP_INFO macro entirely. The 'table' parameter is now required to
have correct pointer (or array) type. Since all invocations of the macro
already had this property and the emitted PNP data continues to include the
element size, there is no functional change.
Mostly done with the coccinelle 'spatch' tool:
$ cat modpnpsize0.cocci
@normaltables@
identifier b,c;
expression a,d,e;
declarer MODULE_PNP_INFO;
@@
MODULE_PNP_INFO(a,b,c,d,
-sizeof(d[0]),
e);
@singletons@
identifier b,c,d;
expression a;
declarer MODULE_PNP_INFO;
@@
MODULE_PNP_INFO(a,b,c,&d,
-sizeof(d),
1);
$ rg -l MODULE_PNP_INFO -- sys | \
xargs spatch --in-place --sp-file modpnpsize0.cocci
(Note that coccinelle invokes diff(1) via a PATH search and expects diff to
tolerate the -B flag, which BSD diff does not. So I had to link gdiff into
PATH as diff to use spatch.)
Tinderbox'd (-DMAKE_JUST_KERNELS).
Make unsigned values uint16_t for pnp table. They are properly
uint16_t befause they are 16-bit PCI IDs. The PNP_INFO language has no
type for bare unsigned.
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: https://github.com/bsdimp/freebsd/pull/5
Header length is optional hint for the ENA device. Because It is not
guaranteed that every packet header will be in the first mbuf
segment, it is better to skip passing any information. If the header
length will be indicating invalid value (different than 0), then the
packet will be dropped.
This kind situation can appear, when the UDP packet will be fragmented
by the stack in the ip_fragment() function.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reported by: Krishna Yenduri <kyenduri@brkt.com>
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
On AWS, a network interface can get reinitialized every 30 minutes due
to the MTU being (re)set when a new DHCP lease is obtained. This can
cause packet drop, along with annoying syslog messages.
Skip setting the MTU in the ena driver if the new MTU is the same as the
old MTU. Note this fix is already in the netfront driver.
Testing: Verified ena up/down messages do not appear every 30 min in
/var/log/messages with the fix in place.
Submitted by: Krishna Yenduri <kyenduri@brkt.com>
Reviewed by: Michal Krawczyk <mk@semihalf.com>
With the current state of the AENQ handlers in the ENA driver, only
implemented handlers should be indicated.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12872
Using only 1 descriptor on RX could be an issue, if system would be low
on resources and could not provide driver with large chunks of
contiguous memory.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12871
The device now provides driver with max available MTU value it
can handle.
The function setting MTU for the interface was simplified and reworked
to follow up this changes.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12870
The maximum number of io_cq was the same number as maximum io_sq
indicated by the device working in normal mode (without LLQ).
It is not always true, especially when LLQ is being enabled.
Fix it.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12869
The driver was printing out a lot of information upon failure, which
does not have to be interested for the user.
Changing logging level required to rebuild driver with proper flags. The
proper sysctl was added, so the level now can be changed dynamically
using bitmask.
Levels of printouts were adjusted to keep on mind end user instead of
debugging purposes.
More verbose messages were added to align the driver with the Linux.
Fix building error introduced by the r325506 by casting csum_flags to
uint64_t.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12868
This bug wasn't impacting anything, because both enums are indicating
the same value, but it could cause a problem on API change.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12867
The gcc compiler is more sensitive when variable is having an value
assigned, but it is not used anywhere further.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: rlibby
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12866
The previous way of checking for DF was not valid.
When DF is enabled, the DF bit should be 1.
The original way of checking it was wrong in 2 ways: first of all, it
was not checking for single bit, secondly, it was checking for 0.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12865
Remove unused macros and fields - some of them were only initialized,
without further usage.
Implement minor style fixes and add required comments.
On the occasion add missing TX completion counter, which was existing,
but mistakenly remained unused.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12864
The situation, where part of the MSI-x was not configured properly, was
not properly handled. Now, the driver reduces number of queues to
reflect number of existing and properly configured MSI-x vectors.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12863
Few counters were imported from the Linux driver and never used,
because of differences between the Linux and FreeBSD APIs.
Queue stops and resumes are no longer supported by the driver and
counters were incremented indicating false events.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: rlibby
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12862
The driver was using it in only few places, so the rest of the code
was covered with those statement.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: rlibby
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12861
* Change all conditional checks in "if" statement to boolean expressions
* Initialize variables with too complex values outside the declaration
* Fix indentations
* Move code associated with sysctls to ena_sysctl.c file
* For consistency, remove unnecesary "return" from void functions
* Use if_getdrvflags() function instead of accesing variable directly
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12860
Some goto tags were renamed for consistency, and few error handling
routines were reworked.
The drbr_free() must be locked just in case code will change in the
future - for now, it should never be an issue, because drbr is being
flushed in the ena_down() call, and the lock is required only when there
are some mbufs inside.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12859
On heavy load, when interrupt handling routine was slowed down, there
could appear memory corruption, because resources were destroyed and
interrupt was still being handled.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12858
Pure cosmetic change for better readability of the driver.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12857
In case when Rx ring is full and driver will fail to allocate Rx mbufs,
the ring could be stalled.
Keep alive is checking every second for Rx ring state, and if it is full
for two cycles, then trigger rx_cleanup routine in another thread.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12856
The RX out of order completion feature, allows to complete RX
descriptors out of order, by keeping trace of all free descriptors in
the separate array.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12855
The function is in hot path of the driver (TX) and asking compiler for
making this function inline was changed for consistency and higher
readability.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: rlibby, byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon, Inc.
Differential Revision: https://reviews.freebsd.org/D12854
The patch contains following changes:
* In conditional checks, always check for NULL or 0 instead of negating values
* Use malloc and free explicitely, instead of ENA_MEM_FREE and ENA_MEM_FREE (the
dmadev passed to macro is never used, and could be a little misleading)
* Always check for NULL after calling malloc (few checks were missing)
* Rework naming of the goto tags in ena_attach() for consistency
* Fix error handling in ena_attach() - few goto instructions were leading to the
wrong tag
* Destroy MMIO req read request if attach failed
* Remove checking for NULL after calling malloc with M_WAITOK flag
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: byenduri_gmail.com
Obtained from: Semihalf
Sponsored by: Amazon.com, Inc.
Differential Revision: https://reviews.freebsd.org/D12853
Do not read all statistics from the device, instead count them in the
driver except from RX drops - they are received directly from the NIC
in the AENQ descriptor.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Reviewed by: imp
Obtained from: Semihalf
Sponsored by: Amazon.com, Inc.
Differential Revision: https://reviews.freebsd.org/D12852
The newest ena-com HAL supports LLQv2 and introduces
API changes. In order not to break the driver compilation
it was updated/fixed in a following way:
* Change version of the driver to 0.8.0
* Provide reset cause when triggering reset of the device
* Reset device after attach fails
* In the reset task free management irq after calling ena_down. Admin
queue can still be used before ena_down is called, or when it is
being handled
* Do not reset device if ena_reset_task fails
* Move call of the ena_com_dev_reset to the ena_down() routine - it
should be called only if interface was up
* Use different function for checking empty space on the sq ring
(ena-com API change)
* Fix typo on ENA_TX_CLEANUP_THRESHOLD
* Change checking for EPERM with EOPNOTSUPP - change in the ena-com API
* Minor style fixes
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Amazon.com, Inc.
Semihalf
Sponsored by: Amazon.com, Inc.
Differential Revision: https://reviews.freebsd.org/D12143
Collapse should be more effective than defragmentation.
Added missing declaration of ena_check_and_collapse_mbuf().
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
TSO settings were not reflecting real HW capabilities.
DMA tags were created with wrong window - high address was the same as
low, so excluding window was not working.
Capabilities of TX dma transaction were not set properly - TSO max size
had been increased and size of one segment had been adjusted.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
RX lock is no longer required. There can only be one RX cleanup task
running at a time, RX cleanup cannot be executed if interface is not
yet initialized and ena_down() will not free any RX resources if any io
interrupt is being handled - RX cleanup task is only called from an
interrupt handler.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
If drbr_advance() is not called before doing cleanup and packet is
already enqueued for sending (tx_info is holding pointer to mbuf), then
mbuf is cleaned both in drbr_flush() and in cleanup routine, when all
mbufs hold by tx_buffer_info are being released.
This causes panic, because mbuf is released twice.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
If driver left MSI-x handlling routine because interface was put down,
it is not unmasking IRQs, so any requesting interrupt will be awaiting
for unmasking.
On ena_up() routine all interrupts are being unmasked and any awaiting
interrupt will be handled right away.
If handler was executed before driver state was set as running, handling
routine is being ended immediately, leaving IO irqs for given queue
masked.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
It is required to hold lock that is associated with buffer ring before
flushing drbr.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
Lack of this lock was causing crash if down was called in
parallel with the initialization routine.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
There could be race condition with TX cleaning routine when cleaning mbufs,
when it was called directly from main sending thread (ena_mq_start).
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
Differential revision: https://reviews.freebsd.org/D10927
Initially, stats were being updated each time OS was requesting for
the first statistic.
To read statistics from hw, condvar was used. cv_timedwait cannot be
called when unsleepable lock is held, and this happens when FreeBSD
is requesting statistic.
Seperate task is reading statistics from NIC each 1 second.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
Differential revision: https://reviews.freebsd.org/D10926
Also, to simplify cleaning routine, reset task is initialized before
allocating statistics and other resources.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
Differential revision: https://reviews.freebsd.org/D10925
Lock only ena_up and ena_down calls in ioctl handler, instead of whole
ioctl. Locking ioctl with sx lock that is sleepable, is not allowed in
some cases, e.g. when multicast options are being changed.
Additional locking was added in deatch function to prevent race condition
with ioctl function.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
Differential revision: https://reviews.freebsd.org/D10924
When mbuf chain is too long and device cannot handle that number
of segments in DMA transaction, mbuf chain will be defragmented.
Initially, driver was dropping all mbuf chains that were exceeding
supported number of segments.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
Differential revision: https://reviews.freebsd.org/D10923
ENA is a networking interface designed to make good use of modern CPU
features and system architectures.
The ENA device exposes a lightweight management interface with a
minimal set of memory mapped registers and extendable command set
through an Admin Queue.
The driver supports a range of ENA devices, is link-speed independent
(i.e., the same driver is used for 10GbE, 25GbE, 40GbE, etc.), and has
a negotiated and extendable feature set.
Some ENA devices support SR-IOV. This driver is used for both the
SR-IOV Physical Function (PF) and Virtual Function (VF) devices.
ENA devices enable high speed and low overhead network traffic
processing by providing multiple Tx/Rx queue pairs (the maximum number
is advertised by the device via the Admin Queue), a dedicated MSI-X
interrupt vector per Tx/Rx queue pair, and CPU cacheline optimized
data placement.
The ENA driver supports industry standard TCP/IP offload features such
as checksum offload and TCP transmit segmentation offload (TSO).
Receive-side scaling (RSS) is supported for multi-core scaling.
The ENA driver and its corresponding devices implement health
monitoring mechanisms such as watchdog, enabling the device and driver
to recover in a manner transparent to the application, as well as
debug logs.
Some of the ENA devices support a working mode called Low-latency
Queue (LLQ), which saves several more microseconds. This feature will
be implemented for driver in future releases.
Submitted by: Michal Krawczyk <mk@semihalf.com>
Jakub Palider <jpa@semihalf.com>
Jan Medala <jan@semihalf.com>
Obtained from: Semihalf
Sponsored by: Amazon.com Inc.
Differential revision: https://reviews.freebsd.org/D10427
