Add cxgbetool(8) to the base system.
ClosedPublic

Authored by np on Mar 2 2017, 2:36 AM.

Details

Summary

Move cxgbetool from tools/tools to usr.sbin. Compile and install it on
platforms where cxgbe(4) is built by default. New knobs (WITH_CXGBETOOL and
WITHOUT_CXGBETOOL) have been added so that the user can override the default
setting.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
np retitled this revision from to Add cxgbetool(8) to the base system..Mar 2 2017, 2:36 AM
np updated this object.
np edited the test plan for this revision. (Show Details)
np added reviewers: imp, rstone, ngie, glebius, gnn, jhb.
np added a comment.Mar 2 2017, 2:42 AM

My plan is to commit src.conf.5 in a separate commit (after commiting the rest and then regenerating src.conf.5).

bdrewery added inline comments.Mar 2 2017, 2:46 AM
usr.sbin/cxgbetool/Makefile.depend
4 ↗(On Diff #25857)

Thanks for generating this.

Could you also connect cxgbetool to the build as well for DIRDEPS? It doesn't use the usr.sbin/Makefile but uses the targets/pseudo/userland/Makefile.depend file (manual). You can see near the bottom how to handle optional directories.

emaste added inline comments.Mar 2 2017, 2:49 AM
share/mk/src.opts.mk
217–223 ↗(On Diff #25857)

I'm not sure how best to sort these sections, but it feels to me like this ought to be later in the file.

np updated this revision to Diff 25859.Mar 2 2017, 2:56 AM

Incorporate feedback from bdrewery.

ngie added a comment.Mar 2 2017, 3:09 AM

Thank you for doing this!

Is there a way that this could be done for cxgbtool as well?

share/mk/src.opts.mk
217–223 ↗(On Diff #25857)

Agreed.

219 ↗(On Diff #25859)

Could this be named CXGBE instead so similar logic could be applied to the drivers as needed?

usr.sbin/cxgbetool/Makefile
6 ↗(On Diff #25859)

Could you please use SRCTOP instead of using .CURDIR-relative paths? That will make the tool buildable with the same logic pre and post this change.

7 ↗(On Diff #25859)

This isn't necessary when moving the tool to usr.sbin/ as Makefile.inc already defines BINDIR.

ngie added inline comments.Mar 2 2017, 3:16 AM
usr.sbin/cxgbetool/Makefile
4 ↗(On Diff #25859)

This is unnecessary (SRCS defaults to ${PROG}.c/${PROG_CXX}.cc)

np updated this revision to Diff 25860.Mar 2 2017, 3:17 AM

Incorporate some feedback from ngie@

  • Use SRCTOP instead of .CURDIR to locate headers.
  • Discard redundant BINDIR.
np updated this revision to Diff 25861.Mar 2 2017, 3:20 AM

Remove redundant SRCS line as suggested by ngie@

ngie accepted this revision.Mar 2 2017, 3:24 AM

My comment about MK_CXGBE isn't blocking, but I agree with emaste that MK_CXGBETOOL needs to be moved down in src.opts.mk .

This revision is now accepted and ready to land.Mar 2 2017, 3:24 AM
np added inline comments.Mar 2 2017, 3:41 AM
share/mk/src.opts.mk
217–223 ↗(On Diff #25857)

I coudn't think of an obvious place for this so I put it right after __T was available.

np added a comment.Mar 2 2017, 3:45 AM
In D9854#203328, @ngie wrote:

My comment about MK_CXGBE isn't blocking, but I agree with emaste that MK_CXGBETOOL needs to be moved down in src.opts.mk .

MK_CXGBE would be a confusing name for this knob given that it affects cxgbetool and not the cxgbe driver. I'd like to keep the MK_CXGBETOOL name.

np marked an inline comment as done.Mar 2 2017, 4:04 AM
np marked 2 inline comments as done.
gnn accepted this revision.Mar 2 2017, 7:02 AM
np updated this revision to Diff 25905.Mar 2 2017, 5:51 PM

Move the changes to src.opts.mk further down.

This revision now requires review to proceed.Mar 2 2017, 5:51 PM
np marked 2 inline comments as done.Mar 2 2017, 5:51 PM
bdrewery accepted this revision.Mar 2 2017, 8:51 PM
bdrewery added a reviewer: bdrewery.
This revision is now accepted and ready to land.Mar 2 2017, 8:51 PM
This revision was automatically updated to reflect the committed changes.