Page MenuHomeFreeBSD

[new driver] tn40xx(4): Tehuti networks tn40xx device driver.
AbandonedPublic

Authored by emaste on Mar 3 2019, 9:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 17, 6:53 AM
Unknown Object (File)
Mon, Mar 11, 4:48 PM
Unknown Object (File)
Mon, Mar 11, 4:48 PM
Unknown Object (File)
Mon, Mar 11, 4:35 PM
Unknown Object (File)
Jan 24 2024, 7:51 PM
Unknown Object (File)
Jan 15 2024, 6:55 AM
Unknown Object (File)
Jan 11 2024, 7:58 AM
Unknown Object (File)
Jan 10 2024, 2:09 AM

Details

Summary

~kevans/tn40xx-build.diff

Added tn40xx

Changes after first round of tests.

Support FreeBSD 12.x and other fixes

Version 1.1

Bug fix.

Ver 1.2: Added README file. Added license header.

Diff Detail

Lint
Lint Warnings
SeverityLocationCodeMessage
Warningsys/dev/tn40xx/README:CHMOD1Invalid Executable
Warningsys/dev/tn40xx/TLK10232_phy.h:CHMOD1Invalid Executable
Warningsys/dev/tn40xx/tn40_mbuf.c:CHMOD1Invalid Executable
Unit
No Test Coverage
Build Status
Buildable 22850
Build 21937: arc lint + arc unit

Event Timeline

linimon retitled this revision from Tehuti networks tn40xx device driver. to [new driver] tn40xx(4): Tehuti networks tn40xx device driver..May 2 2019, 6:35 AM
linimon added a subscriber: linimon.
linimon edited reviewers, added: brooks, Core Team; removed: linimon.
linimon removed a subscriber: linimon.

A least AQR105_phy.c is still missing license headers. Please add headers to all source and header files.

The file list should almost certainly be in sys/conf/files not sys/conf/files.amd64.

I'm copying in @gallatin's comments from another review of this same driver -- please update this specific review with a new version using the "Update Diff" link on the right side of the web interface, or with arc diff --update D19433 (IIRC) if you're using arc. Entirely new reviews breaks the continuity and makes it harder to examine changes version-to-version.

@gallatin wrote:

There is code which is clearly FreeBSD specific that needs to be converted to comply with style(9).

Why did you feel the need to write your own mbuf copy and defrag routines?

Did you consider using iflib? That way you would not have to deal with parsing packets, or mbufs or even busdma. There is a fairly clean interface in iflib, such that iflib parses mbufs and then passes DMA s/g lists to a drivers transmit routine. This is used by the Intel and Broadcom drivers (among others).
sys/dev/tn40xx/tn40_hw.c
2950

@gallatin wrote, with line number adjusted:

Didn't you just increase nr_frags when we you added the new segment around line 3073?

FWIW, if you care at all about packet rate, it would likely to faster to confirm you have trailing space on the mbuf up to 60b, and zero-pad it out to 60b. That way you can just have 1 DMA segment across PCIe rather than 2.

The file list should almost certainly be in sys/conf/files not sys/conf/files.amd64.

Agreed- that one was my bad, but I wrote that long enough ago now that I don't recall why.

I would also suspect the pci dependency on every single file was wrong, and that was again my bad.

Why are there multiple reviews? The other one still appears to be open (https://reviews.freebsd.org/D18856)

I'd also like to hear more from the authors on why they did not use iflib.

erj added inline comments.
sys/dev/tn40xx/tn40_hw.c
1900

Shouldn't this comment be put immediately before the bdx_rx_init() definition?

brooks edited reviewers, added: kevans; removed: brooks, Core Team.

I think I ended up as reviewer due to license issues when I was on core. It seems stalled, but I'm not quite ready to commandeer and abandon it.

Abandon as Tehuti Networks is unfortunately out of business and hardware won't be available.