iflib: invert default restart on VLAN changes

In rS360398, a new iflib device method was added to opt out of VLAN
events needing an interface reset.

I am switching the default to not requiring a re

iflib: invert default restart on VLAN changes

In rS360398, a new iflib device method was added to opt out of VLAN
events needing an interface reset.

I am switching the default to not requiring a restart for:
* VLAN events
* unknown events

After fixing various bugs, I do not think this would be a common need
of hardware and it is undesirable from the user's perspective causing
link flaps and much slower VLAN configuration. Currently, there are no
other restart events besides VLAN events, and setting the
ifdi_needs_restart default to false will alleviate the need to churn
every driver if an odd event is added in the future for specific
hardware.

markj points out this could cause churn in the other direction; I will
solve that problem with an event registration system as he mentions in
the review should we need it in the future.

These drivers will opt into restart and need further inspection or work:
* ixv (needs code audit, 61a8231 fixed principal issue; re-init probably
not necessary)
* axgbe (needs code audit; re-init probably not necessary)
* iavf - (needs code audit; interaction with Malicious Driver Detection
mentioned in rS360398)
* mgb - no VLAN functions are currently implemented. Left a comment.

MFC after: 2 weeks
Sponsored by: BBOX.io
Differential Revision: https://reviews.freebsd.org/D41558

show more ...

iflib drivers: Constify PCI ID LUTs

Since d49e83eac3baf16a22b1c5d42e8438b68b17e6f9, iflib(9) is ready
for this change.
While at it, make isc_driver_version strings (static) const where
not apparentl

iflib drivers: Constify PCI ID LUTs

Since d49e83eac3baf16a22b1c5d42e8438b68b17e6f9, iflib(9) is ready
for this change.
While at it, make isc_driver_version strings (static) const where
not apparently un-const on purpose, too.
This reduces the size of the amd64 GENERIC by about 10 KiB.

show more ...

sys: Remove $FreeBSD$: one-line .c pattern

Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/

Last change for mgb(4) needed for IfAPI conversion

Sponsored by: Juniper Networks, Inc.

mgb: quiten -Wunused-function with LINT builds

As the function itself seems to be helpful don't remove the code but
change the #ifdef DEBUG to #if 0 to avoid:
warning: unused function 'mgb_dump_so

mgb: quiten -Wunused-function with LINT builds

As the function itself seems to be helpful don't remove the code but
change the #ifdef DEBUG to #if 0 to avoid:
warning: unused function 'mgb_dump_some_stats' [-Wunused-function]
with LINT builds.

show more ...

mgb: Remove unused devclass argument to DRIVER_MODULE.

Remove unused miibus_devclass and miibus_fdt_devclass.

mgb: remove set but not used variables

Sponsored by: The FreeBSD Foundation

mgb: Fix nop admin interrupt handling

Previously mgb_admin_intr printed a diagnostic message if no interrupt
status bits were set, but it's not valid to call device_printf() from a
filter. Just dro

mgb: Fix nop admin interrupt handling

Previously mgb_admin_intr printed a diagnostic message if no interrupt
status bits were set, but it's not valid to call device_printf() from a
filter. Just drop the message as it has no user-facing value.

Also return FILTER_STRAY in this case - there is nothing further for
the driver to do.

Reviewed by: kbowling
MFC after: 1 week
Fixes: 8890ab7758b8 ("Introduce if_mgb driver...")
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D32231

show more ...

Revert "mgb: Use MGB_DEBUG instead of DEBUG"

This reverts commit 5aa9f8dae3d40e1780a688ce01401e799b25e7c3.

We might as well get coverage of this code via LINT.

Reported by: mhorne

mgb: Use MGB_DEBUG instead of DEBUG

The debug register dump routine is not hooked up and is really only
useful to driver developers, so put it under an mgb-specific MGB_DEBUG
rather than general DEB

mgb: Use MGB_DEBUG instead of DEBUG

The debug register dump routine is not hooked up and is really only
useful to driver developers, so put it under an mgb-specific MGB_DEBUG
rather than general DEBUG.

MFC after: 1 week
Fixes: 8890ab7758b8 ("Introduce if_mgb driver...")
Sponsored by: The FreeBSD Foundation

show more ...

mgb: Fix DEBUG (and LINT) build

Sponsored by: The FreeBSD Foundation

mgb: Apply some style(9)

Add parens around return values, rewrap lines

MFC after: 1 week
Fixes: 8890ab7758b8 ("Introduce if_mgb driver...")
Sponsored by: The FreeBSD Foundation

mgb: Staticize devclass and iflib structs (as is typical)

MFC after: 1 week
Fixes: 8890ab7758b8 ("Introduce if_mgb driver...")
Sponsored by: The FreeBSD Foundation

mgb: Do not KASSERT on error in mgb_init

There's not much we can do if mii_mediachg() fails, but KASSERT is not
appropriate.

MFC after: 1 week
Fixes: 8890ab7758b8 ("Introduce if_mgb driver...")
Sp

mgb: Do not KASSERT on error in mgb_init

There's not much we can do if mii_mediachg() fails, but KASSERT is not
appropriate.

MFC after: 1 week
Fixes: 8890ab7758b8 ("Introduce if_mgb driver...")
Sponsored by: The FreeBSD Foundation

show more ...

mgb: enable multicast in mgb_init

Receive Filtering Engine (RFE) configuration is not yet implemented,
and mgb intended to enable all broadcast, multicast, and unicast.
However, MGB_RFE_ALLOW_MULTIC

mgb: enable multicast in mgb_init

Receive Filtering Engine (RFE) configuration is not yet implemented,
and mgb intended to enable all broadcast, multicast, and unicast.
However, MGB_RFE_ALLOW_MULTICAST was missed (MGB_RFE_ALLOW_UNICAST was
included twice).

MFC after: 1 week
Fixes: 8890ab7758b8 ("Introduce if_mgb driver...")
Sponsored by: The FreeBSD Foundation

show more ...

mgb: update Microchip URLs

Sponsored by: The FreeBSD Foundation

iflib: ensure that tx interrupts enabled and cleanups

Doing a 'dd' over iscsi will reliably cause stalls. Tx
cleaning _should_ reliably happen as data is sent.
However, currently if the transmit que

iflib: ensure that tx interrupts enabled and cleanups

Doing a 'dd' over iscsi will reliably cause stalls. Tx
cleaning _should_ reliably happen as data is sent.
However, currently if the transmit queue fills it will
wait until the iflib timer (hz/2) runs.

This change causes the the tx taskq thread to be run
if there are completed descriptors.

While here:

- make timer interrupt delay a sysctl

- simplify txd_db_check handling

- comment on INTR types

Background on the change:

Initially doorbell updates were minimized by only writing to the register
on every fourth packet. If txq_drain would return without writing to the
doorbell it scheduled a callout on the next tick to do the doorbell write
to ensure that the write otherwise happened "soon". At that time a sysctl
was added for users to avoid the potential added latency by simply writing
to the doorbell register on every packet. This worked perfectly well for
e1000 and ixgbe ... and appeared to work well on ixl. However, as it
turned out there was a race to this approach that would lockup the ixl MAC.
It was possible for a lower producer index to be written after a higher one.
On e1000 and ixgbe this was harmless - on ixl it was fatal. My initial
response was to add a lock around doorbell writes - fixing the problem but
adding an unacceptable amount of lock contention.

The next iteration was to use transmit interrupts to drive delayed doorbell
writes. If there were no packets in the queue all doorbell writes would be
immediate as the queue started to fill up we could delay doorbell writes
further and further. At the start of drain if we've cleaned any packets we
know we've moved the state machine along and we write the doorbell (an
obvious missing optimization was to skip that doorbell write if db_pending
is zero). This change required that tx interrupts be scheduled periodically
as opposed to just when the hardware txq was full. However, that just leads
to our next problem.

Initially dedicated msix vectors were used for both tx and rx. However, it
was often possible to use up all available vectors before we set up all the
queues we wanted. By having rx and tx share a vector for a given queue we
could halve the number of vectors used by a given configuration. The problem
here is that with this change only e1000 passed the necessary value to have
the fast interrupt drive tx when appropriate.

Reported by: mav@
Tested by: mav@
Reviewed by: gallatin@
MFC after: 1 month
Sponsored by: iXsystems
Differential Revision: https://reviews.freebsd.org/D27683

show more ...

mgb: clean up empty lines in .c and .h files

iflib: leave only 1 receive descriptor unused

The pidx argument of isc_rxd_flush() indicates which is the last valid
receive descriptor to be used by the NIC. However, current code has
multiple issu

iflib: leave only 1 receive descriptor unused

The pidx argument of isc_rxd_flush() indicates which is the last valid
receive descriptor to be used by the NIC. However, current code has
multiple issues:
- Intel drivers write pidx to their RDT register, which means that
NICs will only use the descriptors up to pidx-1 (modulo ring size N),
and won't actually use the one pointed by pidx. This does not break
reception, but it is anyway confusing and suboptimal (the NIC will
actually see only N-2 descriptors as available, rather than N-1).
Other drivers (if_vmx, if_bnxt, if_mgb) adhere to this semantic).
- The semantic used by Intel (RDT is one descriptor past the last
valid one) is used by most (if not all) NICs, and it is also used
on the TX side (also in iflib). Since iflib is not currently
using this semantic for RX, it must decrement fl->ifl_pidx
(modulo N) before calling isc_rxd_flush(), and then the
per-driver callback implementation must increment the index
again (to match the real semantic). This is confusing and suboptimal.
- The iflib refill function is also called at initialization.
However, in case the ring size is smaller than 128 (e.g. if_mgb),
the refill function will actually prepare all the receive
descriptors (N), without leaving one unused, as most of NICs assume
(e.g. to avoid RDT to overrun RDH). I can speculate that the code
looks like this right now because this issue showed up during
testing (e.g. with if_mgb), and it was easy to workaround by
decrementing pidx before isc_rxd_flush().

The goal of this change is to simplify the code (removing a bunch
of instructions from the RX fast path), and to make the semantic of
isc_rxd_flush() consistent across drivers. To achieve this, we:
- change the semantics of the pidx argument to the usual one (that
is the index one past the last valid one), so that both iflib and
drivers avoid the decrement/increment dance.
- fix the initialization code to prepare at most N-1 descriptors.

Reviewed by: markj
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D26191

show more ...

mgb: Correct spello - recieve -> receive

Reported by: N. J. Mann
MFC with: r354410

Introduce if_mgb driver for Microchip LAN743x PCIe NIC

The Microchip LAN7430 is a PCIe 10/100/1000 Ethernet MAC with integrated
PHY, and the LAN7431 is a MAC with RGMII interface.

To be connected t

Introduce if_mgb driver for Microchip LAN743x PCIe NIC

The Microchip LAN7430 is a PCIe 10/100/1000 Ethernet MAC with integrated
PHY, and the LAN7431 is a MAC with RGMII interface.

To be connected to the build after further testing and review.
Committing now so that changes like r354345 (adding a common
ETHER_IS_ZERO macro) will update this driver too.

Submitted by: Gerald ND Aryeetey <aryeeteygerald_rogers.com>
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D20079

show more ...

iflib: invert default restart on VLAN changes

In rS360398, a new iflib device method was added to opt out of VLAN
events needing an interface reset.

I am switching the default to not requiring a re

iflib: invert default restart on VLAN changes

In rS360398, a new iflib device method was added to opt out of VLAN
events needing an interface reset.

I am switching the default to not requiring a restart for:
* VLAN events
* unknown events

After fixing various bugs, I do not think this would be a common need
of hardware and it is undesirable from the user's perspective causing
link flaps and much slower VLAN configuration. Currently, there are no
other restart events besides VLAN events, and setting the
ifdi_needs_restart default to false will alleviate the need to churn
every driver if an odd event is added in the future for specific
hardware.

markj points out this could cause churn in the other direction; I will
solve that problem with an event registration system as he mentions in
the review should we need it in the future.

These drivers will opt into restart and need further inspection or work:
* ixv (needs code audit, 61a8231 fixed principal issue; re-init probably
not necessary)
* axgbe (needs code audit; re-init probably not necessary)
* iavf - (needs code audit; interaction with Malicious Driver Detection
mentioned in rS360398)
* mgb - no VLAN functions are currently implemented. Left a comment.

MFC after: 2 weeks
Sponsored by: BBOX.io
Differential Revision: https://reviews.freebsd.org/D41558

show more ...

iflib drivers: Constify PCI ID LUTs

Since d49e83eac3baf16a22b1c5d42e8438b68b17e6f9, iflib(9) is ready
for this change.
While at it, make isc_driver_version strings (static) const where
not apparentl

iflib drivers: Constify PCI ID LUTs

Since d49e83eac3baf16a22b1c5d42e8438b68b17e6f9, iflib(9) is ready
for this change.
While at it, make isc_driver_version strings (static) const where
not apparently un-const on purpose, too.
This reduces the size of the amd64 GENERIC by about 10 KiB.

show more ...

sys: Remove $FreeBSD$: one-line .c pattern

Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/