Date:	Wed, 07 Mar 2007 09:24:34 -0600
From:	Michal Ostrowski <mostrows@...thlink.net>
To:	Florian Zumbiehl <florz@....de>
Cc:	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: Session ID 0 with PPPoE

This change can be made; the unbinding behavior can be removed and SID 0
can be made valid. I hope I was clear in my previous e-mail that I
didn't object to this.

PPPoE connections are unstable.  Ethernet frames get dropped.  Things
die randomly. And yes, you typically want to have a cron job or script
re-spawning pppd on failure.  Making this change won't increase the
reliability of these connections in any meaningful way, it won't break
pppd either.  

The only question I have is why is this important to you?  I'm simply
curious as to what you are trying to accomplish; is this related to some
other work you are doing or is it correctness as a virtue?

-- 
Michal Ostrowski <mostrows@...thlink.net>



On Wed, 2007-03-07 at 15:32 +0100, Florian Zumbiehl wrote:
> Hi,
> 
> > In the current  code SID 0 indicates that the socket is to be un-bound.
> 
> That are the semantics used by the kernel code, yes - but even pppd uses
> different semantics (which can't quite work, of course ...).
> 
> > Supporting unbinding of the socket was intended to permit the PPPoE
> > session to be reconnected without closing/reopening the socket; which
> > would mean that you'd have to re-bind the PPPoE/PPP channel bindings.
> > Thus it is conceivable to swap or renegotiate PPPoE connection
> > underneath a PPP connection, hypothetically if anyone ever considered
> > doing so.  Is that worth it?  I don't know.  One could eliminate that
> > disconnect behavior and I don't think anyone would care.
> 
> Well, if _you_ don't even know ... =:-)
> 
> Anyway, if it is not to be eliminated, it should be represented some way
> differently, I guess. Which probably would break backwards compatibility
> at the userspace API completely, of course, which is a bad thing[tm], so
> possibly simply changing the semantics to what is already assumed by most
> userspace applications might be the way to go.

> > I'll conceed that a SID of 0 could appear from outer space.   I've never
> > seen that happening.
> 
> Now, the question is, of course: How many samples is this based on
> (both, number of connection attempts and number of different peer
> implementations)?
> 
> > The only way I see this being an issue is if a
> > PPPoE server insists on giving you SID 0 and only SID 0 repeatedly.  And
> > I've never seen *that* happening.
> 
> So, what you are saying is that pppd/kernel-PPP/-PPPoE is so unstable that
> you shouldn't ever be using it without some cron job/wrapper that takes
> care of restarts anyway, so some additional bug that causes pppd to exit
> unexpectedly doesn't matter?

> That is to say: If I didn't overlook anything, pppd (the rp-pppoe plugin,
> that is) warns you if the PADS' session id field is 0 that the AC was
> somehow violating the RFC, but then goes on, using 0 as the session id
> anyway - thus calling connect() with a session id field of 0, obviously
> assuming that this will bind to session id 0 - which it doesn't.
> 
> Now, when it invokes the PPPIOCGCHAN ioctl on that seemingly successfully
> connect()ed socket, it gets an ENOTCONN - and exits with a fatal error.
> And exiting with a fatal error isn't quite what I'd expect pppd to do
> when the peer is behaving correctly according to the RFC, since that will
> not just cause a retry that might be likely to give a different session
> id and thus would cause the connection to finally be established with
> some delay, but rather will break network connectivity until manual
> intervention.
> 
> Of course, you could argue that this is somehow a bug in pppd, as it
> doesn't use the kernel API correctly. But I assume that fixing the
> kernel to support sid 0 rather than fixing pppd to "support" sid 0
> (which would mean implementing an automatic retry when assigned
> sid 0 by the AC) is somehow the better solution.
> 
> > If you'd really like to pursue this, I'll be happy to review and ack
> > patches in this regard.  However, I don't see what there is to be
> > actually gained by pursuing this.   I'm open to being convinced; what is
> > the motivation behind this?  If there is a real problem here I'll be
> > glad to get involved in fixing it myself.
> 
> Now, what is a "real problem"? I haven't noticed this being a problem for
> me yet, no. But I have been using userspace RP-PPPOE until recently and
> I am using a cron job that takes care of restarts in case pppd exits for
> some reason, so it's rather unlikely that I'd notice if it did happen.
> 
> But isn't a problem that does occur seldom, is difficult to reproduce,
> and has more than just temporary effect much worse a problem than something
> that causes a crash every half an hour and thus is simple to come by using
> debugging and doesn't surprise you the very moment you need things to
> simply work?


> Actually, IMO the major question is: Is there any application that does
> (intentionally) make use of this session rebinding/unbinding? Since
> simply dropping that probably would be the easiest fix to implement.
> 
> Florian
> 


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists