ping -I eth1 ....

ping -I eth1 ....

[Joakim Tjernlund]

If I do busybox ping -I eth1 x.x.y.y i see
socket(PF_INET, SOCK_RAW, IPPROTO_ICMP) = 3
getuid()                                = 0
setuid(0)                               = 0
setsockopt(3, SOL_SOCKET, SO_BINDTODEVICE, "eth1\0\0\0\0\0\0\0\0\0\0\0\0\20\0[l\0\0\0\2\20\7\354(\0"..., 32) = 0
setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0
setsockopt(3, SOL_SOCKET, SO_RCVBUF, [7280], 4) = 0

......
then this in a loop:
--- SIGALRM (Alarm clock) @ 0 (0) ---
clock_gettime(CLOCK_MONOTONIC, {1238, 116470107}) = 0
sendto(3, "\10\0\203\345\1\326\0\2I\314(v\0\0\0\0\0\0\0\0\0\0\0\0"..., 64, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("1.1.1.1")}, 16) = 64
rt_sigaction(SIGALRM, {0x10017034, [ALRM], SA_RESTART}, {0x10017034, [ALRM], SA_RESTART}, 8) = 0
alarm(1)                                = 0
sigreturn()                             = ? (mask now [ILL ABRT FPE KILL ALRM STKFLT CONT STOP TTOU URG XCPU XFSZ VTALRM PROF WINCH IO PWR RTMIN])
recvfrom(3, 0xbfe6a620, 192, 0, 0xbfe6a6f0, 0xbfe6a70c) = ? ERESTARTSYS (To be restarted)

Then I do ifconfig eth1 down
and I still see the same. Should not
sendto and/or revcfrom return some error as
the interface is down?

kernel 2.6.35

    Jocke

Re: ping -I eth1 ....

[Eric Dumazet]

Le vendredi 05 novembre 2010 à 14:14 +0100, Joakim Tjernlund a écrit :
> If I do busybox ping -I eth1 x.x.y.y i see
> socket(PF_INET, SOCK_RAW, IPPROTO_ICMP) = 3
> getuid()                                = 0
> setuid(0)                               = 0
> setsockopt(3, SOL_SOCKET, SO_BINDTODEVICE, "eth1\0\0\0\0\0\0\0\0\0\0\0\0\20\0[l\0\0\0\2\20\7\354(\0"..., 32) = 0
> setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0
> setsockopt(3, SOL_SOCKET, SO_RCVBUF, [7280], 4) = 0
> 
> ......
> then this in a loop:
> --- SIGALRM (Alarm clock) @ 0 (0) ---
> clock_gettime(CLOCK_MONOTONIC, {1238, 116470107}) = 0
> sendto(3, "\10\0\203\345\1\326\0\2I\314(v\0\0\0\0\0\0\0\0\0\0\0\0"..., 64, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("1.1.1.1")}, 16) = 64
> rt_sigaction(SIGALRM, {0x10017034, [ALRM], SA_RESTART}, {0x10017034, [ALRM], SA_RESTART}, 8) = 0
> alarm(1)                                = 0
> sigreturn()                             = ? (mask now [ILL ABRT FPE KILL ALRM STKFLT CONT STOP TTOU URG XCPU XFSZ VTALRM PROF WINCH IO PWR RTMIN])
> recvfrom(3, 0xbfe6a620, 192, 0, 0xbfe6a6f0, 0xbfe6a70c) = ? ERESTARTSYS (To be restarted)
> 
> Then I do ifconfig eth1 down
> and I still see the same. Should not
> sendto and/or revcfrom return some error as
> the interface is down?

Hmm, this reminds me one patch, yes...

Search for " ipv4: remove all rt cache entries on UNREGISTER event"

http://permalink.gmane.org/gmane.linux.network/173391

I guess I should respin it for net-next-2.6

Re: ping -I eth1 ....

[Joakim Tjernlund]

Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 14:36:46:
>
> Le vendredi 05 novembre 2010 à 14:14 +0100, Joakim Tjernlund a écrit :
> > If I do busybox ping -I eth1 x.x.y.y i see
> > socket(PF_INET, SOCK_RAW, IPPROTO_ICMP) = 3
> > getuid()                                = 0
> > setuid(0)                               = 0
> > setsockopt(3, SOL_SOCKET, SO_BINDTODEVICE, "eth1\0\0\0\0\0\0\0\0\0\0\0\0\20\0[l\0\0\0\2\20\7\354(\0"..., 32) = 0
> > setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0
> > setsockopt(3, SOL_SOCKET, SO_RCVBUF, [7280], 4) = 0
> >
> > ......
> > then this in a loop:
> > --- SIGALRM (Alarm clock) @ 0 (0) ---
> > clock_gettime(CLOCK_MONOTONIC, {1238, 116470107}) = 0
> > sendto(3, "\10\0\203\345\1\326\0\2I\314(v\0\0\0\0\0\0\0\0\0\0\0\0"..., 64, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("1.1.1.1")}, 16) = 64
> > rt_sigaction(SIGALRM, {0x10017034, [ALRM], SA_RESTART}, {0x10017034, [ALRM], SA_RESTART}, 8) = 0
> > alarm(1)                                = 0
> > sigreturn()                             = ? (mask now [ILL ABRT FPE KILL ALRM STKFLT CONT STOP TTOU URG XCPU XFSZ VTALRM PROF WINCH IO PWR RTMIN])
> > recvfrom(3, 0xbfe6a620, 192, 0, 0xbfe6a6f0, 0xbfe6a70c) = ? ERESTARTSYS (To be restarted)
> >
> > Then I do ifconfig eth1 down
> > and I still see the same. Should not
> > sendto and/or revcfrom return some error as
> > the interface is down?
>
> Hmm, this reminds me one patch, yes...
>
> Search for " ipv4: remove all rt cache entries on UNREGISTER event"
>
> http://permalink.gmane.org/gmane.linux.network/173391

Ah, that does the trick. A few comments though:

1) I think you should include IFF_RUNNING too

2) This only affects the TX(sendtoo) path?
   The RX(recvfrom) should also get an error.

   Jocke

>
> I guess I should respin it for net-next-2.6

Yes, pretty please :)

Re: ping -I eth1 ....

[Thomas Graf]

On Fri, Nov 05, 2010 at 03:01:57PM +0100, Joakim Tjernlund wrote:
> > > Then I do ifconfig eth1 down
> > > and I still see the same. Should not
> > > sendto and/or revcfrom return some error as
> > > the interface is down?
> >
> > Hmm, this reminds me one patch, yes...
> >
> > Search for " ipv4: remove all rt cache entries on UNREGISTER event"
> >
> > http://permalink.gmane.org/gmane.linux.network/173391
> 
> Ah, that does the trick. A few comments though:
> 
> 1) I think you should include IFF_RUNNING too

Probably even better to base it on the operational state of the link

netif_running() && netif_oper_up() && netif_carrier_ok() && !netif_dormant()

Re: ping -I eth1 ....

[Eric Dumazet]

Le vendredi 05 novembre 2010 à 10:25 -0400, Thomas Graf a écrit :
> On Fri, Nov 05, 2010 at 03:01:57PM +0100, Joakim Tjernlund wrote:
> > > > Then I do ifconfig eth1 down
> > > > and I still see the same. Should not
> > > > sendto and/or revcfrom return some error as
> > > > the interface is down?
> > >
> > > Hmm, this reminds me one patch, yes...
> > >
> > > Search for " ipv4: remove all rt cache entries on UNREGISTER event"
> > >
> > > http://permalink.gmane.org/gmane.linux.network/173391
> > 
> > Ah, that does the trick. A few comments though:
> > 
> > 1) I think you should include IFF_RUNNING too
> 
> Probably even better to base it on the operational state of the link
> 
> netif_running() && netif_oper_up() && netif_carrier_ok() && !netif_dormant()

At this point we setup a route.

Is a change of any of this status going to flush/cancel a previously
setup route ?

There must be a reason why in many places we only test (dev->flags &
IFF_UP), and _never_ netif_oper_up() (only in dev_get_flags() to export
it at userspace) 

Re: ping -I eth1 ....

[Thomas Graf]

On Fri, Nov 05, 2010 at 03:34:25PM +0100, Eric Dumazet wrote:
> Le vendredi 05 novembre 2010 à 10:25 -0400, Thomas Graf a écrit :
> > On Fri, Nov 05, 2010 at 03:01:57PM +0100, Joakim Tjernlund wrote:
> > > 1) I think you should include IFF_RUNNING too
> > 
> > Probably even better to base it on the operational state of the link
> > 
> > netif_running() && netif_oper_up() && netif_carrier_ok() && !netif_dormant()
> 
> At this point we setup a route.
> 
> Is a change of any of this status going to flush/cancel a previously
> setup route ?

No it won't. 

> There must be a reason why in many places we only test (dev->flags &
> IFF_UP), and _never_ netif_oper_up() (only in dev_get_flags() to export
> it at userspace) 

The reason is that while the device is not operational we queue all
packets in the qdisc and hope for the link to become operational in the
near future. This does make sense, especially if the link was once
operational.

This case is special however, the interface is specified explicitely
and expectes to be notified immediately if that is not possible.

Re: ping -I eth1 ....

[Joakim Tjernlund]

Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 15:53:21:
>
> On Fri, Nov 05, 2010 at 03:34:25PM +0100, Eric Dumazet wrote:
> > Le vendredi 05 novembre 2010 à 10:25 -0400, Thomas Graf a écrit :
> > > On Fri, Nov 05, 2010 at 03:01:57PM +0100, Joakim Tjernlund wrote:
> > > > 1) I think you should include IFF_RUNNING too
> > >
> > > Probably even better to base it on the operational state of the link
> > >
> > > netif_running() && netif_oper_up() && netif_carrier_ok() && !netif_dormant()
> >
> > At this point we setup a route.
> >
> > Is a change of any of this status going to flush/cancel a previously
> > setup route ?
>
> No it won't.
>
> > There must be a reason why in many places we only test (dev->flags &
> > IFF_UP), and _never_ netif_oper_up() (only in dev_get_flags() to export
> > it at userspace)
>
> The reason is that while the device is not operational we queue all
> packets in the qdisc and hope for the link to become operational in the
> near future. This does make sense, especially if the link was once
> operational.

That feels wrong. There is now way to know if it ever will be operational
again, let alone in the near future. Is there a timeout so that
if the link doesn't become operational, it will fail?

Re: ping -I eth1 ....

[Joakim Tjernlund]

Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 15:53:21:
>
> On Fri, Nov 05, 2010 at 03:34:25PM +0100, Eric Dumazet wrote:
> > Le vendredi 05 novembre 2010 à 10:25 -0400, Thomas Graf a écrit :
> > > On Fri, Nov 05, 2010 at 03:01:57PM +0100, Joakim Tjernlund wrote:
> > > > 1) I think you should include IFF_RUNNING too
> > >
> > > Probably even better to base it on the operational state of the link
> > >
> > > netif_running() && netif_oper_up() && netif_carrier_ok() && !netif_dormant()
> >
> > At this point we setup a route.
> >
> > Is a change of any of this status going to flush/cancel a previously
> > setup route ?
>
> No it won't.
>
> > There must be a reason why in many places we only test (dev->flags &
> > IFF_UP), and _never_ netif_oper_up() (only in dev_get_flags() to export
> > it at userspace)
>
> The reason is that while the device is not operational we queue all
> packets in the qdisc and hope for the link to become operational in the
> near future. This does make sense, especially if the link was once
> operational.

Hmm, perhaps one needs to treat user space differently from
internal kernel calls?

User space should always get an error if the link isn't functional.

Re: ping -I eth1 ....

[Joakim Tjernlund]

Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 15:34:25:
>
> Le vendredi 05 novembre 2010 à 10:25 -0400, Thomas Graf a écrit :
> > On Fri, Nov 05, 2010 at 03:01:57PM +0100, Joakim Tjernlund wrote:
> > > > > Then I do ifconfig eth1 down
> > > > > and I still see the same. Should not
> > > > > sendto and/or revcfrom return some error as
> > > > > the interface is down?
> > > >
> > > > Hmm, this reminds me one patch, yes...
> > > >
> > > > Search for " ipv4: remove all rt cache entries on UNREGISTER event"
> > > >
> > > > http://permalink.gmane.org/gmane.linux.network/173391
> > >
> > > Ah, that does the trick. A few comments though:
> > >
> > > 1) I think you should include IFF_RUNNING too
> >
> > Probably even better to base it on the operational state of the link
> >
> > netif_running() && netif_oper_up() && netif_carrier_ok() && !netif_dormant()
>
> At this point we setup a route.

It fixes my problem too so it is not only about setting up routes.
The ping -I eth1 I did didn't even have an IP address on eth1

Perhaps there is a better place to test the interface status?
Especially as the RX path should be considered too.

>
> Is a change of any of this status going to flush/cancel a previously
> setup route ?
>
> There must be a reason why in many places we only test (dev->flags &
> IFF_UP), and _never_ netif_oper_up() (only in dev_get_flags() to export
> it at userspace)

Hopefully most of that is legacy or just plain wrong? Unless
someone can say why only test IFF_UP one should consider changing them.

Re: ping -I eth1 ....

[Eric Dumazet]

Le vendredi 05 novembre 2010 à 15:57 +0100, Joakim Tjernlund a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 15:34:25:
> t be a reason why in many places we only test (dev->flags &
> > IFF_UP), and _never_ netif_oper_up() (only in dev_get_flags() to export
> > it at userspace)
> 
> Hopefully most of that is legacy or just plain wrong? Unless
> 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 :)

Re: ping -I eth1 ....

[Joakim Tjernlund]

Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
>
> Le vendredi 05 novembre 2010 à 15:57 +0100, Joakim Tjernlund a écrit :
> > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 15:34:25:
> > t be a reason why in many places we only test (dev->flags &
> > > IFF_UP), and _never_ netif_oper_up() (only in dev_get_flags() to export
> > > it at userspace)
> >
> > Hopefully most of that is legacy or just plain wrong? Unless
> > 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.? That
way you still get one test in the hot path and can abstract
what defines an operational link.

 Jocke

Re: ping -I eth1 ....

[Thomas Graf]

On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> >
> > > Hopefully most of that is legacy or just plain wrong? Unless
> > > 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 put a link
down temporarly. Also, the outgoing interface looked up at routing
decision is not necessarly the interface used for sending in the end.
The packet may get mangled and rerouted by netfilter or tc on the way.

Personally I'm even ok with the current behaviour of sendto() while the
socket is bound to an interface but if we choose to return an error
if the interface is down we might as well do so based on the operational
status.

Re: ping -I eth1 ....

[Joakim Tjernlund]

Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 21:31:50:
>
> On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> > >
> > > > Hopefully most of that is legacy or just plain wrong? Unless
> > > > 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 put a link
> down temporarly. Also, the outgoing interface looked up at routing
> decision is not necessarly the interface used for sending in the end.
> The packet may get mangled and rerouted by netfilter or tc on the 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() while the
> socket is bound to an interface but if we choose to return an error
> if the interface is down we might as well do so based on the operational
> status.

Perhaps there is a better way. This all started when pppd hung because
of ping -I <ppp interface>, 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 interfaces
and doesn't noticed that the link is down. Ideally ping should receive
a ENODEV as soon as pppd calls PPPIOCDETACH.


   0.000908 write(0, "Connection terminated.\n", 23) = 23
     0.000481 gettimeofday({1288952770, 566048}, NULL) = 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 become free. Usage count = 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 become free. Usage count = 3
Message from syslogd@Brazil at Fri Nov  5 11:26:51 2010 ...
Brazil last message repeated 3 times
, 0xbfbc3398) = 0
    66.559216 connect(9, {sa_family=AF_PPPOX, sa_data="\0\0\0\0\0\0\0\252\273\314\335\356hd"}, 30) = 0
     0.000693 close(10)                 = 0
     0.000449 close(7)                  = 0
     0.009801 close(9)                  = 0

Re: ping -I eth1 ....

[Joakim Tjernlund]

Joakim Tjernlund/Transmode wrote on 2010/11/06 10:42:46:
> Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 21:31:50:
> >
> > On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> > > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> > > >
> > > > > Hopefully most of that is legacy or just plain wrong? Unless
> > > > > 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 put a link
> > down temporarly. Also, the outgoing interface looked up at routing
> > decision is not necessarly the interface used for sending in the end.
> > The packet may get mangled and rerouted by netfilter or tc on the 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() while the
> > socket is bound to an interface but if we choose to return an error
> > if the interface is down we might as well do so based on the operational
> > status.

> Perhaps there is a better way. This all started when pppd hung because
> of ping -I <ppp interface>, 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 interfaces
> and doesn't noticed that the link is down. Ideally ping should receive
> a ENODEV as soon as pppd calls PPPIOCDETACH.
>
>    0.000908 write(0, "Connection terminated.\n", 23) = 23
>      0.000481 gettimeofday({1288952770, 566048}, NULL) = 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 become free. Usage count = 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 become free. Usage count = 3
> Message from syslogd@Brazil at Fri Nov  5 11:26:51 2010 ...
> Brazil last message repeated 3 times
> , 0xbfbc3398) = 0
>     66.559216 connect(9, {sa_family=AF_PPPOX, sa_data="\0\0\0\0\0\0\0\252\273\314\335\356hd"}, 30) = 0
>      0.000693 close(10)                 = 0
>      0.000449 close(7)                  = 0
>      0.009801 close(9)                  = 0

Any comment on this last strace? It is expected that ping -I should
hold pppd hostage?

 Jocke

Re: ping -I eth1 ....

[Joakim Tjernlund]

Joakim Tjernlund/Transmode wrote on 2010/11/09 20:33:37:
>
> Joakim Tjernlund/Transmode wrote on 2010/11/06 10:42:46:
> > Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 21:31:50:
> > >
> > > On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> > > > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> > > > >
> > > > > > Hopefully most of that is legacy or just plain wrong? Unless
> > > > > > 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 put a link
> > > down temporarly. Also, the outgoing interface looked up at routing
> > > decision is not necessarly the interface used for sending in the end.
> > > The packet may get mangled and rerouted by netfilter or tc on the 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() while the
> > > socket is bound to an interface but if we choose to return an error
> > > if the interface is down we might as well do so based on the operational
> > > status.

> > Perhaps there is a better way. This all started when pppd hung because
> > of ping -I <ppp interface>, 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 interfaces
> > and doesn't noticed that the link is down. Ideally ping should receive
> > a ENODEV as soon as pppd calls PPPIOCDETACH.
> >
> >    0.000908 write(0, "Connection terminated.\n", 23) = 23
> >      0.000481 gettimeofday({1288952770, 566048}, NULL) = 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 become free. Usage count = 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 become free. Usage count = 3
> > Message from syslogd@Brazil at Fri Nov  5 11:26:51 2010 ...
> > Brazil last message repeated 3 times
> > , 0xbfbc3398) = 0
> >     66.559216 connect(9, {sa_family=AF_PPPOX, sa_data="\0\0\0\0\0\0\0\252\273\314\335\356hd"}, 30) = 0
> >      0.000693 close(10)                 = 0
> >      0.000449 close(7)                  = 0
> >      0.009801 close(9)                  = 0
>
> Any comment on this last strace? It is expected that ping -I should
> hold pppd hostage?
>

Ping?

Re: ping -I eth1 ....

[Eric Dumazet]

Le mercredi 17 novembre 2010 à 10:29 +0100, Joakim Tjernlund a écrit :
> Joakim Tjernlund/Transmode wrote on 2010/11/09 20:33:37:
> >
> > Joakim Tjernlund/Transmode wrote on 2010/11/06 10:42:46:
> > > Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 21:31:50:
> > > >
> > > > On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> > > > > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> > > > > >
> > > > > > > Hopefully most of that is legacy or just plain wrong? Unless
> > > > > > > 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 put a link
> > > > down temporarly. Also, the outgoing interface looked up at routing
> > > > decision is not necessarly the interface used for sending in the end.
> > > > The packet may get mangled and rerouted by netfilter or tc on the 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() while the
> > > > socket is bound to an interface but if we choose to return an error
> > > > if the interface is down we might as well do so based on the operational
> > > > status.
> 
> > > Perhaps there is a better way. This all started when pppd hung because
> > > of ping -I <ppp interface>, 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 interfaces
> > > and doesn't noticed that the link is down. Ideally ping should receive
> > > a ENODEV as soon as pppd calls PPPIOCDETACH.
> > >
> > >    0.000908 write(0, "Connection terminated.\n", 23) = 23
> > >      0.000481 gettimeofday({1288952770, 566048}, NULL) = 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 become free. Usage count = 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 become free. Usage count = 3
> > > Message from syslogd@Brazil at Fri Nov  5 11:26:51 2010 ...
> > > Brazil last message repeated 3 times
> > > , 0xbfbc3398) = 0
> > >     66.559216 connect(9, {sa_family=AF_PPPOX, sa_data="\0\0\0\0\0\0\0\252\273\314\335\356hd"}, 30) = 0
> > >      0.000693 close(10)                 = 0
> > >      0.000449 close(7)                  = 0
> > >      0.009801 close(9)                  = 0
> >
> > Any comment on this last strace? It is expected that ping -I should
> > hold pppd hostage?
> >
> 
> Ping?
> 

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 <nicolas.dichtel@6wind.com>
Reported-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 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;
 
 		/* RACE: Check return value of inet_select_addr instead. */
-		if (rcu_dereference(dev_out->ip_ptr) == NULL)
-			goto out;	/* Wrong error code */
+		if (!(dev_out->flags & IFF_UP) ||
+		    rcu_dereference(dev_out->ip_ptr) == NULL) {
+			err = -ENETUNREACH;
+			goto out;
+		}
 
 		if (ipv4_is_local_multicast(oldflp->fl4_dst) ||
 		    ipv4_is_lbcast(oldflp->fl4_dst)) {

Re: ping -I eth1 ....

[Joakim Tjernlund]

Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/17 10:51:07:
>
> Le mercredi 17 novembre 2010 à 10:29 +0100, Joakim Tjernlund a écrit :
> > Joakim Tjernlund/Transmode wrote on 2010/11/09 20:33:37:
> > >
> > > Joakim Tjernlund/Transmode wrote on 2010/11/06 10:42:46:
> > > > Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 21:31:50:
> > > > >
> > > > > On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> > > > > > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> > > > > > >
> > > > > > > > Hopefully most of that is legacy or just plain wrong? Unless
> > > > > > > > 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 put a link
> > > > > down temporarly. Also, the outgoing interface looked up at routing
> > > > > decision is not necessarly the interface used for sending in the end.
> > > > > The packet may get mangled and rerouted by netfilter or tc on the 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() while the
> > > > > socket is bound to an interface but if we choose to return an error
> > > > > if the interface is down we might as well do so based on the operational
> > > > > status.
> >
> > > > Perhaps there is a better way. This all started when pppd hung because
> > > > of ping -I <ppp interface>, 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 interfaces
> > > > and doesn't noticed that the link is down. Ideally ping should receive
> > > > a ENODEV as soon as pppd calls PPPIOCDETACH.
> > > >
> > > >    0.000908 write(0, "Connection terminated.\n", 23) = 23
> > > >      0.000481 gettimeofday({1288952770, 566048}, NULL) = 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 become free. Usage count = 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 become free. Usage count = 3
> > > > Message from syslogd@Brazil at Fri Nov  5 11:26:51 2010 ...
> > > > Brazil last message repeated 3 times
> > > > , 0xbfbc3398) = 0
> > > >     66.559216 connect(9, {sa_family=AF_PPPOX, sa_data="\0\0\0\0\0\0\0\252\273\314\335\356hd"}, 30) = 0
> > > >      0.000693 close(10)                 = 0
> > > >      0.000449 close(7)                  = 0
> > > >      0.009801 close(9)                  = 0
> > >
> > > Any comment on this last strace? It is expected that ping -I should
> > > hold pppd hostage?
> > >
> >
> > Ping?
> >
>
> I thought I posted a patch, is there something else ?

yes, I wondered about the above strace and if it is expected that ping -I
should hold pppd hostage? Should not ping receive a ENODEV as soon as
pppd detaches an interface?

>
> Could you please test with latest net-next-2.6 and following patch ?

I tested the first patch you sent and that one worked well. I can try
again on 2.6.35( our boards takes a while to move forward)?

>
>
> 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 <nicolas.dichtel@6wind.com>
> Reported-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  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;
>
>        /* RACE: Check return value of inet_select_addr instead. */
> -      if (rcu_dereference(dev_out->ip_ptr) == NULL)
> -         goto out;   /* Wrong error code */
> +      if (!(dev_out->flags & IFF_UP) ||
> +          rcu_dereference(dev_out->ip_ptr) == NULL) {
> +         err = -ENETUNREACH;
> +         goto out;
> +      }
>
>        if (ipv4_is_local_multicast(oldflp->fl4_dst) ||
>            ipv4_is_lbcast(oldflp->fl4_dst)) {
>
>

Re: ping -I eth1 ....

[Eric Dumazet]

Le mercredi 17 novembre 2010 à 11:09 +0100, Joakim Tjernlund a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/17 10:51:07:
> >
> > Le mercredi 17 novembre 2010 à 10:29 +0100, Joakim Tjernlund a écrit :
> > > Joakim Tjernlund/Transmode wrote on 2010/11/09 20:33:37:
> > > >
> > > > Joakim Tjernlund/Transmode wrote on 2010/11/06 10:42:46:
> > > > > Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 21:31:50:
> > > > > >
> > > > > > On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> > > > > > > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> > > > > > > >
> > > > > > > > > Hopefully most of that is legacy or just plain wrong? Unless
> > > > > > > > > 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 put a link
> > > > > > down temporarly. Also, the outgoing interface looked up at routing
> > > > > > decision is not necessarly the interface used for sending in the end.
> > > > > > The packet may get mangled and rerouted by netfilter or tc on the 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() while the
> > > > > > socket is bound to an interface but if we choose to return an error
> > > > > > if the interface is down we might as well do so based on the operational
> > > > > > status.
> > >
> > > > > Perhaps there is a better way. This all started when pppd hung because
> > > > > of ping -I <ppp interface>, 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 interfaces
> > > > > and doesn't noticed that the link is down. Ideally ping should receive
> > > > > a ENODEV as soon as pppd calls PPPIOCDETACH.
> > > > >
> > > > >    0.000908 write(0, "Connection terminated.\n", 23) = 23
> > > > >      0.000481 gettimeofday({1288952770, 566048}, NULL) = 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 become free. Usage count = 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 become free. Usage count = 3
> > > > > Message from syslogd@Brazil at Fri Nov  5 11:26:51 2010 ...
> > > > > Brazil last message repeated 3 times
> > > > > , 0xbfbc3398) = 0
> > > > >     66.559216 connect(9, {sa_family=AF_PPPOX, sa_data="\0\0\0\0\0\0\0\252\273\314\335\356hd"}, 30) = 0
> > > > >      0.000693 close(10)                 = 0
> > > > >      0.000449 close(7)                  = 0
> > > > >      0.009801 close(9)                  = 0
> > > >
> > > > Any comment on this last strace? It is expected that ping -I should
> > > > hold pppd hostage?
> > > >
> > >
> > > Ping?
> > >
> >
> > I thought I posted a patch, is there something else ?
> 
> yes, I wondered about the above strace and if it is expected that ping -I
> should hold pppd hostage? Should not ping receive a ENODEV as soon as
> pppd detaches an interface?
> 
> >
> > Could you please test with latest net-next-2.6 and following patch ?
> 
> I tested the first patch you sent and that one worked well. I can try
> again on 2.6.35( our boards takes a while to move forward)?

Well, in this case, apply commit : 

332dd96f7ac15e937088fe11f15cfe0210e8edd1

(net/dst: dst_dev_event() called after other notifiers)

http://git2.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=332dd96f7ac15e937088fe11f15cfe0210e8edd1

Re: ping -I eth1 ....

[Joakim Tjernlund]

Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/17 11:23:32:
>
> Le mercredi 17 novembre 2010 à 11:09 +0100, Joakim Tjernlund a écrit :
> > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/17 10:51:07:
> > >
> > > Le mercredi 17 novembre 2010 à 10:29 +0100, Joakim Tjernlund a écrit :
> > > > Joakim Tjernlund/Transmode wrote on 2010/11/09 20:33:37:
> > > > >
> > > > > Joakim Tjernlund/Transmode wrote on 2010/11/06 10:42:46:
> > > > > > Thomas Graf <tgr@infradead.org> wrote on 2010/11/05 21:31:50:
> > > > > > >
> > > > > > > On Fri, Nov 05, 2010 at 04:54:18PM +0100, Joakim Tjernlund wrote:
> > > > > > > > Eric Dumazet <eric.dumazet@gmail.com> wrote on 2010/11/05 16:06:54:
> > > > > > > > >
> > > > > > > > > > Hopefully most of that is legacy or just plain wrong? Unless
> > > > > > > > > > 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 put a link
> > > > > > > down temporarly. Also, the outgoing interface looked up at routing
> > > > > > > decision is not necessarly the interface used for sending in the end.
> > > > > > > The packet may get mangled and rerouted by netfilter or tc on the 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() while the
> > > > > > > socket is bound to an interface but if we choose to return an error
> > > > > > > if the interface is down we might as well do so based on the operational
> > > > > > > status.
> > > >
> > > > > > Perhaps there is a better way. This all started when pppd hung because
> > > > > > of ping -I <ppp interface>, 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 interfaces
> > > > > > and doesn't noticed that the link is down. Ideally ping should receive
> > > > > > a ENODEV as soon as pppd calls PPPIOCDETACH.
> > > > > >
> > > > > >    0.000908 write(0, "Connection terminated.\n", 23) = 23
> > > > > >      0.000481 gettimeofday({1288952770, 566048}, NULL) = 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 become free. Usage count = 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 become free. Usage count = 3
> > > > > > Message from syslogd@Brazil at Fri Nov  5 11:26:51 2010 ...
> > > > > > Brazil last message repeated 3 times
> > > > > > , 0xbfbc3398) = 0
> > > > > >     66.559216 connect(9, {sa_family=AF_PPPOX, sa_data="\0\0\0\0\0\0\0\252\273\314\335\356hd"}, 30) = 0
> > > > > >      0.000693 close(10)                 = 0
> > > > > >      0.000449 close(7)                  = 0
> > > > > >      0.009801 close(9)                  = 0
> > > > >
> > > > > Any comment on this last strace? It is expected that ping -I should
> > > > > hold pppd hostage?
> > > > >
> > > >
> > > > Ping?
> > > >
> > >
> > > I thought I posted a patch, is there something else ?
> >
> > yes, I wondered about the above strace and if it is expected that ping -I
> > should hold pppd hostage? Should not ping receive a ENODEV as soon as
> > pppd detaches an interface?
> >
> > >
> > > Could you please test with latest net-next-2.6 and following patch ?
> >
> > I tested the first patch you sent and that one worked well. I can try
> > again on 2.6.35( our boards takes a while to move forward)?
>
> Well, in this case, apply commit :
>
> 332dd96f7ac15e937088fe11f15cfe0210e8edd1
>
> (net/dst: dst_dev_event() called after other notifiers)
>
> http://git2.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=332dd96f7ac15e937088fe11f15cfe0210e8edd1

hmm, with (or without) this and/or your other patch I get:
  vconfig add eth0 1
  ifconfig eth0.1 up
  ping -I eth0.1
<in other session do>
  vconfig rem eth0.1

Now vconfig rem eth0.1 hangs and I get several:
localhost kernel: unregister_netdevice: waiting for eth0.1 to become free. Usage count = 3
After some time vconfig rem eth0.1 succeeds
hmm, last test I did is still stuck, vconfig rem eth0.1 still hangs after 5 minutes.

The ping -I dies right after vconfig rem eth0.1

kernel 2.6.35