Page MenuHomeFreeBSD

bnxt: Use correct firmware ioctl to get number of Rx/Tx queues supported by firmware
ClosedPublic

Authored by bhargava.marreddy_broadcom.com on Aug 26 2017, 11:30 AM.

Details

Summary
  1. Based on the suggestion from firmware team, derive scctx->isc_ntxqsets_max & scctx->isc_nrxqsets_max based on FUNC_QCFG (instead of FUNC_QCAPS).
  2. Bump-up driver version to "1.0.0.2".

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

shurd requested changes to this revision.Aug 26 2017, 4:40 PM
shurd added inline comments.
sys/dev/bnxt/bnxt.h
541 ↗(On Diff #32405)

If only four of these values are being used, they don't all need to be in the softc.

sys/dev/bnxt/bnxt_hwrm.c
486 ↗(On Diff #32405)

All of these values except the MACs and other network fields need to be converted to host byte order.

This revision now requires changes to proceed.Aug 26 2017, 4:40 PM
sys/dev/bnxt/if_bnxt.c
779 ↗(On Diff #32405)

Why is making the max TX and max RX the same value desirable? Users can request that TX and RX have different nrxqs via the dev.bnxt.X..iflib.override_qs_enable sysctl.

If a VF is allocated a small number of one ring type, but a large number of the other, this will result in not being able to use the allocated resources.

sys/dev/bnxt/bnxt.h
541 ↗(On Diff #32405)

Thought of using them in future for display / other way.
Sure!! I'll delete unused ones now .

sys/dev/bnxt/bnxt_hwrm.c
486 ↗(On Diff #32405)

Thanks, I'll fix it!!

sys/dev/bnxt/if_bnxt.c
779 ↗(On Diff #32405)

Only reason based on my experience with other 10G/40G drivers is - having same number of Rx & Tx queues works well for features like "Queue pairing" etc..
Thanks for pointing me to dev.bnxt.X..iflib.override_qs_enable, Can I remove that 'same number' restriction?

sys/dev/bnxt/if_bnxt.c
779 ↗(On Diff #32405)

By default, iflib will allocate equal numbers of queues. This variable is intended to be the max supported by the hardware, not the suggested value.

bhargava.marreddy_broadcom.com edited edge metadata.
bhargava.marreddy_broadcom.com marked an inline comment as done.

Thanks Stephen!! Taken care of your review comments.

This revision is now accepted and ready to land.Aug 27 2017, 4:25 PM

Sean Bruno,

If there are no further review comments, can you please commit this change,

Thanks,
Chenna.

sys/dev/bnxt/if_bnxt.c
779 ↗(On Diff #32405)

Understood!! I'll do the change.

You should update the summary to remove item #2 so it's useful as a commit message.

You should update the summary to remove item #2 so it's useful as a commit message.

<Chenna> Done!!

Sean Bruno,

If there are no further review comments, can you please commit this change,

This revision was automatically updated to reflect the committed changes.