All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Xing <kerneljasonxing@gmail.com>
To: Eric Dumazet <edumazet@google.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, eric.dumazet@gmail.com
Subject: Re: [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device
Date: Fri, 29 Mar 2024 16:44:09 +0800	[thread overview]
Message-ID: <CAL+tcoDq5m5zK9G9t8v8zpRZEhp=BDwYft2jnC4pqwhS1RHPTA@mail.gmail.com> (raw)
In-Reply-To: <CANn89iLpY7iWppW1Vxze6Gf0ki5YFN9qF-w=+ig+=YfLqaLZyg@mail.gmail.com>

On Fri, Mar 29, 2024 at 2:31 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Mar 29, 2024 at 4:21 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > On Fri, Mar 29, 2024 at 1:07 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > If the device attached to the packet given to enqueue_to_backlog()
> > > is not running, we drop the packet.
> > >
> > > But we accidentally increase sd->dropped, giving false signals
> > > to admins: sd->dropped should be reserved to cpu backlog pressure,
> > > not to temporary glitches at device dismantles.
> >
> > It seems that drop action happening is intended in this case (see
> > commit e9e4dd3267d0c ("net: do not process device backlog during
> > unregistration")). We can see the strange/unexpected behaviour at
> > least through simply taking a look at /proc/net/softnet_stat file.
>
> I disagree.
>
> We are dismantling a device, temporary drops are expected, and this
> patch adds a more precise drop_reason.
>
> I have seen admins being worried about this counter being not zero on
> carefully tuned hosts.
>
> If you think we have to carry these drops forever in
> /proc/net/softnet_stat, you will have give
> a more precise reason than "This was intentionally added in 2015"
>
> e9e4dd3267d0c5234c changelog was very long, but said nothing
> about why sd->dropped _had_ to be updated.

Indeed, the author might think the skb is dropped in the RPS process
so it's necessary to record that and reflect these actions in this
proc file.

Don't get me wrong. I'm not against what you did, just proposing my
concern about changing the meaning/understanding of 'drop'.

Thanks,
Jason

  reply	other threads:[~2024-03-29  8:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 1/8] net: move kick_defer_list_purge() to net/core/dev.h Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 2/8] net: move dev_xmit_recursion() helpers " Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device Eric Dumazet
2024-03-29  3:21   ` Jason Xing
2024-03-29  6:31     ` Eric Dumazet
2024-03-29  8:44       ` Jason Xing [this message]
2024-03-28 17:03 ` [PATCH net-next 4/8] net: make softnet_data.dropped an atomic_t Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 5/8] net: enqueue_to_backlog() cleanup Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save() Eric Dumazet
2024-03-29 12:07   ` kernel test robot
2024-03-29 12:29   ` kernel test robot
2024-03-28 17:03 ` [PATCH net-next 7/8] net: rps: add rps_input_queue_head_add() helper Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 8/8] net: rps: move received_rps field to a better location Eric Dumazet
2024-03-29  5:45 ` [PATCH net-next 0/8] net: rps: misc changes Jakub Kicinski
2024-03-29  6:44   ` Eric Dumazet

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='CAL+tcoDq5m5zK9G9t8v8zpRZEhp=BDwYft2jnC4pqwhS1RHPTA@mail.gmail.com' \
    --to=kerneljasonxing@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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.