Re: Session ID 0 with PPPoE
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[EMAIL PROTECTED]> 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 >
Re: Session ID 0 with PPPoE
Hi, > 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. Not quite. But now I think I got it ;-) > 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. I mean, you are right from my experience that you want to have that for pppd (which is why I _do_ have that cron job) - but your rationale doesn't look very convincing to me. IP networks are unstable, you know. IP packets get dropped. Things (read: TCP peers) die randomly. But no, I don't have a cron job that restarts the MTA or web server in case some TCP peer died - because they don't exit when some TCP connection times out or something. The very same thing I'd expect from pppd. If I do specify the persist and maxfail 0 options, I expect pppd not to exit even when it receives complete garbage from the network, much less when it receives completely valid packets as per the RFC. The only reasonable reasons for exiting IMO are lack of system resources or missing system features/invalid configuration. The only reason why I do have a cron job that takes care of restarts are all the bugs I encountered in pppd so far that caused it to exit because it used the kernel PPP API wrongly in case of reconnects or because it leaked ptys until there were none available anymore, not the fact that PPP(oE) is a potentially unreliable network protocol or because modems do lose their connection at times. > 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? It isn't extremely important to me as it doesn't affect me. But if for any reason, then because I hope that others will take care of bugs that they stumble across before they become a potentially difficult to solve problem for me, too. Florian - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Session ID 0 with PPPoE
From: Florian Zumbiehl <[EMAIL PROTECTED]> Date: Sun, 4 Mar 2007 03:30:00 +0100 > I noticed that the PPPoE code doesn't allow session id 0x to be used > for an actual session but rather considers 0 a special value denoting > that the socket is unbound. Now, when reading RFC 2516, I couldn't really > find anything that would forbid 0x as a session id. Only 0x "is > reserved for future use and MUST NOT be used", while 0x is specified > as the only allowed value for the session id field on certain types of > packets, but neither can I find any statement that forbids 0x as > an ordinary session identifier, nor can I find any reasons that would > prevent PPPoE from functioning properly with a session id of 0x. > > Does anyone of you see any reason why a server would not be allowed to > select 0x as the session id for a PPPoE session? I can't, feel free to provide a patch to remove this limitation if it's important to you. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Session ID 0 with PPPoE
>From the RFC: 5.4 The PPPoE Active Discovery Session-confirmation (PADS) packet When the Access Concentrator receives a PADR packet, it prepares to begin a PPP session. It generates a unique SESSION_ID for the PPPoE session and replies to the Host with a PADS packet. The DESTINATION_ADDR field is the unicast Ethernet address of the Host that sent the PADR. The CODE field is set to 0x65 and the SESSION_ID MUST be set to the unique value generated for this PPPoE session. The PADS packet contains exactly one TAG of TAG_TYPE Service-Name, indicating the service under which Access Concentrator has accepted the PPPoE session, and any number of other TAG types. If the Access Concentrator does not like the Service-Name in the PADR, then it MUST reply with a PADS containing a TAG of TAG_TYPE Service-Name-Error (and any number of other TAG types). In this case the SESSION_ID MUST be set to 0x. As you can see from the last paragraph, a session id of 0 implies a rejection of the PADR. Thus, you can't possibly get a PADS packet that completes and initiates a valid session if the session id is 0. Note that the RFC does not prohibit all other aspects of the PADS to be structured as if it were a valid success response; the only condition and requirement of a failure mode here is the session id. Also 0x is reserved for future use. Thus it cannot be used as a sentinel value to indicate an invalid session id. Changing this code would require that the user-space component be synchronized with this change; as the socket interface implies that 0 is an invalid/unbound session id. Lots of badness will occur if 0 is allowed as a session id, and nothing will be gained because it can't possibly be a valid session id. -- Michal Ostrowski <[EMAIL PROTECTED]> On Sat, 2007-03-03 at 21:07 -0800, David Miller wrote: > From: Florian Zumbiehl <[EMAIL PROTECTED]> > Date: Sun, 4 Mar 2007 03:30:00 +0100 > > > I noticed that the PPPoE code doesn't allow session id 0x to be used > > for an actual session but rather considers 0 a special value denoting > > that the socket is unbound. Now, when reading RFC 2516, I couldn't really > > find anything that would forbid 0x as a session id. Only 0x "is > > reserved for future use and MUST NOT be used", while 0x is specified > > as the only allowed value for the session id field on certain types of > > packets, but neither can I find any statement that forbids 0x as > > an ordinary session identifier, nor can I find any reasons that would > > prevent PPPoE from functioning properly with a session id of 0x. > > > > Does anyone of you see any reason why a server would not be allowed to > > select 0x as the session id for a PPPoE session? > > I can't, feel free to provide a patch to remove this limitation > if it's important to you. > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Session ID 0 with PPPoE
Hi, > >>From the RFC: > > 5.4 The PPPoE Active Discovery Session-confirmation (PADS) packet > >When the Access Concentrator receives a PADR packet, it prepares to >begin a PPP session. It generates a unique SESSION_ID for the PPPoE >session and replies to the Host with a PADS packet. The >DESTINATION_ADDR field is the unicast Ethernet address of the Host >that sent the PADR. The CODE field is set to 0x65 and the SESSION_ID >MUST be set to the unique value generated for this PPPoE session. > >The PADS packet contains exactly one TAG of TAG_TYPE Service-Name, >indicating the service under which Access Concentrator has accepted >the PPPoE session, and any number of other TAG types. > >If the Access Concentrator does not like the Service-Name in the >PADR, then it MUST reply with a PADS containing a TAG of TAG_TYPE >Service-Name-Error (and any number of other TAG types). In this case >the SESSION_ID MUST be set to 0x. > > > > As you can see from the last paragraph, a session id of 0 implies a > rejection of the PADR. Thus, you can't possibly get a PADS packet that > completes and initiates a valid session if the session id is 0. > > Note that the RFC does not prohibit all other aspects of the PADS to be > structured as if it were a valid success response; the only condition > and requirement of a failure mode here is the session id. | [...] then it MUST reply with a PADS containing a TAG of TAG_TYPE | Service-Name-Error [...] !?! To my understanding, the indicator is the Service-Name-Error tag, and the RFC only states that if such a tag is present (indicating that the AC "doesn't like" the requested service name and thus rejects the session request), the session id field must be 0x - not that the session id field may not be 0x if this tag is not present (which would indicate that this is a valid session). > Also 0x is reserved for future use. Thus it cannot be used as a > sentinel value to indicate an invalid session id. Well, currently it could (IMO, a connect() specifying 0x as the session ID should fail anyway as of now as it is not a valid session id as per the RFC - and 0x in the session id field could be used to mean basically anything at the protocol level in the future) - however that probably wouldn't be a good choice for extensibility reasons: If at some point, a protocol session id field of 0x does somehow mean something that would sensibly be represented as 0x in the session id field of the internal data structure, one would have to change the code again. So I guess the session id simply shouldn't be overloaded, not even with an indication of its validity. > Changing this code would require that the user-space component be > synchronized with this change; as the socket interface implies that 0 is > an invalid/unbound session id. Well, either that or the indication as to whether the session id is currently valid should be stored in some different way. > Lots of badness will occur if 0 is allowed as a session id, and nothing > will be gained because it can't possibly be a valid session id. Well, if that was the case, sure. But I still don't see any reason why it can't be. Florian - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
