[asterisk-dev] dahdi_device branch

[Shaun Ruffell]

Tzafrir Cohen mentioned that Oron had intended for this reply to go to the
list. Therefore it's quoted in it's entirety with my comments inline.

On Sun, Jan 16, 2011 at 01:02:39PM +0200, Oron Peled wrote:
> On Thursday 13 January 2011 22:14:31 Tzafrir Cohen wrote:
> > On Mon, Jan 10, 2011 at 06:33:15PM -0600, Shaun Ruffell wrote:
> > > ..... Instead I'll need
> > > to add a dahdi_create_device / dahdi_destroy_device so that I can move
> > > the lifetime management of those objects out of the board drivers and
> > > into dahdi-base.c
> 
> Sounds very good.
> 
> > > My intent with the dahdi_devices (besides removing duplication
> > > information from the spans) is to allow a two-stage registration.  The
> > > dahdi_device can show up in sysfs when first registered.  A script
> > > started by udev can then set the span number and channel numbers on the
> > > child spans, which will then be registered themselves and show up in
> > > sysfs. 
> 
> Good.
> 
> > > I'm not convinced that we need to wait to merge your byname [3] branch
> > > until after this goes in anymore.  I was originally thinking that we
> > > wouldn't need to virtual DAHDI bus in order to plug the spans in on, and
> > > that we would only have a tree rooted in /sys/devices but I think I was
> > > wrong about that.  So, I don't see how slightly delaying the span
> > > registration in order to set the numbers conflicts with anything you
> > > have in the byname branch.
> 
> Completely agreed. Moreover, I now see that the two approaches may
> live together and actually complement each other:
>  * The first step is to have sysfs representation for spans with
>     attributes that uniquely identify the span. These will be the
>     references we can use in either dial_byname or to configure
>     the "base channel number" in your proposal:
> 
>     - hardware_id string -- is not modified if we move a card from
>        slot to slot (burned on the card, but not every card has this).
> 
>     - connector string -- is not modified if a card is replaced in the
>        same slot
> 
>      Either attribute together with the additional numeric span_id
>      attribute gives unique span identification.
> 
>      All these attributes are exported by DAHDI, but are assigned
>      values by low-level drivers.

Agreed.  I would also add that by allowing the DAHDI devices to be children of
the "real" device that is exporting them (either the 'struct pci_dev' or
'struct usb_device'), it the scripts can also get any information that
might be available on the parent (serial number, etc..).

> 
>  * Than, one option is to use 'dial_byname'. For this to work the user
>     needs:
>     - A 'dial_byname' udev rules to create the required device files.
>     - A very recent asterisk (1.8)
>     - A 'dial_byname' capable dahdi_cfg
>     - And a 'dial_byname' dialplan, config files etc.
> 
>  * However, a user can elect to use your proposed extension
>     (let's call it 'pinned_spans' from here) and configure what base
>     channel number is assigned to which spans:
>     - Only edit some simple (future) config file to match the base channel 
>        numbers to the span identifier (hardware_id|connector) + span_id
>     - The user do not need to modify hers dialplan, config-files etc.
>        Hooray!!!
>     - The user gets almost all benefits of dial_byname
>     - The user needs a 'pinned_spans' udev rules
> 
> There's no problem augmenting the udev scripts/programs so they
> generate both 'dial_byname' device files and also calls the 'pinned_spans'
> ioctl's for the spans appearing in the configuration.

I'm not thinking of using ioctl's for this, but rather attributes in
sysfs....but same idea.

> 
> > From #asterisk-dev (Tzafrir Cohen, Shaun Ruffel):
> > <tzafrir_laptop> So basically instead of low-level drivers registering
> > spans with dahdi and getting "first available channel numbers"
> > <tzafrir_laptop> they now register "dahdi devices", those devices become
> > available to userspace
> > <tzafrir_laptop> (e.g. generate a udev event)
> > <tzafrir_laptop> and udev registers spans and chanenls
> > <tzafrir_laptop> right?
> > <tzafrir_laptop> So we'll have some /etc/dahdi/channels.conf where
> > devices will have channel numbers allocated to them?
> > 
> > <sruffell> In a nutshell, yes, although udev will use the sysfs
> > interface for the devices to have the driver actually register the
> > individual spans and specify a beginning channel number for the spans
> > that are registered that way.
> > <sruffell> yes, and if there is a span plugged in for which there is no
> > definition, it will be automatically registered in a otherwise
> > unallocated range.
> 
> Two options I'm thinking about:
>  * A new span does not register its channels until it receives the
>    "base channel" assignment ioctl() from udev:
>    - Pros: When a channel is assigned, it is the final verdict.
>    - Cons: There is time when spans do not have channels.
>       How would it affect user space tools? What happens if nobody
>       calls this ioctl? [legacy user-space... related to your next point]

Instead of thinking about spans registering channels I am instead of
only of dahdi_devices registering their child spans.  Before the span is
registered, the span number and base channel number will already need to
be set.  That way spans are never visible or live until those pieces of
information are known.

> 
>  * A new span is assigned channels on FCFS basis. A "base channel"
>     assignment ioctl() cause re-assignment of channel numbers.
>     - Pros: Legacy user-space behaves the same while "pinned span"
>       udev rules work as expected.
>     - Cons: No idea how this could be implemented safely.

If span number and channels are assigned on a FCFS basis, then the
behavior stays the way it is now.  I am no longer of the opinion that
spans and channels should be renumbered while 'live'.  If they do need
to be moved, the spans should be unregistered and then reregistered
(perhaps via sysfs).

> 
> > <sruffell> But there will be a module parameter for dahdi that will
> > force the first come, first-serve style for people who don't want to
> > update *anything* in user space.
> 
> Good idea, this simplify the world:
>  * With pinned_spans=1 -- the driver would wait for the "base channel"
>     ioctl() before registering channels.
>  * With pinned_spans=0 -- the driver would register the channels
>     immediately upon span registration (with sequential numbering as today)
> 
> Optionally the span should send an "online" udev event after the channels
> are ready, so user space know when they're available and can run
> optional actions (e.g: reload chan_dahdi until we have totally dynamic
> chan_dahdi).
> 
> > <sruffell> (in addition to a time-out…if a device was registered, and
> > the spans weren't slotted…dahdi can assume user space isn't going to
> > help it, and go ahead and switch to first-come, first-serve)
> 
> Please don't! Time dependent behavior would bite back and hard.
> I would suggest instead:
>  * The relevant span/spans would not have registered channels.
>  * Optionally, after a timeout we can printk() an informative message
>    to that effect.
>  * The user may at her option:
>     - Decide that pinned_spans=0 is what she need.
>     - Or upgrade the user-space tools
>     - Or further debug the problem

Good point.  Agreed.

> 
> Another, unrelated, issue is how asterisk (or rather, chan_dahdi)
> handles failures in spans. Currently if any span fails to load, than
> chan_dahdi (correctly) refuses to load.
> Otherwise, the sequential channel assignment would wreck havoc in
> the dialplan.
> 
> This could be changed, but *only* if all spans:
>  * Use dial_byname
>  * Or are pinned spans
>  * .... or maybe a mix... what a mess ;-)
> How would we pass this knowledge to chan_dahdi? (sysfs attr?)

My early thoughts about this were that if chan_dahdi.conf has channels
listed that aren't in the system, it would fail to load.  If pinned
spans are in use, then an administrator could just comment out the
failed span/channel lines without having to renumber all the spans.  I
think we can revist this after we get the kernel modules doing what we
would like.

> 
> > <sruffell> udev will get the uevent, look in the configuration file to
> > see what span number should be assigned, and the base channel number for
> > each span, and give that information back to dahdi.ko for registration
> > of the spans.
> 
> Something we noticed during dial_byname development and may be relevant
> to this item: There is no device file that represent a span -- only
> individual channels. This isn't critical as I think you can use one of
> the general dahdi device files (e.g: /dev/dahdi/channel) for the
> relevant ioctl() it just look awkward.
> 
> > <sruffell> Something I wanted to ask you about the byname
> > branch of asterisk…
> > .... cut ...
> > <sruffell> ahh….so you need the minor number to always be equal to the
> > channel number.
> > <sruffell> So the TRANSCODE/CTL etc.. have to be moved off major 196?
> > <sruffell> so you don't have holes in those channel numbers?
> 
> Exactly. We leave the TRANSCODE/CTL etc DAHDI device files with the
> original  major/minor (for backward compat). And dynamically assign new
> devices files on a totally new range where minor==channo
> 
> > <tzafrir_laptop> at open time we translate names to channel numbers
> > <tzafrir_laptop> and from that point everything should be as before
> > <tzafrir_laptop> open, and channel generation
> 
> So our plan is to continue merge the dial_byname branch into trunk
> where the priority should be the sysfs span representation, so we
> could have the same code base for dial_byname and pinned_spans.
> Almost all code would be a the (new) dahdi-sysfs.c so it won't collide
> too much with other dahdi-base.c related changes.

Sounds good to me!

--
Shaun Ruffell
Digium, Inc. | Linux Kernel Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: www.digium.com & www.asterisk.org