[PATCH net] nfp: move indirect block cleanup to flower app stop callback

[PATCH net] nfp: move indirect block cleanup to flower app stop callback

[Simon Horman]

The indirect block cleanup may cause control messages to be sent
if offloaded flows are present. However, by the time the flower app
cleanup callback is called txbufs are no longer available and attempts
to send control messages result in a NULL-pointer dereference in
nfp_ctrl_tx_one().

This problem may be resolved by moving the indirect block cleanup
to the stop callback, where txbufs are still available.

As suggested by Jakub Kicinski and Louis Peens.

Fixes: a1db217861f3 ("net: flow_offload: fix flow_indr_dev_unregister path")
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Louis Peens <louis.peens@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index bb448c82cdc2..c029950a81e2 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -860,9 +860,6 @@ static void nfp_flower_clean(struct nfp_app *app)
 	skb_queue_purge(&app_priv->cmsg_skbs_low);
 	flush_work(&app_priv->cmsg_work);
 
-	flow_indr_dev_unregister(nfp_flower_indr_setup_tc_cb, app,
-				 nfp_flower_setup_indr_tc_release);
-
 	if (app_priv->flower_ext_feats & NFP_FL_FEATS_VF_RLIM)
 		nfp_flower_qos_cleanup(app);
 
@@ -951,6 +948,9 @@ static int nfp_flower_start(struct nfp_app *app)
 static void nfp_flower_stop(struct nfp_app *app)
 {
 	nfp_tunnel_config_stop(app);
+
+	flow_indr_dev_unregister(nfp_flower_indr_setup_tc_cb, app,
+				 nfp_flower_setup_indr_tc_release);
 }
 
 static int
-- 
2.20.1

Re: [PATCH net] nfp: move indirect block cleanup to flower app stop callback

[Jakub Kicinski]

On Wed, 16 Dec 2020 15:57:01 +0100 Simon Horman wrote:
> The indirect block cleanup may cause control messages to be sent
> if offloaded flows are present. However, by the time the flower app
> cleanup callback is called txbufs are no longer available and attempts
> to send control messages result in a NULL-pointer dereference in
> nfp_ctrl_tx_one().
> 
> This problem may be resolved by moving the indirect block cleanup
> to the stop callback, where txbufs are still available.
> 
> As suggested by Jakub Kicinski and Louis Peens.
> 
> Fixes: a1db217861f3 ("net: flow_offload: fix flow_indr_dev_unregister path")
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> Signed-off-by: Louis Peens <louis.peens@netronome.com>

Applied, thank you!