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

Authored by emaste on Mar 3 2019, 9:19 AM.

Details

Reviewers

kevans
ami_tehutinetworks.net

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

Severity	Location	Code	Message
Warning	sys/dev/tn40xx/README:	CHMOD1	Invalid Executable
Warning	sys/dev/tn40xx/TLK10232_phy.h:	CHMOD1	Invalid Executable
Warning	sys/dev/tn40xx/tn40_mbuf.c:	CHMOD1	Invalid Executable

Unit

No Test Coverage

Build Status

Buildable 22850
Build 21937: arc lint + arc unit

Event Timeline

ami_tehutinetworks.net created this revision.Mar 3 2019, 9:19 AM

Harbormaster completed remote builds in B22850: Diff 54630.Mar 3 2019, 9:19 AM

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 added a reviewer: linimon.May 10 2019, 12:03 AM

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

linimon removed a subscriber: linimon.

linimon mentioned this in D19386: [new driver] tn40xx: Tehuti networks tn40xx device driver..May 10 2019, 12:06 AM

mav added a subscriber: mav.Oct 28 2019, 2:21 PM

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).

kevans added inline comments.Dec 28 2019, 2:11 AM

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.