Commit Graph

405 Commits

Author SHA1 Message Date
Mark Johnston
ffe3def903 iflib: Make if_shared_ctx_t a pointer to const
This structure is shared among multiple instances of a driver, so we
should ensure that it doesn't somehow get treated as if there's a
separate instance per interface.  This is especially important for
software-only drivers like wg.

DEVICE_REGISTER() still returns a void * and so the per-driver sctx
structures are not yet defined with the const qualifier.

Reviewed by:	gallatin, erj
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D29102
2021-03-08 12:39:06 -05:00
Sai Rajesh Tallamraju
38bfc6dee3 iflib: Free resources in a consistent order during detach
Memory and PCI resources are freed with no particular order.  This could
cause use-after-frees when detaching following a failed attach.  For
instance, iflib_tx_structures_free() frees ctx->ifc_txqs[] but
iflib_tqg_detach() attempts to access this array. Similarly, adapter
queues gets freed by IFDI_QUEUES_FREE() but IFDI_DETACH() attempts to
access adapter queues to free PCI resources.

MFC after:	2 weeks
Sponsored by:	NetApp, Inc.
Differential Revision:	https://reviews.freebsd.org/D27634
2021-02-01 11:15:54 -05:00
Marius Strobl
c262e8e87e e1000: fix build after c1655b0f 2021-01-27 15:30:35 +01:00
Marius Strobl
c1655b0f89 e1000: consistently use the hw variables
It's rather confusing when adapter->hw and hw are mixed and matched
within a particular function.
Some of this was missed in cd1cf2fc1d
and r353778 respectively.
2021-01-26 22:28:55 +01:00
Jessica Clarke
e07d687ebe Fix whitespace in r368698
MFC with:	r368698
2020-12-16 14:48:46 +00:00
Michal Meloun
63d973c3e9 Use the standard method for localizing of MSI-X table bar.
Current way, hardcoded value plus heuristic is not conform to the PCI(e)
specification and it fails on systems where MSI-X bar is not initialized by
BIOS/ACPI (many arm or arm64 systems for example).
Instead, use the standard PCI(e) capability for determining of
MSIX table bar address.

MFC after: 3 weeks
Differential Revision: https://reviews.freebsd.org/D27265
2020-12-16 14:39:24 +00:00
Mitchell Horne
a3cd24398a em: fix a null de-reference in em_free_pci_resources
A failure in iflib_device_register() can result in
em_free_pci_resources() being called after receive queues have already
been freed. In particular, a failure to allocate IRQ resources will goto
fail_queues, where IFDI_QUEUES_FREE() will be called via
iflib_tx_structures_free(), preceding the call to IFDI_DETACH().

Cope with this by checking adapter->rx_queues before dereferencing it.
A similar check is present in ixgbe(4) and ixl(4).

MFC after:	1 week
Sponsored by:	NetApp, Inc.
Sponsored by:	Klara, Inc.
Differential Revision:	https://reviews.freebsd.org/D27260
2020-12-02 17:37:32 +00:00
Eric Joyner
a3b9a7366e e1000: Properly retain promisc flag
From Franco:
The iflib rewrite forced the promisc flag but it was not reported
to the system.  Noticed on a stock VM that went into unsolicited
promisc mode when dhclient was started during bootup.

PR:		248869
Submitted by:	Franco Fichtner <franco@opnsense.org>
Reviewed by:	erj@
MFC after:	3 days
2020-09-15 21:07:30 +00:00
Eric Joyner
467515a494 igb(4): Fix define and includes with RSS option enabled
This re-adds the opt_rss.h header to the driver and includes some
RSS-specific headers when RSS is defined.

PR:		249191
Submitted by:	Milosz Kaniewski <milosz.kaniewski@gmail.com>
MFC after:	3 days
2020-09-15 21:00:25 +00:00
Vincenzo Maffione
7e6223b23f em(4): honor vlanhwtag offload
The FreeBSD em driver fails to properly reset the VME flag
in the e1000 CTRL register oneg the following ifconfig command

	ifconfig em1 -vlanhwtag

Tested on the e1000 device emulated by QEMU, and on a real
NIC (chip=0x10d38086).

PR:	236584
Submitted by:	 murat@sunnyvalley.io
Reported by:	 murat@sunnyvalley.io
MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D25286
2020-08-06 21:01:26 +00:00
Eric Joyner
104d75a051 em(4): Always reinit interface when adding/removing VLAN
This partially reverts r361053 since there have been reports
by users that this breaks some functionality for em(4)
devices; it seems at first glance that some sort of interface
restart is required for those cards.

This isn't a proper fix; this unbreaks those users until a proper
fix is found for their issues.

PR:		240818
Reported by:	Marek Zarychta <zarychtam@plan-b.pwste.edu.pl>
MFC after:	3 days
2020-06-11 15:59:49 +00:00
Eric Joyner
51569bd793 em(4): Add support for Comet Lake Mobile Platform, update shared code
This change introduces Comet Lake Mobile Platform support in the e1000
driver along with shared code patches described below.

- Cast return value of e1000_ltr2ns() to higher type to avoid overflow
- Remove useless statement of assigning act_offset
- Add initialization of identification LED
- Fix flow control setup after connected standby:
  After connected standby the driver blocks resets during
  "AdapterStart" and skips flow control setup. This change adds
  condition in e1000_setup_link_ich8lan() to always setup flow control
  and to setup physical interface only when there is no need to block
  resets.

Signed-off-by: Piotr Pietruszewski <piotr.pietruszewski@intel.com>

Submitted by:	Piotr Pietruszewski <piotr.pietruszewski@intel.com>
Reviewed by:	erj@
Tested by:	Jeffrey Pieper <jeffrey.e.pieper@intel.com>
MFC after:	1 week
Relnotes:	yes
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D25035
2020-06-04 20:39:28 +00:00
Eric Joyner
cf1509179c em/ix/ixv/ixl/iavf: Implement ifdi_needs_restart iflib method
Pursuant to r360398, implement driver-specific versions of the
ifdi_needs_restart iflib device method.

Some (if not most?) Intel network cards don't need reinitializing when a
VLAN is added or removed from the device hardware, so these implement
ifdi_needs_restart in a way that tell iflib not to bring the interface
up or down when a VLAN is added or removed, regardless of whether the
VLAN_HWFILTER interface capability flag is set or not.

This could potentially solve several PRs relating to link flaps that
occur when VLANs are added/removed to devices.

Signed-off-by: Eric Joyner <erj@freebsd.org>

PR:		240818, 241785
Reviewed by:	gallatin@, olivier@
MFC after:	3 days
MFC with:	r360398
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D24659
2020-05-11 17:42:04 +00:00
Pawel Biernacki
20b91f0aa5 Mark more nodes as CTLFLAG_MPSAFE or CTLFLAG_NEEDGIANT (15 of many)
r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly marked).
Use it in preparation for a general review of all nodes.

This is non-functional change that adds annotations to SYSCTL_NODE and
SYSCTL_PROC nodes using one of the soon-to-be-required flags.
2020-02-24 10:51:26 +00:00
George V. Neville-Neil
927dd68e04 Add support for latest Intel I219 device, supported in Lenovo Carbon X1 v7
MFC after:	2 weeks
2020-01-20 12:53:02 +00:00
Eric Joyner
e81998f407 net: prefer ETHER_ADDR_LEN over ETH_ADDR_LEN
A couple of drivers and one place in if.c use ETH_ADDR_LEN, even though
net/ethernet.h provides an equivalent ETHER_ADDR_LEN definition.

Cleanup all of the locations which refer to ETH_ADDR_LEN to use the
standard ETHER_ADDR_LEN instead.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
Reviewed by:	erj@, jpaetzel@
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D21239
2019-11-04 22:57:36 +00:00
Gleb Smirnoff
c42980268b Convert to if_foreach_llmaddr() KPI. 2019-10-21 18:11:24 +00:00
Marius Strobl
cd1cf2fc1d - In em_intr(), just call em_handle_link() instead of duplicating it.
- In em_msix_link(), properly handle IGB-class devices after the iflib(4)
  conversion again by only setting EM_MSIX_LINK for the EM-class 82574
  and by re-arming link interrupts unconditionally, i. e. not only in
  case of spurious interrupts. This fixes the interface link state change
  detection for the IGB-class. [1]
- In em_if_update_admin_status(), only re-arm the link state change
  interrupt for 82574 and also only if such a device uses MSI-X, i. e.
  takes advantage of autoclearing. In case of INTx and MSI as well as
  for LEM- and IGB-class devices, re-arming isn't appropriate here and
  setting EM_MSIX_LINK isn't either.
  While at it, consistently take advantage of the hw variable.

PR:	236724 [1]
Differential Revision:	https://reviews.freebsd.org/D21924
2019-10-20 17:40:50 +00:00
Eric Joyner
ea0e3f4db5 e1000: correctly set isc_pause_frames only when XOFF increases
From Jake:
The e1000 driver sets the iflib shared context isc_pause_frames value to
the number of received xoff frames. This is done so that the iflib
watchdog timer won't trigger a Tx Hang due to pause frames.

Unfortunately, the function simply sets it to the value of the xoffrxc
counter. Once the device has received a single XOFF packet, the driver
always reports that we received pause frames. This will prevent the Tx
hang detection entirely from that point on.

Fix this by assigning isc_pause_frames to a non-zero value if we
received any XOFF packets in the last timer interval.

We could attempt to calculate the total number of received packets by
doing a subtraction, but the iflib stack only seems to check if
isc_pause_frames is non-zero.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
Reviewed by:	gallatin@
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D21868
2019-10-16 17:13:46 +00:00
Marius Strobl
ca2ebb27ed o Avoid determining the MAC class (LEM/EM or IGB) - possibly even multiple
times - on every interrupt by using an own set of device methods for the
  IGB class. This translates to introducing igb_if_intr_{disable,enable}()
  and igb_if_{rx,tx}_queue_intr_enable() with that IGB-specific code moved
  out of their EM counterparts and otherwise continuing to use the EM IFDI
  methods also for IGB.
  Note that igb_if_intr_{disable,enable}() also issue E1000_WRITE_FLUSH as
  lost with the conversion of igb(4) to iflib(4).
  Also note, that the em_if_{disable,enable}_intr() methods are renamed to
  em_if_intr_{disable,enable}() for consistency with the names used in the
  interface declaration.
o In em_intr():
  - Don't bother to bail out if the interrupt type is "legacy", i. e. INTx
    or MSI, as iflib(4) doesn't use ift_legacy_intr methods for MSI-X. All
    other iflib(4)-based drivers avoid this check, too.
  - Given that only the MSI-X interrupts have one-shot behavior (by taking
    advantage of the EIAC register), explicitly disable interrupts. Hence,
    em_intr() now matches what {em,igb}_irq_fast() previously did (in case
    of igb(4) supposedly also to work around MSI message reordering errata
    on certain systems).
o In em_if_intr_disable():
  - Clear the EIAC register unconditionally for 82574 and not just in case
    of MSI-X, matching em_if_intr_enable() and bringing back the last hunk
    of r206437 lost with the iflib(4) conversion.
  - Write to EM_EIAC for clearing said register instead of to the IGB-only
    E1000_EIAC used ever since the iflib(4) conversion.

Reviewed by:	shurd
Differential Revision:	https://reviews.freebsd.org/D20176
2019-05-07 08:31:54 +00:00
Eric Joyner
1b9d93948a iflib: expose the Rx mbuf buffer size to drivers
From Jake:
iflib_fl_setup calculates a suitable buffer size for the Rx mbufs based
on the isc_max_frame_size value that drivers setup. This calculation is
repeated by drivers when programming their hardware with the size of
each Rx buffer.

This can lead to a mismatch where the iflib mbuf size is different from
the expected size of the buffer as programmed by the hardware. This can
lead to unexpected results.

If iflib ever wants to support mbuf sizes larger than one page, every
driver must be updated to account for the new possible buffer sizes.

Fix this by calculating the mbuf size prior to calling IFDI_INIT, and
adding the iflib_get_rx_mbuf_sz function which will expose this value to
drivers, so that they do not repeat the same calculation.

Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
Reviewed by:	shurd@, erj@
MFC after:	1 week
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D19489
2019-03-19 17:59:56 +00:00
Eric Joyner
bc408c7d61 Remove references to CONTIGMALLOC_WORKS in iflib and em
From Jake:
"The iflib_fl_setup() function tries to pick various buffer sizes based
on the max_frame_size value defined by the parent driver. However, this
code was wrapped under CONTIGMALLOC_WORKS, which was never actually
defined anywhere.

This same code pattern was used in if_em.c, likely trying to match
what iflib uses.

Since CONTIGMALLOC_WORKS is not defined, remove this dead code from
iflib_fl_setup and if_em.c

Given that various iflib drivers appear to be using a similar
calculation, it might be worth making this buffer size a value that the
driver can peek at in the future."

Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
Reviewed by:	shurd@
MFC after:	1 week
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D19199
2019-03-05 19:12:51 +00:00
Marius Strobl
6143b97764 - Remove the redundant device disabled hint handling; ever since
r241119 that's performed globally by device_attach(9).
- As for the EM-class of devices, em(4) supports multiple queues
  and MSI-X respectively only with 82574 devices. However, since
  the conversion to iflib(4), em(4) relies on the interrupt type
  fallback mechanism, i. e. MSI-X -> MSI -> INTx, of iflib(4) to
  figure out the interrupt type to use for the EM-class (as well
  as the IGB-class) of MACs. Moreover, despite the datasheet for
  82583V not mentioning any support of MSI-X, there actually are
  82583V devices out there that report a varying number of MSI-X
  messages as supported. The interrupt type fallback of iflib(4)
  is causing two failure modes depending on the actual number of
  MSI-X messages supported for such instances of 82583V:
  1) With only one MSI-X message supported, none is left for the
     RX/TX queues as that one message gets assigned to the admin
     interrupt. Worse, later on - which will be addressed with a
     separate fix - iflib(4) interprets that one messages as MSI
     or INTx to be set up, but fails to actually do so as it has
     previously called pci_alloc_msix(9). [1, 2]
  2) With more message supported, their distribution is okay but
     then em_if_msix_intr_assign() doesn't work for 82583V, with
     the interface being left in a non-working state, too. [3]
  Thus, let em_if_attach_pre() indicate to iflib(4) to try MSI-X
  with 82574 only, and at most MSI for the remainder of EM-class
  devices.
  While at it, remove "try_second_bar" as it's polarity inverted
  and not actually needed.
- Remove code from em_if_timer() that effectively is a NOP since
  the conversion to iflib(4) ("trigger" is no longer read).
  While at it, let the comment for em_if_timer() reflect reality
  after said conversion.
- Implement an ifdi_watchdog_reset method which only updates the
  em(4) "watchdog_events" counter but doesn't perform any reset,
  so that the em(4) "watchdog_timeouts" SYSCTL (iflib(4) doesn't
  provide a counterpart) reflects reality and these timeouts add
  to IFCOUNTER_OERRORS again after the iflib(4) conversion.
- Remove the "mbuf_defrag_fail" and "tx_dma_fail" SYSCTLS; since
  the iflib(4) conversion, associated counters are disconnected,
  but iflib(4) provides "mbuf_defrag_failed" and "tx_map_failed"
  respectively as equivalents.
- Move the description preceding lem_smartspeed() to the correct
  spot before em_reset() and bring back appropriate comments for
  {igb,em}_initialize_rss_mapping() and lem_smartspeed() lost in
  the iflib(4) conversion.
- Adapt some other function descriptions and INIT_DEBUGOUT() use
  to match reality after the iflib(4) conversion.
- Put the debugging message of em_enable_vectors_82574() (missed
  in r343578) under bootverbose, too.

PR:		219428 [1], 235246 [2], 235147 [3]
Reviewed by:	erj (previous version)
Differential Revision:	https://reviews.freebsd.org/D19108
2019-02-09 11:58:40 +00:00
Patrick Kelsey
d533db848c Fix em(4) interrupt routing
When configured with more tx queues than rx queues,
em_if_msix_intr_assign() was incorrectly routing the tx event
interrupts.

Reviewed by:	erj, marius
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D19070
2019-02-08 20:34:47 +00:00
Marius Strobl
b97de13ae0 - Stop iflib(4) from leaking MSI messages on detachment by calling
bus_teardown_intr(9) before pci_release_msi(9).
- Ensure that iflib(4) and associated drivers pass correct RIDs to
  bus_release_resource(9) by obtaining the RIDs via rman_get_rid(9)
  on the corresponding resources instead of using the RIDs initially
  passed to bus_alloc_resource_any(9) as the latter function may
  change those RIDs. Solely em(4) for the ioport resource (but not
  others) and bnxt(4) were using the correct RIDs by caching the ones
  returned by bus_alloc_resource_any(9).
- Change the logic of iflib_msix_init() around to only map the MSI-X
  BAR if MSI-X is actually supported, i. e. pci_msix_count(9) returns
  > 0. Otherwise the "Unable to map MSIX table " message triggers for
  devices that simply don't support MSI-X and the user may think that
  something is wrong while in fact everything works as expected.
- Put some (mostly redundant) debug messages emitted by iflib(4)
  and em(4) during attachment under bootverbose. The non-verbose
  output of em(4) seen during attachment now is close to the one
  prior to the conversion to iflib(4).
- Replace various variants of spelling "MSI-X" (several in messages)
  with "MSI-X" as used in the PCI specifications.
- Remove some trailing whitespace from messages emitted by iflib(4)
  and change them to consistently start with uppercase.
- Remove some obsolete comments about releasing interrupts from
  drivers and correct a few others.

Reviewed by:	erj, Jacob Keller, shurd
Differential Revision:	https://reviews.freebsd.org/D18980
2019-01-30 13:21:26 +00:00
Eric Joyner
088a0b27ad intel iflib drivers: correct initialization of tx_cidx_processed
From Jake:

In r341156 ("Fix first-packet completion", 2018-11-28) a hack to work
around a delta calculation determining how many descriptors were used
was added to ixl_isc_tx_credits_update_dwb.

The same fix was also applied to the em and igb drivers in r340310, and
to ix in r341156.

The hack checked the case where prev and cur were equal, and then added
one. This works, because by the time we do the delta check, we already
know there is at least one packet available, so the delta should be at
least one.

However, it's not a complete fix, and as indicated by the comment is
really a hack to work around the real bug.

The real problem is that the first time that we transmit a packet,
tx_cidx_processed will be set to point to the start of the ring.
Ultimately, the credits_update function expects it to point to the
*last* descriptor that was processed. Since we haven't yet processed any
descriptors, pointing it to 0 results in this incorrect calculation.

Fix the initialization code to have it point to the end of the ring
instead. One way to think about this, is that we are setting the value
to be one prior to the first available descriptor.

Doing so, corrects the delta calculation in all cases. The original fix
only works if the first packet has exactly one descriptor. Otherwise, we
will report 1 less than the correct value.

As part of this fix, also update the MPASS assertions to match the real
expectations. First, ensure that prev is not equal to cur, since this
should never happen. Second, remove the assertion about prev==0 || delta
!= 0. It looks like that originated from when the em driver was
converted to iflib. It seems like it was supposed to ensure that delta
was non-zero. However, because we originally returned 0 delta for the
first calculation, the "prev == 0" was tacked on.

Instead, replace this with a check that delta is greater than zero,
after the correction necessary when the ring pointers wrap around.

This new solution should fix the same bug as r341156 did, but in a more
robust way.

Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
Reviewed by:	shurd@
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D18545
2019-01-24 01:03:00 +00:00
Stephen Hurd
cf49cdd5a3 Fix first-packet completion
The first packet after the ring is initialized was never
completed as isc_txd_credits_update() would not include it in the
count of completed packets. This caused netmap to never complete
a batch. See PR 233022 for more details.

PR:		233022
Reported by:	lev
Reviewed by:	lev
MFC after:	3 days
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D17931
2018-11-09 22:18:43 +00:00
Eric Joyner
adf93b56dc em/igb/ix(4): Port two Tx/Rx fixes made to ixl in r339338
- Fix assert/panic on receive when Jumbo Frames are enabled.

From the commit I made to ixl:
"It turns out that *_isc_rxd_available is supposed to return how many
packets are available to be cleaned on the rx ring. This patch removes
a section of code where if the budget argument is 1, the function would return
one if there was a descriptor available, not necessarily a packet.

This is okay in regular mtu 1500 traffic since the max frame size is less
than the configured receive buffer size (2048), but this doesn't work when
received packets can span more than one  descriptor, as is the case when the
mtu is 9000 and the receive buffer size is 4096."

- Fix possible Tx hang because *_isc_txd_credits_update returns incorrect result

From the commit by Krzysztof Galazka to ixl: "Function isc_txd_update_credits
called with clear set to false should return 1 if there are TX descriptors
already handled by HW. It was always returning 0 causing troubles with UDP TX
traffic."

PR:             231659
Reported by:    lev@
Approved by:	re (gjb@)
Sponsored by:   Intel Corporation
2018-10-14 05:09:43 +00:00
Stephen Hurd
0544676baf Use mbuf defines to construct csum offload masks rather than literals
Reviewed by:	erj
Approved by:	re (rgrimes)
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D17442
2018-10-09 20:16:19 +00:00
Eric van Gyzen
382000a1fd em/igb: Do not print link state messages
These messages are totally redundant with the iflib messages.
They're also not very useful, since they don't include the
interface name.

Discussed with:	shurd
Approved by:	re (rgrimes)
Sponsored by:	Dell EMC Isilon
2018-10-08 01:28:46 +00:00
Stephen Hurd
e873ccd0fc Fix igb corrupting checksums with BPF and VLAN
When using a vlan with igb and the vlanhwcsum option, any mbufs which
already had the TCP, UDP, or SCTP checksum calculated and therefore don't
have the CSUM_[IP|IP6]_[TCP|UDP|SCTP] bits set in the csum_flags field would
have the L4 checksum corrupted by the hardware.

This was caused by the driver setting E1000_TXD_POPTS_TXSM any time a
checksum bit was set OR a vlan tag was present.

The patched driver only sets E1000_TXD_POPTS_TXSM when an offload is
requested.

PR:		231416
Reported by:	pi
Approved by:	re (gjb)
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D17404
2018-10-05 20:16:20 +00:00
Stephen Hurd
861437f83a Add IFCAP_TSO6 for igb
It seems igb supports TSO6, but the capability got lost in
the iflib update. Restore this capability.

PR:		231476
Reported by:	lev
Reviewed by:	erj
Approved by:	re (gjb)
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D17242
2018-09-20 20:06:44 +00:00
Marius Strobl
73ed47f04f Remove the duplicated CSUM_IP6_TCP introduced in r311849 from the TX
checksum capabilities of IGB-class MACs. While at it, fix the line
wrapping.

PR:	230571
2018-08-13 20:29:39 +00:00
Marius Strobl
9820d94555 o In em_if_update_admin_status():
- Don't bother calling if_setbaudrate(9) as iflib_link_state_change(9)
    takes care of that,
  - correctly check for E1000_CTRL_EXT_LINK_MODE_GMII in E1000_CTRL_EXT [1],
  - properly convert the uint16_t link_speed to a uint64_t baudrate by
    using IF_Mbps() which contains an appropriate cast [2],
  - remove the duplicate link down announcement when bootverbose isn't
    zero and bring the remaining one in line with the other link state
    messages.
o Remove a dead store to rid in em_if_msix_intr_assign(). [3]
o Or in the DMA coalescing Rx threshold so the other bits set in E1000_DMACR
  remain intact as intended in igb_init_dmac(). [4]

Reported by:	Coverity
CID:		1378464 [1], 1368765 [2], 1381681 [3], 1304929 [4]
2018-07-22 17:40:13 +00:00
Marius Strobl
c1176e63e8 Update igb_sctx_init for r336313, missed when incorporating shurd@'s
feedback on the initial D15720.

Reported by:	kib
2018-07-16 19:47:57 +00:00
Marius Strobl
7f87c0406d Assorted TSO fixes for em(4)/iflib(9) and dead code removal:
- Ever since the workaround for the silicon bug of TSO4 causing MAC hangs
  was committed in r295133, CSUM_TSO always got disabled unconditionally
  by em(4) on the first invocation of em_init_locked(). However, even with
  that problem fixed, it turned out that for at least e. g. 82579 not all
  necessary TSO workarounds are in place, still causing MAC hangs even at
  Gigabit speed. Thus, for stable/11, TSO usage was deliberately disabled
  in r323292 (r323293 for stable/10) for the EM-class by default, allowing
  users to turn it on if it happens to work with their particular EM MAC
  in a Gigabit-only environment.
  In head, the TSO workaround for speeds other than Gigabit was lost with
  the conversion to iflib(9) in r311849 (possibly along with another one
  or two TSO workarounds). Yet at the same time, for EM-class MACs TSO4
  got enabled by default again, causing device hangs. Therefore, change the
  default for this hardware class back to have TSO4 off, allowing users
  to turn it on manually if it happens to work in their environment as
  we do in stable/{10,11}. An alternative would be to add a whitelist of
  EM-class devices where TSO4 actually is reliable with the workarounds in
  place, but given that the advantage of TSO at Gigabit speed is rather
  limited - especially with the overhead of these workarounds -, that's
  really not worth it. [1]
  This change includes the addition of an isc_capabilities to struct
  if_softc_ctx so iflib(9) can also handle interface capabilities that
  shouldn't be enabled by default which is used to handle the default-off
  capabilities of e1000 as suggested by shurd@ and moving their handling
  from em_setup_interface() to em_if_attach_pre() accordingly.
- Although 82543 support TSO4 in theory, the former lem(4) didn't have
  support for TSO4, presumably because TSO4 is even more broken in the
  LEM-class of MACs than the later EM ones. Still, TSO4 for LEM-class
  devices was enabled as part of the conversion to iflib(9) in r311849,
  causing device hangs. So revert back to the pre-r311849 behavior of
  not supporting TSO4 for LEM-class at all, which includes not creating
  a TSO DMA tag in iflib(9) for devices not having IFCAP_TSO4 set. [2]
- In fact, the FreeBSD TCP stack can handle a TSO size of IP_MAXPACKET
  (65535) rather than FREEBSD_TSO_SIZE_MAX (65518). However, the TSO
  DMA must have a maxsize of the maximum TSO size plus the size of a
  VLAN header for software VLAN tagging. The iflib(9) converted em(4),
  thus, first correctly sets scctx->isc_tx_tso_size_max to EM_TSO_SIZE
  in em_if_attach_pre(), but later on overrides it with IP_MAXPACKET
  in em_setup_interface() (apparently, left-over from pre-iflib(9)
  times). So remove the later and correct iflib(9) to correctly cap
  the maximum TSO size reported to the stack at IP_MAXPACKET. While at
  it, let iflib(9) use if_sethwtsomax*().
  This change includes the addition of isc_tso_max{seg,}size DMA engine
  constraints for the TSO DMA tag to struct if_shared_ctx and letting
  iflib_txsd_alloc() automatically adjust the maxsize of that tag in case
  IFCAP_VLAN_MTU is supported as requested by shurd@.
- Move the if_setifheaderlen(9) call for adjusting the maximum Ethernet
  header length from {ixgbe,ixl,ixlv,ixv,em}_setup_interface() to iflib(9)
  so adjustment is automatically done in case IFCAP_VLAN_MTU is supported.
  As a consequence, this adjustment now is also done in case of bnxt(4)
  which missed it previously.
- Move the reduction of the maximum TSO segment count reported to the
  stack by the number of m_pullup(9) calls (which in the worst case,
  can add another mbuf and, thus, the requirement for another DMA
  segment each) in the transmit path for performance reasons from
  em_setup_interface() to iflib_txsd_alloc() as these pull-ups are now
  done in iflib_parse_header() rather than in the no longer existing
  em_xmit(). Moreover, this optimization applies to all drivers using
  iflib(9) and not just em(4); all in-tree iflib(9) consumers still
  have enough room to handle full size TSO packets. Also, reduce the
  adjustment to the maximum number of m_pullup(9)'s now performed in
  iflib_parse_header().
- Prior to the conversion of em(4)/igb(4)/lem(4) and ixl(4) to iflib(9)
  in r311849 and r335338 respectively, these drivers didn't enable
  IFCAP_VLAN_HWFILTER by default due to VLAN events not being passed
  through by lagg(4). With iflib(9), IFCAP_VLAN_HWFILTER was turned on
  by default but also lagg(4) was fixed in that regard in r203548. So
  just remove the now redundant and defunct IFCAP_VLAN_HWFILTER handling
  in {em,ixl,ixlv}_setup_interface().
- Nuke other redundant IFCAP_* setting in {em,ixl,ixlv}_setup_interface()
  which is (more completely) already done in {em,ixl,ixlv}_if_attach_pre()
  now.
- Remove some redundant/dead setting of scctx->isc_tx_csum_flags in
  em_if_attach_pre().
- Remove some IFCAP_* duplicated either directly or indirectly (e. g.
  via IFCAP_HWCSUM) in {EM,IGB,IXL}_CAPS.
- Don't bother to fiddle with IFCAP_HWSTATS in ixgbe(4)/ixgbev(4) as
  iflib(9) adds that capability unconditionally.
- Remove some unused macros from em(4).
- Bump __FreeBSD_version as some of the above changes require the modules
  of drivers using iflib(9) to be recompiled.

Okayed by:	sbruno@ at 201806 DevSummit Transport Working Group [1]
Reviewed by:	sbruno (earlier version), erj
PR:	219428 (part of; comment #10) [1], 220997 (part of; comment #3) [2]
Differential Revision:	https://reviews.freebsd.org/D15720
2018-07-15 19:04:23 +00:00
Sean Bruno
94f8b882e5 igb(4):
I210 restore functionality if pxeboot rom is enabled on this device.

r333345 attempted to determine if this code was needed or it was some kind
of work around for a problem.  Turns out, its definitely a work around for
hardware locking and synchronization that manifests itself if the option
Rom is enabled and is selected as a boot device (there was a PXE attempt).

Reviewed by:	mmacy
Differential Revision:	https://reviews.freebsd.org/D15439
2018-05-15 13:30:59 +00:00
Matt Macy
ca9551221b Remove bogus panic
r333345 added a panic to the default case statement on the incorrect
premise that it should "never happen" when in fact it is simply a
different adapter version.

Reported by:	markj
Approved by:	sbruno
2018-05-09 17:48:52 +00:00
Matt Macy
d5210708dd Sleep rather than spin in e1000 when doing long running config operations.
With r333218 it is now possible for drivers to use an sx lock and thus sleep while
waiting on long running operations rather than DELAY().

Reported by:	gallatin
Reviewed by:	sbruno
Approved by:	sbruno
MFC after:	1 month
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D14984
2018-05-08 01:39:45 +00:00
Matt Macy
8fd222ebb4 fix case where pidx_last might be used uninitialized
Reviewed by:	sbruno
2018-05-04 18:59:01 +00:00
Mark Johnston
fbf8b74c10 Use C99 initializers for iflib function tables.
Reviewed by:	sbruno
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D15041
2018-04-11 15:15:34 +00:00
Stephen Hurd
7021bf0569 Update copyright per Matthew Macy
"Under my tutelage Nicole did 85% of the work. At the time it seemed
simplest for a number of reasons to put my copyright on it. I now consider
that to have been a mistake."

Submitted by:	Matthew Macy <mmacy@mattmacy.io>
Reviewed by:	shurd
Approved by:	shurd
Differential Revision:	https://reviews.freebsd.org/D14766
2018-03-21 15:57:36 +00:00
Toomas Soome
a5b0fd9ca9 e1000g: this statement may fall through
The gcc 7 does check for switch statement fall through cases, and if legit,
such complaint can besilenced by /* FALLTHROUGH */ comment. Unfortunately
such comment is quite limited, but will still notify the reader.

This patch is backport from illumos, see
https://www.illumos.org/rb/r/941/

Reviewed by:	eadler
Differential Revision:	https://reviews.freebsd.org/D14663
2018-03-12 17:05:53 +00:00
Pedro F. Giffuni
ac2fffa4b7 Revert r327828, r327949, r327953, r328016-r328026, r328041:
Uses of mallocarray(9).

The use of mallocarray(9) has rocketed the required swap to build FreeBSD.
This is likely caused by the allocation size attributes which put extra pressure
on the compiler.

Given that most of these checks are superfluous we have to choose better
where to use mallocarray(9). We still have more uses of mallocarray(9) but
hopefully this is enough to bring swap usage to a reasonable level.

Reported by:	wosch
PR:		225197
2018-01-21 15:42:36 +00:00
Eitan Adler
8f07d847a1 sys/dev/e1000: fix uses of 1 << 31
Reviewed by:		erj (D13858)
2018-01-12 06:37:53 +00:00
Eric Joyner
1b65356b43 e1000: Fix typos in value written to register and a comment
The value written to E1000_TARC(0) wasn't intended to have every bit but
E1000_TARC0_CB_MULTIQ_3_REQ cleared; a ~ was missing.

Also change the referenced spec update section in the comment to the correct
section.

Sponsored by:	Intel Corporation
2018-01-11 19:24:51 +00:00
Pedro F. Giffuni
efaa3e0789 dev/(e1000,ixl): Make some use of mallocarray(9).
Reviewed by:	erj
Differential Revision: https://reviews.freebsd.org/D13833
2018-01-11 15:25:26 +00:00
Sean Bruno
6fe4c0a063 e1000: Add support for Ice Lake and Cannon Lake
Ths add initial support for Ice Lake and Cannon Lake ethernet devices.

This also addressed errata 1.5.4.4 for Sky Lake and Kabby Lake devices:
https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/i218-i219-ethernet-connection-spec-update.pdf?asset=9561

Submitted by:	Kevin Bowling <kevin.bowling@kev009.com>
Relnotes:	Yes
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D13660
2017-12-28 21:26:40 +00:00
Eitan Adler
caa7e52f3f kernel: Fix several typos and minor errors
- duplicate words
- typos
- references to old versions of FreeBSD

Reviewed by:	imp, benno
2017-12-27 03:23:21 +00:00
Alexander Kabaev
151ba7933a Do pass removing some write-only variables from the kernel.
This reduces noise when kernel is compiled by newer GCC versions,
such as one used by external toolchain ports.

Reviewed by: kib, andrew(sys/arm and sys/arm64), emaste(partial), erj(partial)
Reviewed by: jhb (sys/dev/pci/* sys/kern/vfs_aio.c and sys/kern/kern_synch.c)
Differential Revision: https://reviews.freebsd.org/D10385
2017-12-25 04:48:39 +00:00