netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2 2/3] netconsole: pr_err() when netpoll_setup fails
Date: Thu, 22 Aug 2024 03:01:25 -0700	[thread overview]
Message-ID: <ZscMdc6wmUGlusM4@gmail.com> (raw)
In-Reply-To: <20240821155404.5fc89ff6@kernel.org>

Hello Jakub,

On Wed, Aug 21, 2024 at 03:54:04PM -0700, Jakub Kicinski wrote:
> On Wed, 21 Aug 2024 01:41:55 -0700 Breno Leitao wrote:

> > Do you think this is useless?
> 
> I think it's better to push up more precise message into the fail sites.

Makese sense, I will remove it, and add the failing message once we
refactor ndo_netpoll_setup() callbacks.

> > Would it be better if the hot path just get one of the skbs from the
> > pool, and refill it in a workqueue? If the skb_poll() is empty, then
> > alloc_skb(len, GFP_ATOMIC) !?
> 
> Yeah, that seems a bit odd. If you can't find anything in the history
> that would explain this design - refactoring SG.

Thanks. I will add it to my todo list.

> > 2) Report statistic back from netpoll_send_udp(). netpoll_send_skb()
> > return values are being discarded, so, it is hard to know if the packet
> > was transmitted or got something as NET_XMIT_DROP, NETDEV_TX_BUSY,
> > NETDEV_TX_OK.
> > 
> > It is unclear where this should be reported two. Maybe a configfs entry?
> 
> Also sounds good. We don't use configfs much in networking so IDK if
> it's okay to use it for stats. But no other obviously better place
> comes to mind for me.

Exactly, configfs seems a bit weird, but, at the same time, I don't have
a better idea. Let me send a patch for this one, and we can continue the
discussion over there.

Thanks
--breno

  reply	other threads:[~2024-08-22 10:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19 10:36 [PATCH net-next v2 0/3] netconsole: Populate dynamic entry even if netpoll fails Breno Leitao
2024-08-19 10:36 ` [PATCH net-next v2 1/3] netpoll: Ensure clean state on setup failures Breno Leitao
2024-08-20 23:20   ` Jakub Kicinski
2024-08-21  8:44     ` Breno Leitao
2024-08-19 10:36 ` [PATCH net-next v2 2/3] netconsole: pr_err() when netpoll_setup fails Breno Leitao
2024-08-20 23:24   ` Jakub Kicinski
2024-08-21  8:41     ` Breno Leitao
2024-08-21 22:54       ` Jakub Kicinski
2024-08-22 10:01         ` Breno Leitao [this message]
2024-08-19 10:36 ` [PATCH net-next v2 3/3] netconsole: Populate dynamic entry even if netpoll fails Breno Leitao
2024-08-20 23:27   ` Jakub Kicinski
2024-08-21  8:21     ` Breno Leitao
2024-08-21 22:49       ` Jakub Kicinski
2024-08-22 11:00         ` Breno Leitao

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=ZscMdc6wmUGlusM4@gmail.com \
    --to=leitao@debian.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.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 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).