[openbeosnetteam] Re: moving ppp to other location

  • From: "Waldemar Kornewald" <Waldemar.Kornewald@xxxxxx>
  • To: <openbeosnetteam@xxxxxxxxxxxxx>
  • Date: Sun, 7 Sep 2003 20:01:43 +0200

> Exactly, an object that's exlusively used by modules, hence that
> directory.
> It's the same for i.e. the kernel/core/util directory - its contents
> are build as libkernel_util.a - and are just linked against the kernel
> like everything else, it's just a *part* of the kernel.

Okay, I can agree on this view. :)

> > Because then you cannot create a new connection from userland. How do
> > you
> > want to tell the core stack's PPP module that it should create a new
> > connection? There is no interface to the core stack (through which I
> > could
> > talk to the PPP stack).
> > This is the reason why I need the ppp driver. It does nothing except
> > call
> > some of those exported functions (mainly create_interface() and
> > control()).
> > I even removed all C++ stuff from those exported functions. The ppp
> > driver
> > does not link against libkernelppp.a. It is a light-weight wrapper to
> > the
> > ppp_interface_manager (the ppp stack).
> > I think after I told you that it is a 3-4k mini-driver you accepted
> > my
> > decision.
>
> Well, in that case I don't see a point for another driver even more.
> The main stack *is* accessed by a driver, you can just add the few
> wrapper functions to it directly.
> Since we've chosen to put the networking code into modules, we have a
> driver that will be opened from userland in order to talk to the stack,
> anyway.

What I need is a control() function in the net_module_info.
Do we need a length parameter? Is that call okay:
control(uint32 op, void *data, size_t length)
This would also affect the ethernet and loopback interfaces because of the
changed info structure.
As the core knows about all available modules it is better to let it call
the modules' control() functions. Or should the core allow to iterate
through the available module infos and should the driver use those functions
to call the control() functions?
Again, Philippe should have the last word.

> > The problem is that I cannot call the needed functions from userland.
> > IOCTL
> > would be enough for that task, but our network stack does not even
> > have a
> > general driver with which you can iterate through the interface
> > modules and
> > talk to them (as I said above).
> > Or am I wrong here? If I can talk to the manager using an IOCTL-like
> > function, then there is no need for the driver.
>
> That seems to explain the issue: you can just add your ioctl() commands
> to the existing driver.

I do not want to pollute the stack driver with PPP-specific code.

> > > So even if it will be a real shared library at one point in the
> > > future
> > > (which is obviously not now or in the realm of R1), that location
> > > looks
> > > appropriate to me, unless someone convinces me otherwise.
> > > Does that make my point a bit clearer?
> > Some day we will have shared libs support in the kernel, so it will
> > become a
> > real module (at the moment it is only intended to be a module).
> > From this point of view I can accept your suggestion.
>
> You still don't like the "is-part-of" explanation I tried to advertise
> above? :-)

I can live with it. ;)

> I know, having someone have a look at your sources/headers is a great
> thing, and I must admit that I haven't found the time to have a single
> look at your stuff yet.
> But ATM I don't think we should make a PPP API public, because I see
> little use in doing so.

What if the MDR wants to know when a PPP connection is established (we may
want to test our stack with MDR, too).

> And after having had a brief look at it, I have some short remarks:
> PPPControl.h:
> PPPC_SET_MTU - I don't think that should be part of the PPP stuff,
> since that's a feature of all other interfaces as well

It is actually PPPC_SET_MRU (not MTU). The MRU is only the base for the MTU
(in PPP: InterfaceMTU).
I could actually translate the SET_MRU call into a SET_MTU call by
calculating the encapsulator overhead and then substracting it from the
given MTU...

> PPPDefs.h:
> I would have a look if it isn't possible to use standard posix/Be
> error codes
> (but if not, it's fine to introduce new ones).

I did not find good alternatives for the PPP error codes in the BeOS
headers.

> PPP_MODULES_PATH shouldn't be "network/ppp-modules", but only
> "network/ppp" - please don't introduce a new naming scheme and add
> redundant suffixes.

I actually wanted to call the ppp interface module "ppp". Where should the
modules go then? How should the user know that "network/ppp" is the
directory to put modules into? IMHO, "network/ppp-modules" is much more
intuitive.

> As a general note, I think enum names should be lowercase
> (PPP_ENCAPSULATION_LEVEL -> ppp_encapsulation_level, only the contents
> there should be uppercase).

Oh, okay, I will change that...later...

> And before introducing another copy of Locker.h, please use the one
> already in our repository, and before introducing something like
> LockerHelper, please use the class that's already there for you:
> BAutolock.

I now even included the disk_device_manager's locker implementation.

Oh, and a question that I always had:
Should BLocker not set the semaphore's owner to B_KERNEL_TEAM (only for
kernel-space)?

To BAutolock:
The current implementation is missing a call:
UnlockNow(). This method is used quite frequently in PPP.
What does it do? It unlocks the BLocker before the destructor was called.
I have changed the implementation in my repository to support this feature
(it is only slightly bigger than the current implementation because the
destructor can use UnlockNow(), too).
You probably do not like the name. :) Is Unlock() better?
Shall I commit the change?

> Maybe it's a good idea to make the userland API more flexible and just
> have a driver_settings interface, and no other control structures.

How do you want to pass a pointer to the ifnet structure or other
structures? Should we really export the structure to userland? Maybe some
app will change it and crash the system.
Also, I think it would be far too complicated to use if you had to (e.g.,
for PPPProtocol) search for the "protocol" parameter and then use atoi() to
get the actual number.

> Also, I would try to make the whole thing as integrated with the core
> stack and its interfaces as possible.

Is it not integrated enough? :)
What do you mean by "integrated"? PPP is actually a completely new stack
that cannot be expressed with our netstack.
And as there is a new netstack implementation in development it would be a
bad idea to make PPP depend too much on the current stack.

> > > Yes, sounds reasonable.
> > > But in any way, I would wait for a word from our head chief.
> > Yes, me, too. I just did not want to leave your mail unanswered.
> > I hope Philippe will answer soon. ;;;)
>
> Yeah, but give him some time :-)

Of course. ;)

Waldemar


Other related posts: