Re: [mpls-linux-devel] Re: 2.6 Spec: Random comments.

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

On Fri, 2004-02-13 at 10:17, Ramon Casellas wrote:
> On Fri, 13 Feb 2004, Jamal Hadi Salim wrote:
> 
> disclaimer:
> 
> from now on, all mails will be sent to mpl...@li... (
> that is what I wa going to do, but then i received your email about the 
> mailing list failing) 
> 

Following on your statement - removed Dave. I like ccing original
sender in case mailing list goes down ..

>  
> > Essentially it is an identifier of a NHLFE entry. 
> > So you are right naming it a FEC identifier may not be the best.
> 
> So the relationship is:
> 
> 1 FEC F                         to      N available objects 
        ^ is that F a typo?
> 1 Incoming Label/Labelspace     to      N available objects

Essentially yes if the F is a typo.


> > I dont wanna call the so-far-called fecid lspid but it is close.
> 
> I see what you mean. Let us check what the RFC says:
>    The "Incoming Label Map" (ILM) maps each incoming label to a set of
>    NHLFEs. (...)
> 

[..]

> (RCAS: N.B. you don't need equal cost paths)...

We can do it , so lets just add it.

> > Given the above info, suggest a new name. Maybe NHid?
> 
> I would say something like a "fwd_id" from "Forward Id", or "out_id" it 
> should not prelude one or other Next Hop.

you are sure you dont want nh somewhere in there?
since this is a reference to the NHlfe; 
heck why dont we just call it nhlfe_id ? ;->
 

> > Also i would think most of these lists would contain a single entry.
> 
> Yes, unicast MPLS with no load sharing enabled. However, you may need them 
> when doing multicast and/or load sharing.
> 


You mentioning multicast has given me some interesting thoughts.
Essentially multicast would be just another algorithm in the thought
that i previously posted (response to James).

> 
> > BTW, the ILM is not multihop ready. We should be able to add easily.
> Do you really need it to? Just hold a set of fwd_ids. The policy to select 
> one should be configurable (discipline) & pluggable. A common impl. Is a 
> hash table.

Almost like you read my mind. 
Refer to my earlier email for the suggestions i made. 

> The RFC also defines the interaction with routing in this case. (although 
> vaguely)

any routing/IP details in my opinion are NHLFE related.
Example a neighbor needs to have an IP address.

> 
> > The ability to select this value by policy allows us to be able to
> > select the NHLFE entries from other subsystems; eg a u32 classifier
> > on ingress could select all IP addresses from 10.1.1.1/24 to have a
> > fecid of 10. The skb->fecid is then set to 10. When the packet gets to
> 
> I see, but as long as it is not called fec_id, it's fine :) call it 
> fwd_id.

check my earlier view above. Toss a coin and pick something and lets
stick with it.

> 
> > 
> > dsts are still managed from the MPLS code. There is some generic stuff
> > (create, destriy, gc etc) for which there is no point in recreating in
> > the MPLS code
> 
> I am not sure that you need to. This is what was done in James' impl.  
> mpls_dst. The only thing you need is a means to allocate mpls_dsts and
> hang the reference into the skb's dst. The advantage is that you don't add
> another member to dst (I still don't like adding a mpls ptr to a generic
> dst, but I assume you are far more knowledgeable than I am), but of 
> course, you still have to modify the skb dst. (e.g. release it and hold a 
> new reference).
> 

Ok i will need to look at the code.

> 
> > The way it is right now works fine. What could probably have been a
> > better approach is to stack dsts. It would require some surgery and i am
> > not sure i have the patience for it. 
> 
> Well, I though we agreed on doing it the right way :) I am not stating 
> which one it is though.

Absolutely, but that also means not sticking unnecessary ifdefs in 20
files just so that you can supports some funky xfrm approach.

> In  mpls_unicast_forward
> 
>         lt = (struct ltable *)skb->dst;
>         skb->dst = &lt->u.dst;
> 
> would not it be possible here to allocate a mpls_dst with a new dst_ops 
> with the right size?

Yes, this is the dirtiest scene in the usage of the skb->dst in that
code. It is not too too bad as far as obscenity level is concerned and.
if there are better ways to do this, lets move on to those approaches.
The big challenge would be the other issues associated with it such as
hh, neighbors etc. 

> comment: I *do* think that mpls_tunnel.c from James impl can directly be 
> used and it's very convenient. Just %s/moi/fwd_id/g

What is the mpls_tunnel.c for? Is it a netdevice? What is it used for?

cheers,
jamal