[mpls-linux-devel] More comments on Jamal's spec

[Ramon C. <cas...@in...>]

Jamal, All,


Please, find my comments inline below. They concern userspace app grammar and
syntax, as well as discussing opcodes.

GENERIC COMMENTS (no flaming intented).
########################################################

Although I think that we are indeed on the right path, IMHO, for the
moment, your implementation is lagging functionnality w.r.t. James' one
(available userspace apps, diffserv mapping, tunnels, procfs, sysfs,etc.
although some are not strictly required), although I admit that there are
still serious issues with James' impl (locking and SMP safeness are the
most notorious ones), and that it is just a matter of time and work.

Given the fact that DaveM explicitely supports yours, it seems clear to me
that we should focus on it (James? yours is the last word... I have
been working on yours for only several months, you have spent the last
five years), and avoid any other fork. So the question is "what can we do
now?". When I started working on James implementation, I appreciated being
almost immediately given write access, so I could do some documenting
tasks while I was understanding the inner works..., and I was trusted. I
understand that you may see things differently. What is your position on
this? Neither James nor I have (for the moment?) access to the patch/CVS
repository, l2c userspace application.... Somehow I feel hand tied :) I
have spare time and I'm afraid that you may want a centralized approach,
which may have some inconvenients (although you have all right to).

In other words, if you were to be this project manager ;). How would you define
the tasks so everyone may contribute to the project, see the others recognize
his work, etc? Personally, I am interested and I would like to play a nice part
on this. What is missing?


QUESTIONS
########################################################

question: Are there performance studies regarding radix trees w.r.t Hash
buckets and linked lists? If the number of labels is large, isn't the O(N) walk
op going to slow things down?  How many labels are managed in average? for
example, if we assume 100000 BGP prefixes and (why not) a label per prefix,
with hash&walk (1024 hash buckets) it makes 100 entries (average) per bucket vs
approx log2(N) with binary trees?

what about other advanced ADT, like Hash buckets and radix trees or similar?
Thoughts?


question: regarding dst management. Maybe Alexey could enlighten us. It may be
interesting to know his point of view about adding a specific mpls ptr to the
generic dst struct, or he may even propose alternate solutions...







COMMENTS ON USERSPACE APP
########################################################

Jamal's proposal:

l2c mpls nhlfe <cmd> dev <devname>
index <val> proto <ipv4|ipv6> nh <neighbor>
<operation set> fec <FECid>
operation set := (op <operation>)*
* cmd is one of:  <add | del | replace | get>


20040214-RCAS- We should work on both grammars. I understand that they are a
work in progress, but they are imprecise and inconvenient. for example the
"del" operation should not require the user to give the  neighbour. OTOH,  I
think we don't need replace (simple remove and add)

* index could be used to store the LSPid
20040214-RCAS- (I don't understand this. ???)

* FECid is the FEC identifier to be used as the key for searching.

20040214-RCAS- nhlfe_id :)


Well, IMHO, I think the grammar can be improved. All the opcodes
need to be defined, with their arguments (get them from James
Implementation. They are quite complete and comprehensive)


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
SUGGESTION What about this ?

l2c mpls nhlfe COMMAND <nhlfe_id>

COMMAND      := [add | del | get | SETCOMMAND]
SETCOMMAND   := set proto <ipv4|ipv6> nh <neighbour> "OPERATIONSET"
OPERATIONSET := [swap SWAPARGS | pop | dlv | mapexp....],+
SWAPARGS     := labelvalue[:labelspace]..
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Examples

# l2c mpls nhlfe add <nhlfe_id>
Add an empty entry (default, drop). Error if exists.

# l2c mpls nhlfe del <nhlfe_id>
Remove the given entry. Ignore if not exists.

# l2c mpls nhlfe get <nhlfe_id>
Dump entry. Ignore if not exists.

# l2c mpls nhlfe set proto ipv4 nh 10.0.0.2 "swap 20,push 50" <nhlfe_id>




##############################################################
The ip tool should allow you specify route you want then
specify the FECid for that route, i.e:
ip route ... FECid <FECid>
where FECid is the NHLFE keyid we want to use
Example:
ip route add 10.0.0.21/32 via 10.0.0.9 dev eth0 fecid 1

20040214-RCAS all occurrences of FECId should be changed to nhlfe_id.
:)




##############################################################################
JAMAL:
l2c mpls ilm <cmd> dev <devname>
index <val> label fec <FECid>


RCAS : This should be <label> otherwise it looks like a keyword.

* cmd is one of:  <add | del | replace | get>

RCAS: I think we don't need replace. Let the user del and add. Too
many commands are cumbersome.
RCAS: Let's work on the grammar. The user should only need to give the
incoming label to remove, not the nhlfe_id that it points to.

* devname is the input device to be used
RCAS: right, but we need more flexibility.
RCAS: one option would be to use wilcards, e.g.
RCAS: l2c mpls ilm add "ethO:15"
RCAS: l2c mpls ilm add "*:15"
RCAS: but, I do think that the labelspace approach in james impl.
RCAS: is better. Let the user set a labelspace as a netdevice
RCAS: attribute and let the user define ILM entries as
RCAS: labelspace+value.


* Index is an additional identifier that could be used to
store LSP info.

RCAS : What is val?
RCAS: I still don't understand this. Could you please give examples?


* FECid is the FECid to be used for searching the NHLFE.

RCAS:  nhlfe_id is the NHLFE id to use.


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
SUGGESTION What about this ?

l2c mpls ilm COMMAND <ls:label>

COMMAND      := [add | del | get | BINDCOMMAND ]
BINDCOMMAND   := bind <nhlfe_id> to
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
well, or something like that



######################################################################


3.0 Allowed OPCODEs

20040214-RCAS: We need more advanced (DiffServ, etc.) opcodes. We can leverage
James implementation for this.




3.1 Modifiying opcodes

- REDIRECT: redirect a packet to a different LSP
(useful for testing or redirecting to a control plane)

20040214-RCAS: this can be useful. Nice.



- MIRROR: send a copy of a packet somewhere else for further
processing (useful for LSP pings, traceroute, debug etc)

20040214-RCAS: Idem.


3.2 Label action opcodes


20040214-RCAS: Why do we need to introduce two concepts "Label action opcodes"
and "Atomic operations"?  aren't all "Atomic operations" "label actions" and
viceversa???


The atomic operations are:"

- POP
- PUSH
- REPLACE

20040214-RCAS: Whats wrong with the standard name "SWAP"???



Note:
a stack of consisting atomic operations can be implemented; example:
a pop followed by several pushes.

20040214-RCAS: Ein??? Be more specific.




Well, I have some urgent boring things to do,  more comments to follow,


Thanks,

Ramon


// -------------------------------------------------------------------
// Ramon Casellas - GET/ENST/INFRES/RHD/A508 - cas...@in...