All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org,
	Stephen Hemminger <stephen@networkplumber.org>,
	David Ahern <dsahern@gmail.com>,
	brouer@redhat.com
Subject: Re: [PATCH net-next V2] net: sched: fallback to qdisc noqueue if default qdisc setup fail
Date: Fri, 1 May 2020 13:56:02 +0200	[thread overview]
Message-ID: <20200501135602.0671c73d@carbon> (raw)
In-Reply-To: <20200430124549.3272afb1@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Thu, 30 Apr 2020 12:45:49 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu, 30 Apr 2020 13:42:22 +0200 Jesper Dangaard Brouer wrote:
> > Currently if the default qdisc setup/init fails, the device ends up with
> > qdisc "noop", which causes all TX packets to get dropped.
> > 
> > With the introduction of sysctl net/core/default_qdisc it is possible
> > to change the default qdisc to be more advanced, which opens for the
> > possibility that Qdisc_ops->init() can fail.
> > 
> > This patch detect these kind of failures, and choose to fallback to
> > qdisc "noqueue", which is so simple that its init call will not fail.
> > This allows the interface to continue functioning.
> > 
> > V2:
> > As this also captures memory failures, which are transient, the
> > device is not kept in IFF_NO_QUEUE state.  This allows the net_device
> > to retry to default qdisc assignment.
> > 
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>  
> 
> I have mixed feelings about this one, I wonder if I'm the only one.
> Seems like failure to allocate the default qdisc is pretty critical,
> the log message may be missed, especially in the boot time noise.
> 
> I think a WARN_ON() is in order here, I'd personally just replace the
> netdev_info with a WARN_ON, without the fallback.

It is good that we agree that failure to default qdisc is pretty
critical.  I guess we disagree on whether (1) we keep network
functioning in a degraded state, (2) drop all packets on net_device
such that people notice.

This change propose (1) keeping the box functioning.  For me it was a
pretty bad experience, that when I pushed a new kernel over the network
to my embedded box, then I lost all network connectivity.  I
fortunately had serial console access (as this was not an OpenWRT box
but a full devel board) so I could debug, but I could no-longer upgrade
the kernel.  I clearly noticed, as the box was not operational, but I
guess most people would just give up at this point. (Imagine a small
OpenWRT box config setting default_qdisc to fq_codel, which brick the
box as it cannot allocate memory).

I hope that people will notice this degrade state, when they start to
transfer data to the device.  Because running 'noqueue' on a physical
device will result in net_crit_ratelimited() messages below:

 [86971.609318] Virtual device eth0 asks to queue packet!
 [86971.622183] Virtual device eth0 asks to queue packet!
 [86971.627510] Virtual device eth0 asks to queue packet!

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


  reply	other threads:[~2020-05-01 11:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30 11:42 [PATCH net-next V2] net: sched: fallback to qdisc noqueue if default qdisc setup fail Jesper Dangaard Brouer
2020-04-30 19:45 ` Jakub Kicinski
2020-05-01 11:56   ` Jesper Dangaard Brouer [this message]
2020-05-01 19:01     ` Jakub Kicinski
2020-05-04 18:51 ` David Miller

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=20200501135602.0671c73d@carbon \
    --to=brouer@redhat.com \
    --cc=dsahern@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    /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.