Page MenuHomeFreeBSD

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 - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

np retitled this revision from to Add cxgbetool(8) to the base system..
np updated this object.
np edited the test plan for this revision. (Show Details)
np added reviewers: imp, rstone, ngie, glebius, gnn, jhb.

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

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.

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 edited edge metadata.

Incorporate feedback from bdrewery.

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.

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

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

np edited edge metadata.

Incorporate some feedback from ngie@

  • Use SRCTOP instead of .CURDIR to locate headers.
  • Discard redundant BINDIR.
np edited edge metadata.

Remove redundant SRCS line as suggested by ngie@

ngie edited edge metadata.

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

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 edited edge metadata.
np edited edge metadata.

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