[B.A.T.M.A.N.] Changing vis output ready for mainline...

[B.A.T.M.A.N.] Changing vis output ready for mainline...

[Andrew Lunn]

Hi Folks

We need to discuss the changes which are needed to the VIS output for
inclusion of batman into mainline.

At the moment the vis output can be read from /proc/net/batman-adv/vis. It
supports two formats, graphviz dot and JSON. This can be selected
using the file /proc/net/batman-adv/vis_format.

I see a few problems here. 

1) No other /proc file in mainline does any special formatting. Either
comma separated value or space separated value is used. User space
reads this and then performs any formatting needed.

2) No other /proc file supports two userspace switchable formats!

3) We have problems when multiple applications wants different formats
at the same time. One wants dot the other wants JSON. Currently there
is no locking, so something bad is going to happen.

There are a number of ways to solve this. We should discuss different
proposals.

I would suggest changing /proc/net/batman-adv/vis to space separated
value and moving the current formatting code into batctl. I don't yet
have a 100% defined format in mind.

A current vis output would be:

        "fe:fe:00:00:04:01" -> "fe:fe:00:00:05:01" [label="1.0"]
        "fe:fe:00:00:04:01" -> "fe:fe:00:00:03:01" [label="1.0"]
        "fe:fe:00:00:04:01" -> "00:ff:7c:3c:ec:e0" [label="HNA"]
        subgraph "cluster_fe:fe:00:00:04:01" {
                "fe:fe:00:00:04:01" [peripheries=2]
                "fe:fe:00:00:04:02"
        }

I suggest something like:

fe:fe:00:00:04:01 fe:fe:00:00:05:01 255 fe:fe:00:00:03:01 255 00:ff:7c:3c:ec:e0 HNA fe:fe:00:00:04:02 S

In BNF something like

<Primary MAC> <Doublets>
<Doublets>   ::= {<Doublet> }
<Doublet>    ::= <MAC> <Flag/Value>
<Flag/Value> ::= <TQ_Value>|<HNA>|<Secondary>
<TQ_Value>   ::= uint8_t
<HNA>        ::= "HNA"
<Secondary>  ::= "S"

We then have a list of these, one line per mesh node which has
provided VIS information.

What do others thing of this? Does anybody have a different proposal?

     Thanks
        Andrew

Re: [B.A.T.M.A.N.] Changing vis output ready for mainline...

[Antoine van Gelder]

On 12 Oct 2009, at 10:11 , Andrew Lunn proposed:

> I would suggest changing /proc/net/batman-adv/vis to space separated
> value and moving the current formatting code into batctl. I don't yet
> have a 100% defined format in mind.


+1

/proc output formats should be unary and certainly be parse-able using  
command line tools.

Moving the DOT & JSON formatting code to batctl should also ensure  
that this change has minimal impact on users.



> A current vis output would be:
>
>        "fe:fe:00:00:04:01" -> "fe:fe:00:00:05:01" [label="1.0"]
>        "fe:fe:00:00:04:01" -> "fe:fe:00:00:03:01" [label="1.0"]
>        "fe:fe:00:00:04:01" -> "00:ff:7c:3c:ec:e0" [label="HNA"]
>        subgraph "cluster_fe:fe:00:00:04:01" {
>                "fe:fe:00:00:04:01" [peripheries=2]
>                "fe:fe:00:00:04:02"
>        }
>
> I suggest something like:
>
> fe:fe:00:00:04:01 fe:fe:00:00:05:01 255 fe:fe:00:00:03:01 255 00:ff: 
> 7c:3c:ec:e0 HNA fe:fe:00:00:04:02 S
>
> In BNF something like
>
> <Primary MAC> <Doublets>
> <Doublets>   ::= {<Doublet> }
> <Doublet>    ::= <MAC> <Flag/Value>
> <Flag/Value> ::= <TQ_Value>|<HNA>|<Secondary>
> <TQ_Value>   ::= uint8_t
> <HNA>        ::= "HNA"
> <Secondary>  ::= "S"
> We then have a list of these, one line per mesh node which has
> provided VIS information.


Couple of questions:

* Would (<TQ_Value> / 255) be equivalent to the label value in the  
current vis output?

* Does <Secondary> refer to a secondary interface reachable from  
<Primary MAC> ?

Looks sane!

  - antoine

Re: [B.A.T.M.A.N.] Changing vis output ready for mainline...

[Andrew Lunn]

> >A current vis output would be:
> >
> >       "fe:fe:00:00:04:01" -> "fe:fe:00:00:05:01" [label="1.0"]
> >       "fe:fe:00:00:04:01" -> "fe:fe:00:00:03:01" [label="1.0"]
> >       "fe:fe:00:00:04:01" -> "00:ff:7c:3c:ec:e0" [label="HNA"]
> >       subgraph "cluster_fe:fe:00:00:04:01" {
> >               "fe:fe:00:00:04:01" [peripheries=2]
> >               "fe:fe:00:00:04:02"
> >       }
> >
> >I suggest something like:
> >
> >fe:fe:00:00:04:01 fe:fe:00:00:05:01 255 fe:fe:00:00:03:01 255
> >00:ff:7c:3c:ec:e0 HNA fe:fe:00:00:04:02 S
> >
> >In BNF something like
> >
> ><Primary MAC> <Doublets>
> ><Doublets>   ::= {<Doublet> }
> ><Doublet>    ::= <MAC> <Flag/Value>
> ><Flag/Value> ::= <TQ_Value>|<HNA>|<Secondary>
> ><TQ_Value>   ::= uint8_t
> ><HNA>        ::= "HNA"
> ><Secondary>  ::= "S"
> >We then have a list of these, one line per mesh node which has
> >provided VIS information.
> 
> 
> Couple of questions:
> 
> * Would (<TQ_Value> / 255) be equivalent to the label value in the
> current vis output?

I need to look at the current code to see how it derives its
value. Since you cannot use floating point in the kernel i know it
does something different than plain /255. What ever it does, i would
keep the same algorithm in batctl so the values stay the same. It
would also be possible to add a flag to batctl to really do a floating
point TQ_Value/255. 

> * Does <Secondary> refer to a secondary interface reachable from
> <Primary MAC> ?

Linus added the subgraph for the primary and secondary interfaces. 

https://lists.open-mesh.net/pipermail/b.a.t.m.a.n/2009-August/001669.html
https://lists.open-mesh.net/pipermail/b.a.t.m.a.n/2009-August/001723.html

The word "secondary" is mine, i don't think Linus used it
anywhere. I'm happy to change this to something else.
 
> Looks sane!

Thanks

        Andrew

Re: [B.A.T.M.A.N.] Changing vis output ready for mainline...

[Marek Lindner]

Hi,

> 1) No other /proc file in mainline does any special formatting. Either
> comma separated value or space separated value is used. User space
> reads this and then performs any formatting needed.
> 
> 2) No other /proc file supports two userspace switchable formats!
> 
> 3) We have problems when multiple applications wants different formats
> at the same time. One wants dot the other wants JSON. Currently there
> is no locking, so something bad is going to happen.
> 
> What do others thing of this? Does anybody have a different proposal?

thanks a lot for summarizing the situation and pushing for a solution.

It seems we have to decide where we want to go with batctl. Until now batctl 
was not mandatory to operate the batman-adv kernel module. batctl extended the 
modules functionality and made things easier to configure but remained 
optional.

Personally, I like it a lot that way because it does not create unnecessary 
dependencies. It can become an uphill battle if you want to keep your kernel 
and its configuration tools in sync. Everyone that tried to use oprofile knows 
what I'm talking about (or the wireless tools to name another example).

So, the question is whether we want to make batctl the almighty tool that we 
always depend on or are we trying to find alternative solutions to avoid that 
dependency ?

One option for the case at hand might be debugfs 
(http://lwn.net/Articles/115282/). It is another filesystem which needs to be 
mounted seperately - there we can output whatever we want (unlike /proc). It 
would be possible to create virtual files for the dot draw / json / raw output 
(raw means the neutral format suggested by Andrew).

Soon we also will have to decide what to do with routing log as we can't 
"spam" the system log with our data. Again, debugfs seems like a good 
solution. That does not need to be decided now but could be a consistent path 
towards mainline integration.

Regards,
Marek

Re: [B.A.T.M.A.N.] Changing vis output ready for mainline...

[elektra]

Hi!

> It seems we have to decide where we want to go with batctl. Until now
> batctl was not mandatory to operate the batman-adv kernel module. batctl
> extended the modules functionality and made things easier to configure but
> remained optional.
>
> Personally, I like it a lot that way because it does not create unnecessary
> dependencies.

Ack.

> It can become an uphill battle if you want to keep your
> kernel and its configuration tools in sync. Everyone that tried to use
> oprofile knows what I'm talking about (or the wireless tools to name
> another example).
>
> So, the question is whether we want to make batctl the almighty tool that
> we always depend on or are we trying to find alternative solutions to avoid
> that dependency ?

I'd opt for the latter.

>
> One option for the case at hand might be debugfs
> (http://lwn.net/Articles/115282/). It is another filesystem which needs to
> be mounted seperately - there we can output whatever we want (unlike
> /proc). It would be possible to create virtual files for the dot draw /
> json / raw output (raw means the neutral format suggested by Andrew).
>
> Soon we also will have to decide what to do with routing log as we can't
> "spam" the system log with our data. Again, debugfs seems like a good
> solution. That does not need to be decided now but could be a consistent
> path towards mainline integration.

Seems like the way to go.

Cheers,
Elektra

Re: [B.A.T.M.A.N.] Changing vis output ready for mainline...

[Andrew Lunn]

> So, the question is whether we want to make batctl the almighty tool that we 
> always depend on or are we trying to find alternative solutions to avoid that 
> dependency ?
> 
> One option for the case at hand might be debugfs 
> (http://lwn.net/Articles/115282/). It is another filesystem which needs to be 
> mounted seperately - there we can output whatever we want (unlike /proc). It 
> would be possible to create virtual files for the dot draw / json / raw output 
> (raw means the neutral format suggested by Andrew).

I personally don't like the idea of using debugfs for VIS. I would not
call the mesh topology debug information for kernel hackers, which is
the purpose of debugfs.

However i do see a compromise. 

1) Make /proc/net/batman-adv/vis the neutral format i suggested, or
something similar. Extend batctl to use this and apply formatting as
wanted.

2) Add a kernel module internal interface which allows other kernel
modules to get access to the raw VIS data. Write a batman-graphviz and
a batman-json kernel module which outputs the VIS data in formatted
text to debugfs.

If you don't want userspace dependencies, and you know your kernel
supports the optional debugfs, you can use the kernel modules. If not,
batctl will always work, it just requires you have a version which
understands the current proc format.

       Andrew

Re: [B.A.T.M.A.N.] Changing vis output ready for mainline...

[Andrew Lunn]

>         "fe:fe:00:00:04:01" -> "fe:fe:00:00:05:01" [label="1.0"]
>         "fe:fe:00:00:04:01" -> "fe:fe:00:00:03:01" [label="1.0"]
>         "fe:fe:00:00:04:01" -> "00:ff:7c:3c:ec:e0" [label="HNA"]
>         subgraph "cluster_fe:fe:00:00:04:01" {
>                 "fe:fe:00:00:04:01" [peripheries=2]
>                 "fe:fe:00:00:04:02"
>         }
> 
> I suggest something like:
> 
> fe:fe:00:00:04:01 fe:fe:00:00:05:01 255 fe:fe:00:00:03:01 255 00:ff:7c:3c:ec:e0 HNA fe:fe:00:00:04:02 S
> 
> In BNF something like
> 
> <Primary MAC> <Doublets>
> <Doublets>   ::= {<Doublet> }
> <Doublet>    ::= <MAC> <Flag/Value>
> <Flag/Value> ::= <TQ_Value>|<HNA>|<Secondary>
> <TQ_Value>   ::= uint8_t
> <HNA>        ::= "HNA"
> <Secondary>  ::= "S"

A small refinement:
<Doublets>   ::= {<Doublet> ","}

Adding the comma makes the format more future proof. If in the future
we need to add more doublets, triplets, etc we can do it without
breaking backwards compatibility. If batctl finds something it does
not understand, it skips to the next , separator and keeps going.

    Andrew

Re: [B.A.T.M.A.N.] Changing vis output ready for mainline...

[Andrew Lunn]

> Couple of questions:
> 
> * Would (<TQ_Value> / 255) be equivalent to the label value in the
> current vis output?


Here is the current kernel code:

#define TQ_MAX_VALUE 255
[snip]
    /* kernel has no printf-support for %f? it'd be better
     * to return this in float. */
    int_part = TQ_MAX_VALUE / entry->quality;
    frac_part = (1000 * TQ_MAX_VALUE / entry->quality) -
            (int_part * 1000);
[snip]
            seq_printf(seq,
                       "\t\"%s\" -> \"%s\" [label=\"%d.%d\"]\n",
                       from, to, int_part, frac_part);

  Andrew