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

[Jamal H. S. <ha...@zn...>]

On Sun, 2004-02-15 at 05:36, Ramon Casellas wrote:
> re-hi,
> 
> FYI:
> http://www.enst.fr/~casellas/mpls-linux-2.6/spec/spec.pdf
> http://www.enst.fr/~casellas/mpls-linux-2.6/spec/index.html
> 
> Work in progress. Things to fix.

That document looks pretty now ;->
Some comments:
-Names: List by last name first (Casellas, Hadi Salim, Leu, Miller).
This way noone inteprets the documentation to mean it is listed by
contribution.
- General comments: Originally the doc was written informally with
"I" meaning myself. Its all over the doc. You may wanna fix that.
- 1.2.1: The TODO is a separate document now.
- 1.2.2.5: You still have that fecid in there. We may also need to
provide an example on ECMP using "ip route nexhop .."
- General: there should be consistency with the name for nhid - at times
it reads nhlfe_id and others nhlfeid.
- 1.2.3.1: Your comment on downstream on demand; i will respond below
since you have that comment in this email as well.
- figure 1.1: 
Logically you should draw an arrow from the route cache to the NHLFE
entry without anything in between. Implementation wise at the moment
there is a dst->mpls_route; but lets ignore that since we are not
talking implementation here.

What would be useful in this document as well is to describe the
interface between the kernel and user space. i.e describe the packets
used, events generated (at the moment any addition to NHLFE or ILM will
generate an event); start by looking at: include/linux/l2cnetlink.h; 
to cutnpaste from there:

----------------------------------
/* ILM related */

struct ilmmsg
{
        __u32   in_fecid;
        __u32   in_ifindex;
        __u32   in_space;
        __u32   in_label;
        __u8    in_owner;
};


/* ILM attributes */
enum
{
        ILM_UNSPEC,
        ILM_STATS,
};

/* NHLFE related */

struct nhlfemsg
{
        __u32   nh_fecid;
        __u32   nh_index;
        __u32   nh_ifindex;
        __u32   nh_space;
        __u32   nh_class;
        __u32   nh_flags;
        __u8    nh_owner;
        __u8    nh_proto;
        __u8    nh_dscp;
        __u8    nh_ttl;
        __u32   nh_ltype;
};
/* owner - who installed the rule */

enum
{
        L2C,            /* the l2c tool */
};

/* nh_proto choices */
enum {
        MPLS_IPV4,
        MPLS_IPV6,
};
/* nh_flags */
#define MPLS_FLAG_I_TC_INDEX    0x01 /* Input: Classify packet */
#define MPLS_FLAG_I_DIFFSERV    0x02 /* Input: Propagate diffserv bits
*/
#define MPLS_FLAG_O_TC_INDEX    0x04 /* Output: Classify packet */
#define MPLS_FLAG_O_DIFFSERV    0x08 /* Output: Propagate diffserv bits
*/
#define MPLS_FLAG_TTL_PROPAGATE 0x10 /* Input/Output: TTL propagation */
#define MIR_FLAG_TTL_PROPAGATE  MPLS_FLAG_TTL_PROPAGATE


/* NHLFE attributes */
enum
{
        NH_UNSPEC,
        NH_OP_INS,
        NH_STATS,
        NH_NEIGH_IP,
};


struct mpls_op_u {
        __u32 op;
        __u32 operand;
};

----------------------------------------

For how we document this typically look at:
http://www.faqs.org/rfcs/rfc3549.html

 
> 
> w.r.t Downstream on demand:
> 
> I *do* think it's valuable and we *must* support it (e.g RSVP-TE). If I
> cannot use RSVP-TE to setup LSPs in Linux I'm going back right now to
> James implementation ;-)
> 
> RCAS: I think we are confusing label distribution modes with
> implementation details. RSVP-TE uses downstream on demand as a label
> distribution mode, and it could be implemented as a two step process where
> a dummy NHLFE is created during the RSVP_PATH message so the ILM may point
> to it and then replaced by the right one upon reception of the RSVP_RESV.

Ok. So we may need some extra speacilized NHLFE entries. I am not a big
fan of the two step process unless you guys really insist - then we can
go and convince davem. 
My opinion is lets have 3 new speacial NHLFEs:
- something that sends the packet to a blackhole which will work for
such a scenarion as above.
- Another one will send the packet to user space via netlink. This may
also be used for resolving what you have above.
- A third one is for locally destined packets. I was not sure whether
this should just be a flag which says neighbor = local or not. 

> What we do not support is sending orphan packets to userspace, and that
> when an entry is added in the ILM or FTN there must be an exisiting
> NHLFEid. I'm not saying that we need to match the exact words of the RFC,
> but we can (and must) support downstream on demand.
> 

sure. Let me know what you think of the above.


> > him, if theres a > 20% improvement we have more strength);-> I wanna see
> > MPLS in 2.6 soon and i think this is the fastest way to get there.
> 
> Well, me too :) but I'd rather see it in 2.6 when it's ready. Most
> probably you're right and it is....
> 

As you can see, we are fixing things; good it didnt go in right away.

> > For example for something like the ILM, where lookup is based on a 12
> > bit label, then i would think making it anything more than a hash and
> (...)
> > make it 256 buckets and suddenly you are looking at 16 worst case.
> 
> Where do you get this numbers ? :)
> I thought the label was 20 bits, and
> with 256 buckets (2**8) you have 2**12 = 4096 worst case.
> even with 1024 buckets you get 1024 worst case.

Never mid - too many things being computed in my brain. I was thinking
of VLAN tags.

> am I missing something? Are these values acceptable? In that case 4ill
> shut up :)

Well, have some student do a project ;-> Let them measure the perfomance
differences under different scenarios with hash-and-walk vs radix tree
or another funky lookup scheme for say many many entries.. 
With data we can challenge the current scheme.

cheers,
jamal