Paths

Table of Contentst

Differential D26683

ntb: Add Xeon Gen3 support
ClosedPublic
Actions

Authored by markj on Oct 5 2020, 2:52 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Mon, Dec 2, 11:28 PM

	Unknown Object (File)
	Nov 22 2024, 3:14 PM

	Unknown Object (File)
	Nov 19 2024, 10:08 PM

	Unknown Object (File)
	Nov 18 2024, 2:29 AM

	Unknown Object (File)
	Nov 18 2024, 12:03 AM

	Unknown Object (File)
	Nov 16 2024, 6:02 AM

	Unknown Object (File)
	Nov 1 2024, 6:10 AM

	Unknown Object (File)
	Oct 20 2024, 6:41 PM

View All 83 Files

Subscribers

Details

Reviewers

bret_ketchum_dell.com
mav
cem
vangyzen

Group Reviewers

Commits

rS366969: ntb: Add Intel Xeon Gen3 support

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 34230
Build 31375: arc lint + arc unit

Event Timeline

markj requested review of this revision.Oct 5 2020, 2:52 PM

markj created this revision.

Harbormaster completed remote builds in B34001: Diff 77900.Oct 5 2020, 2:52 PM

markj added a reviewer: bret_ketchum_dell.com.Oct 5 2020, 2:52 PM

allanjude added a subscriber: NetApp.Oct 6 2020, 10:02 PM

Looks good to me.

sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1301	Might want to keep the comment and assert on default.

markj added inline comments.Oct 13 2020, 2:06 PM

sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1301	Note that the NTB type is an enum, so I'm not sure that having a default case makes sense.
1312	Isilon's patch and Linux and DPDK perform 8-byte reads and writes for doorbells.

A few fixups, switch on the NTB hardware type to make code a bit clearer.
Update the man page.

Harbormaster completed remote builds in B34132: Diff 78164.Oct 13 2020, 2:07 PM

markj added reviewers: mav, cem.Oct 13 2020, 2:07 PM

No objection. I don’t really want to be an NTB maintainer, fwiw. I skimmed maybe half of this and it seemed reasonable.

This revision is now accepted and ready to land.Oct 13 2020, 4:59 PM

In D26683#596653, @cem wrote:

No objection. I don’t really want to be an NTB maintainer, fwiw. I skimmed maybe half of this and it seemed reasonable.

Thanks for looking. If I don't get any feedback in the next few days I'll just go ahead and commit.

In D26683#596676, @markj wrote:

Thanks for looking. If I don't get any feedback in the next few days I'll just go ahead and commit.

👌

I'm not an NTB expert, but LGTM.

sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1146	`desired_vectors` was just assigned to `num_vectors`, so this is always true.
1155	Indentation?
1301	It would catch a scribbler earlier.
1835	Indentation?
2142–2143	Indentation? I assume this is a rare case, but adding the sizes to the error message could be helpful.
3253–3256	You could remove the condition and keep only `limit = base + size;`. Or maybe a different condition is needed?

markj marked 4 inline comments as done.Oct 17 2020, 3:16 PM

markj added inline comments.

sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1155	I think this can just be removed.
3253–3256	I believe we can just remove the conditional.

Address feedback.

This revision now requires review to proceed.Oct 17 2020, 3:16 PM

Harbormaster completed remote builds in B34230: Diff 78352.Oct 17 2020, 3:16 PM

vangyzen accepted this revision.Oct 18 2020, 10:18 PM

This revision is now accepted and ready to land.Oct 18 2020, 10:18 PM

Closed by commit rS366969: ntb: Add Intel Xeon Gen3 support (authored by markj). · Explain WhyOct 23 2020, 2:17 PM

This revision was automatically updated to reflect the committed changes.

markj added a commit: rS366969: ntb: Add Intel Xeon Gen3 support.

Herald added a reviewer: manpages. · View Herald TranscriptOct 23 2020, 2:17 PM

Herald added a subscriber: imp. · View Herald Transcript

Revision Contents
Changeset List

Path

Size

share/

man/

man4/

19 lines

sys/

dev/

ntb/

ntb_hw/

87 lines

398 lines

Diff 78352

share/man/man4/ntb_hw_intel.4

Loading...

sys/dev/ntb/ntb_hw/ntb_hw_intel.h

Loading...

sys/dev/ntb/ntb_hw/ntb_hw_intel.c

Loading...