On Thu, Mar 19, 2015 at 8:32 AM, Martin Pieuchot <[email protected]> wrote:
> On 18/03/15(Wed) 22:58, Rafael Zalamena wrote:
>> mpe(4) is not installing routes / label in the interface in -current.
>>
>> Snippet:
>> # ifconfig mpe0 mplslabel 100
>> ifconfig: SIOCSETLABEL: Network is unreachable
>>
>> Quickly looking at the code I found out that since the old MPLS route
>> installer function (mpe_newlabel) doesn't include an ifa pointer later
>> on rt_getifa() will fail and return ENETUNREACH.
>>
>> Trace:
>> mpe_newlabel -> rtrequest1 -> switch (RTM_ADD) -> rt_getifa
>>
>> I tried moving it to rt_ifa_add() using my old VPLS datapath diffs,
>> but there are some other problems like panic()s or NULL MPLS routes
>> installed for mpeX that might be happening because of my poor
>> understanding of the new network stack design (no more
>> ifp->if_lladdr).
>
> So mpe(4) was also abusing if_lladdr?
>
>> (this commit:
>> https://github.com/rzalamena/vpls-src/commit/675216b75b665f42b06bd2b0b18cbd0deab84f57)
>
> This is good. You can initialize sc_ifa in mpe_clone_create(), look at
> how enc(4) does it.
Yeah, adding this line to mpe_clone_create() or mpe_newlabel() fixes the panic.
mpe_clone_create(): mpeif->sc_ifa.ifa_rtrequest = link_rtrequest;
or
mpe_newlabel(): ifa->ifa_rtrequest = link_request;
>
> I'd also suggest using rt_ifa_add/del() directly and moving the "rdomain
> = 0" trick to these functions with an appropriate comment.
>
>> NOTE: to test mpe(4) I used the claudio@ MPLS article:
>> http://2011.eurobsdcon.org/papers/jeker/MPLS.pdf
>>
>>
>> NOTE 2:
>> Panic trace:
>> rt_msg2()
>> sysctl_dumpentry()
>> rn_walktree()
>> sysctl_rtable()
>> sys___sysctl()
>> syscall()
>
> With your diff I can create a mpe0 interface but this trace does not
> contain enough information for me to debug it. What's the panic
> message? What where you doing, what's the ps output?
To reproduce that trace: create an mpeX interface with some label,
delete the mpeX interface and do 'route -n show -mpls'.
Even though it does not panic() anymore, it still shows a (null)
interface route in 'route -n show -mpls' which might be fixed by
properly removing the route when destroying the clone interface.
Like this:
mpe_clone_destroy() {
...
if (mpeif->sc_shim.shim_label)
mpe_newlabel(ifp, RTM_DELETE, &ifm->sc_shim);
...
}
Thanks, I'll send a diff sometime soon if you don't do it first.