From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: ping -I eth1 .... Date: Wed, 17 Nov 2010 10:51:07 +0100 Message-ID: <1289987467.2687.15.camel@edumazet-laptop> References: <1288964206.2882.402.camel@edumazet-laptop> <20101105142510.GA14986@canuck.infradead.org> <1288967665.2882.522.camel@edumazet-laptop> <1288969614.2882.590.camel@edumazet-laptop> <20101105203150.GA12118@canuck.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Thomas Graf , netdev@vger.kernel.org To: Joakim Tjernlund Return-path: Received: from mail-wy0-f174.google.com ([74.125.82.174]:39701 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750736Ab0KQJvN (ORCPT ); Wed, 17 Nov 2010 04:51:13 -0500 Received: by wyb28 with SMTP id 28so1761275wyb.19 for ; Wed, 17 Nov 2010 01:51:11 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Le mercredi 17 novembre 2010 =C3=A0 10:29 +0100, Joakim Tjernlund a =C3= =A9crit : > Joakim Tjernlund/Transmode wrote on 2010/11/09 20:33:37: > > > > Joakim Tjernlund/Transmode wrote on 2010/11/06 10:42:46: > > > Thomas Graf wrote on 2010/11/05 21:31:50: > > > > > > > > On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrot= e: > > > > > Eric Dumazet wrote on 2010/11/05 16:= 06:54: > > > > > > > > > > > > > Hopefully most of that is legacy or just plain wrong? Unl= ess > > > > > > > someone can say why only test IFF_UP one should consider = changing them. > > > > > > > > > > > > > > > > > > > Most of the places are hot path. > > > > > > > > > > > > You dont want to replace one test by four tests. > > > > > > > > > > > > _This_ would be wrong :) > > > > > > > > > > Wrong is wrong, even if it is in the hot path :) > > > > > Perhaps it is time define and internal IFF_OPERATIONAL flag > > > > > which is the sum of IFF_UP, IFF_RUNNING etc.? Tht > > > > > way you still get one test in the hot path and can abstract > > > > > what defines an operational link. > > > > > > > > You definitely don't want to have your send() call fail simply = because > > > > the carrier was off for a few msec or the routing daemon has pu= t a link > > > > down temporarly. Also, the outgoing interface looked up at rout= ing > > > > decision is not necessarly the interface used for sending in th= e end. > > > > The packet may get mangled and rerouted by netfilter or tc on t= he way. > > > > > > But do you handle the case when the link is non operational for a= long time? > > > > > > > > > > > Personally I'm even ok with the current behaviour of sendto() w= hile the > > > > socket is bound to an interface but if we choose to return an e= rror > > > > if the interface is down we might as well do so based on the op= erational > > > > status. >=20 > > > Perhaps there is a better way. This all started when pppd hung be= cause > > > of ping -I , then someone pulled the cable for the= on the link. > > > > > > This is a strace where we have two ping -I, > > > ping -I p1-2-1-2-2 .. and ping -I p1-2-3-2-4 .. > > > Notice how pppd hangs for a long time in PPPIOCDETACH > > > As far as I can tell this is due to ping -I has claimed the ppp i= nterfaces > > > and doesn't noticed that the link is down. Ideally ping should re= ceive > > > a ENODEV as soon as pppd calls PPPIOCDETACH. > > > > > > 0.000908 write(0, "Connection terminated.\n", 23) =3D 23 > > > 0.000481 gettimeofday({1288952770, 566048}, NULL) =3D 0 > > > 0.001553 ioctl(7, PPPIOCDETACH > > > Message from syslogd@Brazil at Fri Nov 5 11:26:20 2010 ... > > > Brazil kernel: unregister_netdevice: waiting for p1-2-1-2-2 to be= come free. Usage count =3D 3 > > > Message from syslogd@Brazil at Fri Nov 5 11:26:20 2010 ... > > > Brazil kernel: unregister_netdevice: waiting for p1-2-3-2-4 to be= come free. Usage count =3D 3 > > > Message from syslogd@Brazil at Fri Nov 5 11:26:51 2010 ... > > > Brazil last message repeated 3 times > > > , 0xbfbc3398) =3D 0 > > > 66.559216 connect(9, {sa_family=3DAF_PPPOX, sa_data=3D"\0\0\0= \0\0\0\0\252\273\314\335\356hd"}, 30) =3D 0 > > > 0.000693 close(10) =3D 0 > > > 0.000449 close(7) =3D 0 > > > 0.009801 close(9) =3D 0 > > > > Any comment on this last strace? It is expected that ping -I should > > hold pppd hostage? > > >=20 > Ping? >=20 I thought I posted a patch, is there something else ? Could you please test with latest net-next-2.6 and following patch ? Thanks [PATCH net-next-2.6] ipv4: dont create a route if device is down ip_route_output_slow() should not create a route if device is down, so that we report -ENETUNREACH error to users. Reported-by: Nicolas Dichtel Reported-by: Joakim Tjernlund Signed-off-by: Eric Dumazet --- net/ipv4/route.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 66610ea..3cc4191 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2559,8 +2559,11 @@ static int ip_route_output_slow(struct net *net,= struct rtable **rp, goto out; =20 /* RACE: Check return value of inet_select_addr instead. */ - if (rcu_dereference(dev_out->ip_ptr) =3D=3D NULL) - goto out; /* Wrong error code */ + if (!(dev_out->flags & IFF_UP) || + rcu_dereference(dev_out->ip_ptr) =3D=3D NULL) { + err =3D -ENETUNREACH; + goto out; + } =20 if (ipv4_is_local_multicast(oldflp->fl4_dst) || ipv4_is_lbcast(oldflp->fl4_dst)) {