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
next prev parent 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 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.