| Message ID | 20240103102332.3642417-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bnxt: fix building without CONFIG_RFS_ACCEL | expand |
On Wed, Jan 03, 2024 at 11:23:11AM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > A recent patch series generalized the filter logic in bnxt to no > longer just be used for RFS, but it now fails to build when RFS_ACCEL > is disabled: > > drivers/net/ethernet/broadcom/bnxt/bnxt.c: In function 'bnxt_cfg_ntp_filters': > drivers/net/ethernet/broadcom/bnxt/bnxt.c:14077:37: error: implicit declaration of function 'rps_may_expire_flow' [-Werror=implicit-function-declaration] > 14077 | if (rps_may_expire_flow(bp->dev, fltr->base.rxq, > | ^~~~~~~~~~~~~~~~~~~ > > Add back one #ifdef check around a call to the missing rps_may_expire_flow() > function. > > Fixes: 59cde76f33fa ("bnxt_en: Refactor filter insertion logic in bnxt_rx_flow_steer().") > Cc: Michael Chan <michael.chan@broadcom.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > I don't know if this is a correct fix, only checked that it is plausible > and that it does address the build failure. If a different fix is needed, > please just treat this as a bug report. Are you using a kernel config with CONFIG_SMP=n ? That was how I was able to reproduce this. There is a good oppportunity to clean this up a little better. When CONFIG_RFS_ACCEL is not set there is no reason to even have bnxt_cfg_ntp_filters included in the driver build. I'll talk to Michael and we will post a fix for this. > --- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > index 827821e89c40..83a97c65b728 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > @@ -14074,6 +14074,7 @@ static void bnxt_cfg_ntp_filters(struct bnxt *bp) > if (test_bit(BNXT_FLTR_VALID, &fltr->base.state)) { > if (fltr->base.flags & BNXT_ACT_NO_AGING) > continue; > +#if IS_ENABLED(CONFIG_RFS_ACCEL) > if (rps_may_expire_flow(bp->dev, fltr->base.rxq, > fltr->flow_id, > fltr->base.sw_id)) { > @@ -14081,6 +14082,7 @@ static void bnxt_cfg_ntp_filters(struct bnxt *bp) > fltr); > del = true; > } > +#endif > } else { > rc = bnxt_hwrm_cfa_ntuple_filter_alloc(bp, > fltr); > -- > 2.39.2 >
On Wed, Jan 3, 2024, at 16:46, Andy Gospodarek wrote: > On Wed, Jan 03, 2024 at 11:23:11AM +0100, Arnd Bergmann wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> A recent patch series generalized the filter logic in bnxt to no >> longer just be used for RFS, but it now fails to build when RFS_ACCEL >> is disabled: >> >> drivers/net/ethernet/broadcom/bnxt/bnxt.c: In function 'bnxt_cfg_ntp_filters': >> drivers/net/ethernet/broadcom/bnxt/bnxt.c:14077:37: error: implicit declaration of function 'rps_may_expire_flow' [-Werror=implicit-function-declaration] >> 14077 | if (rps_may_expire_flow(bp->dev, fltr->base.rxq, >> | ^~~~~~~~~~~~~~~~~~~ >> >> Add back one #ifdef check around a call to the missing rps_may_expire_flow() >> function. >> >> Fixes: 59cde76f33fa ("bnxt_en: Refactor filter insertion logic in bnxt_rx_flow_steer().") >> Cc: Michael Chan <michael.chan@broadcom.com> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> I don't know if this is a correct fix, only checked that it is plausible >> and that it does address the build failure. If a different fix is needed, >> please just treat this as a bug report. > > Are you using a kernel config with CONFIG_SMP=n ? That was how I was > able to reproduce this. I saw this in two randconfig test builds on 32-bit arm, both with SMP disabled, though I did not expect that to make a difference. In case it helps, this is one of the two config files: https://pastebin.com/raw/uxHEXzG2 > There is a good oppportunity to clean this up a little better. When > CONFIG_RFS_ACCEL is not set there is no reason to even have > bnxt_cfg_ntp_filters included in the driver build. > > I'll talk to Michael and we will post a fix for this. Ok, thanks! Arnd
On Wed, Jan 3, 2024 at 8:01 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Jan 3, 2024, at 16:46, Andy Gospodarek wrote: > > There is a good oppportunity to clean this up a little better. When > > CONFIG_RFS_ACCEL is not set there is no reason to even have > > bnxt_cfg_ntp_filters included in the driver build. > > > > I'll talk to Michael and we will post a fix for this. > > Ok, thanks! Yes, we can clean this up better. User configured ntuple filters are directly added and don't go through bnxt_cfg_ntp_filters() by way of the workqueue. I'll post the fix later today. Thanks.
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 827821e89c40..83a97c65b728 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -14074,6 +14074,7 @@ static void bnxt_cfg_ntp_filters(struct bnxt *bp) if (test_bit(BNXT_FLTR_VALID, &fltr->base.state)) { if (fltr->base.flags & BNXT_ACT_NO_AGING) continue; +#if IS_ENABLED(CONFIG_RFS_ACCEL) if (rps_may_expire_flow(bp->dev, fltr->base.rxq, fltr->flow_id, fltr->base.sw_id)) { @@ -14081,6 +14082,7 @@ static void bnxt_cfg_ntp_filters(struct bnxt *bp) fltr); del = true; } +#endif } else { rc = bnxt_hwrm_cfa_ntuple_filter_alloc(bp, fltr);