All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Wei Fang <wei.fang@nxp.com>
Cc: Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	"ast@kernel.org" <ast@kernel.org>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>,
	"hawk@kernel.org" <hawk@kernel.org>,
	"john.fastabend@gmail.com" <john.fastabend@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"imx@lists.linux.dev" <imx@lists.linux.dev>
Subject: Re: [PATCH net 3/3] net: enetc: reset xdp_tx_in_flight when updating bpf program
Date: Fri, 20 Sep 2024 17:25:11 +0300	[thread overview]
Message-ID: <20240920142511.aph5wpmiczcsxfgr@skbuf> (raw)
In-Reply-To: <PAXPR04MB85101DE84124D424264BB4FD886C2@PAXPR04MB8510.eurprd04.prod.outlook.com>

On Fri, Sep 20, 2024 at 05:05:14PM +0300, Wei Fang wrote:
> > zero init is good but shouldn't you be draining these buffers when removing
> > XDP resources at least? what happens with DMA mappings that are related to
> > these cached buffers?
> > 
> 
> All the buffers will be freed and DMA will be unmapped when XDP program is
> installed.

There is still a problem with the patch you proposed here, which is that
enetc_reconfigure() has one more call site, from enetc_hwtstamp_set().
If enetc_free_rxtx_rings() is the one that gets rid of the stale
buffers, it should also be the one that resets xdp_tx_in_flight,
otherwise you will still leave the problem unsolved where XDP_TX can be
interrupted by a change in hwtstamping state, and the software "in flight"
counter gets out of sync with the ring state.

Also, I suspect that the blamed commit is wrong. Also the normal netdev
close path should be susceptible to this issue, not just enetc_reconfigure().
Maybe something like ff58fda09096 ("net: enetc: prioritize ability to go
down over packet processing"). That's when we started rushing the NAPI
poll routing to finish. I don't think it was possible, before that, to
close the netdev while there were XDP_TX frames pending to be recycled.

> I am thinking that another solution may be better, which is mentioned
> in another thread replying to Vladimir, so that xdp_tx_in_flight will naturally drop
> to 0, and the TX-related statistics will be more accurate.

Please give me some more time to analyze the flow after just your patch 2/3.
I have a draft reply, but I would still like to test some things.

  reply	other threads:[~2024-09-20 14:25 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-19  8:41 [PATCH net 0/3] net: enetc: fix some issues of XDP Wei Fang
2024-09-19  8:41 ` [PATCH net 1/3] net: enetc: remove xdp_drops statistic from enetc_xdp_drop() Wei Fang
2024-09-19  8:57   ` kernel test robot
2024-09-20 12:08   ` Maciej Fijalkowski
2024-09-23  5:02   ` Ratheesh Kannoth
2024-09-23  5:53     ` Wei Fang
2024-09-27 14:25       ` Vladimir Oltean
2024-09-19  8:41 ` [PATCH net 2/3] net: enetc: fix the issues of XDP_REDIRECT feature Wei Fang
2024-09-20 12:50   ` Maciej Fijalkowski
2024-09-27 14:41   ` Vladimir Oltean
2024-09-29  1:35     ` Wei Fang
2024-09-19  8:41 ` [PATCH net 3/3] net: enetc: reset xdp_tx_in_flight when updating bpf program Wei Fang
2024-09-19 13:21   ` Vladimir Oltean
2024-09-20  3:12     ` Wei Fang
2024-09-23  4:56       ` Ratheesh Kannoth
2024-09-23  5:48         ` Wei Fang
2024-09-20 13:04   ` Maciej Fijalkowski
2024-09-20 14:05     ` Wei Fang
2024-09-20 14:25       ` Vladimir Oltean [this message]
2024-09-20 14:29         ` Vladimir Oltean
2024-09-23  1:59         ` Wei Fang
2024-09-27 14:33           ` Vladimir Oltean
2024-09-29  1:31             ` Wei Fang
2024-09-27 14:38   ` Vladimir Oltean
  -- strict thread matches above, loose matches on Subject: below --
2024-09-19 11:12 Subbaraya Sundeep

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=20240920142511.aph5wpmiczcsxfgr@skbuf \
    --to=vladimir.oltean@nxp.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=claudiu.manoil@nxp.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=wei.fang@nxp.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.