[PATCH net] nfp: do not send control messages during cleanup

[PATCH net] nfp: do not send control messages during cleanup

[Simon Horman]

On cleanup the txbufs are freed before app cleanup. But app clean-up may
result in control messages due to use of common control paths. There is no
need to clean-up the NIC in such cases so simply discard requests. Without
such a check a NULL pointer dereference occurs.

Fixes: a1db217861f3 ("net: flow_offload: fix flow_indr_dev_unregister path")
Cc: wenxu <wenxu@ucloud.cn>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Louis Peens <louis.peens@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index b4acf2f41e84..d86f68aa89bf 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -2084,6 +2084,15 @@ nfp_ctrl_tx_one(struct nfp_net *nn, struct nfp_net_r_vector *r_vec,
 	dp = &r_vec->nfp_net->dp;
 	tx_ring = r_vec->tx_ring;
 
+	if (!tx_ring->txbufs)
+		/* On cleanup the txbufs are freed before app cleanup.
+		 * But app clean-up may result in control messages due to
+		 * use of common control paths. There is no need to
+		 * clean-up the NIC in such cases so simply discard
+		 * requests.
+		 */
+		goto err_free;
+
 	if (WARN_ON_ONCE(skb_shinfo(skb)->nr_frags)) {
 		nn_dp_warn(dp, "Driver's CTRL TX does not implement gather\n");
 		goto err_free;
-- 
2.20.1

Re: [PATCH net] nfp: do not send control messages during cleanup

[Jakub Kicinski]

On Fri, 11 Dec 2020 10:27:38 +0100 Simon Horman wrote:
> On cleanup the txbufs are freed before app cleanup. But app clean-up may
> result in control messages due to use of common control paths. There is no
> need to clean-up the NIC in such cases so simply discard requests. Without
> such a check a NULL pointer dereference occurs.
> 
> Fixes: a1db217861f3 ("net: flow_offload: fix flow_indr_dev_unregister path")
> Cc: wenxu <wenxu@ucloud.cn>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> Signed-off-by: Louis Peens <louis.peens@netronome.com>

Hm. We can apply this as a quick fix, but I'd think that app->stop
(IIRC that's the callback) is responsible for making sure that
everything gets shut down and no more cmsgs can be generated after
ctrl vNIC goes down. Perhaps some code needs to be reshuffled between
init/clean and start/stop for flower? WDYT?

Re: [PATCH net] nfp: do not send control messages during cleanup

[Simon Horman]

On Mon, Dec 14, 2020 at 06:26:50PM -0800, Jakub Kicinski wrote:
> On Fri, 11 Dec 2020 10:27:38 +0100 Simon Horman wrote:
> > On cleanup the txbufs are freed before app cleanup. But app clean-up may
> > result in control messages due to use of common control paths. There is no
> > need to clean-up the NIC in such cases so simply discard requests. Without
> > such a check a NULL pointer dereference occurs.
> > 
> > Fixes: a1db217861f3 ("net: flow_offload: fix flow_indr_dev_unregister path")
> > Cc: wenxu <wenxu@ucloud.cn>
> > Signed-off-by: Simon Horman <simon.horman@netronome.com>
> > Signed-off-by: Louis Peens <louis.peens@netronome.com>
> 
> Hm. We can apply this as a quick fix, but I'd think that app->stop
> (IIRC that's the callback) is responsible for making sure that
> everything gets shut down and no more cmsgs can be generated after
> ctrl vNIC goes down. Perhaps some code needs to be reshuffled between
> init/clean and start/stop for flower? WDYT?

Thanks Jakub,

I was a bit concerned with fragility in the clean-up path, which is why I
had opted for this simple solution. However, looking at your suggestion
above it seems simple to move the cleanup to app->stop. I'll work on
posting a patch to implement your suggestion.