On Fri, 4 Feb 2000, Michal Ostrowski wrote:
> I hope that they do comment soon, there's some bigger issue involved
> here than a piddly little protocol. In particular I would like to
I think we are almost reaching a consensus. And no news means good news
;->
> Regardless of the kernel support, I think we know what needs to be
> done to pppd (more hooks, wrapping the tty code up as a plugin). Once
> this is done it's almost trivial to write plugins to support any kind
> of kernel support for PPPo*.
>
Nod
> That's a huge chunk of the code. I was going through my code trying
> to identify the PPPoE specific stuff and the somewhat more generic
> stuff and unfortunately I'm having a very tough time identifying any
> sizeable portion of the code that could be made generic. Essentially,
> every time I refer to a pppoe_hdr struct, that's code that's PPPoE
> specific-- and there's very little code that doesn't do that.
>
Thats because you were thinking PPPOE when you wrote that code;
think PPPOX now ;->
> I'd like to refer you to my pppoe_bind function -- this is very
> similar to what a pppoe_connect would look like if the discovery is
> removed. The primary point of this code is to create a hash-table
> entry, and thus all of this code is pppoe specific. Hence, if you did
> have AF_PPPOX, pppox_bind would essentially look something like:
>
> pppox_bind(...){
> ...
> return sk->sock_ops->bind(...)
> }
>
> In this case I'm worried that the PPPoX code would simply be there to
> provide another level of indirection --- and note that the only common
You can call it indirection or a wrapper; the point is you dont have to
right that particular piece of code for every one of them,
I dont think you'll be loosing any speed (or if you do it wont be that
much).
> code you could have is the code from the system-call side of things.
>
> So, here's what I suggest we consider:
>
> 1. Rename AF_PPPOE to AF_PPPOX.
>
> 2. Define a protocol number PPPOX_ETH for PPPoE --- require that this be
> used to identifty PPPOE sockets.
>
> 3. Define a sockaddr_pppox to be something like:
>
> struct sockaddr_pppox {
> sa_family_t sa_family; /* address family, AF_PPPOX */
> unsigned int sa_proto; /* protocol identifier */
> union {
> struct pppoe_addr pppoe;
> } sa_proto_data; /* protocol data */
> }
>
> 4. Add appropriate checks to the AF_PPPOE code to ensure that it is
> being used with AF_PPPOX semantics (that is check that the user is
> correctly specifying the protocol where applicable).
>
Sounds like a plan to me.
> What this will do is create an AF_PPPOX interface that is flexible ---
> when we want to define other protocols we simply add stuff to the union.
>
Yes.
> This way we get to keep binary compatibility with code that was
> compiled to use PPPOE, and we get to add protocols as appropriate.
> Each protocol gets to use whatever addressing scheme it requires.
>
You are thinking PPPOX ;->
> When it comes time to actually implement these other protocols, that's
> when we start working on seperating out protocol specific code (as is
> done with AF_INET). The point of this is that we won't know exactly
> how that is to be done until we have other protocols to work with.
>
true. But we should provide as much genericity with the current code as
we can. We can always rip it off in 2.5 when we learn more from this
experience.
> Mitch:
>
> That's the perfect example of why it's very hard to do proper
> negotiation in kernel-space. We'd like to be able to present the user
> with a funky GUI plugin which they can use to select their ISP. This
> would mean that we'd probably have to break out of the middle of a
> "connect" syscall in some way.
>
Thanks for pointing this out.
cheers
jamal
|