netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Horman <simon.horman@netronome.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: David Miller <davem@davemloft.net>,
	Louis Peens <louis.peens@netronome.com>,
	netdev@vger.kernel.org, oss-drivers@netronome.com,
	wenxu <wenxu@ucloud.cn>
Subject: Re: [PATCH net] nfp: do not send control messages during cleanup
Date: Tue, 15 Dec 2020 11:22:51 +0100	[thread overview]
Message-ID: <20201215102249.GA5855@netronome.com> (raw)
In-Reply-To: <20201214182650.4d03cc7c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

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.

      reply	other threads:[~2020-12-15 10:23 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-11  9:27 [PATCH net] nfp: do not send control messages during cleanup Simon Horman
2020-12-15  2:26 ` Jakub Kicinski
2020-12-15 10:22   ` Simon Horman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201215102249.GA5855@netronome.com \
    --to=simon.horman@netronome.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=louis.peens@netronome.com \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@netronome.com \
    --cc=wenxu@ucloud.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).