On Thu, Mar 19, 2015 at 11:50 PM, Rafael Zalamena <rzalam...@gmail.com> wrote: > On Thu, Mar 19, 2015 at 8:32 AM, Martin Pieuchot <m...@openbsd.org> 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. > > --- SNIPPED OLD CHAT --- > > Thanks, I'll send a diff sometime soon if you don't do it first.
Here is a diff to fix the mpe(4) route installation that wasn't working. Code changes: * Add sc_ifa field and change sc_shim to sc_smpls (struct shim_hdr -> struct sockaddr_smpls) in mpe_softc; sc_ifa will be used by rt_ifa_* functions to install routes and sc_smpls was changed to simplify route install. * Removed old mpe_newlabel() function and replaced it with rt_ifa_*() calls; * Introduced code to deal with MPLS routes in rt_ifa_add() and rt_ifa_del(); rt_ifa_add() and rt_ifa_del() should work on rdomain 0 when dealing with MPLS. Index: sys/net/if_mpe.c =================================================================== RCS file: /cvs/src/sys/net/if_mpe.c,v retrieving revision 1.41 diff -u -p -r1.41 if_mpe.c --- sys/net/if_mpe.c 22 Dec 2014 11:05:53 -0000 1.41 +++ sys/net/if_mpe.c 21 Mar 2015 19:00:13 -0000 @@ -57,7 +57,6 @@ int mpeioctl(struct ifnet *, u_long, cad void mpestart(struct ifnet *); int mpe_clone_create(struct if_clone *, int); int mpe_clone_destroy(struct ifnet *); -int mpe_newlabel(struct ifnet *, int, struct shim_hdr *); LIST_HEAD(, mpe_softc) mpeif_list; struct if_clone mpe_cloner = @@ -85,7 +84,6 @@ mpe_clone_create(struct if_clone *ifc, i M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL) return (ENOMEM); - mpeif->sc_shim.shim_label = 0; mpeif->sc_unit = unit; ifp = &mpeif->sc_if; snprintf(ifp->if_xname, sizeof ifp->if_xname, "mpe%d", unit); @@ -105,6 +103,12 @@ mpe_clone_create(struct if_clone *ifc, i bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t)); #endif + mpeif->sc_ifa.ifa_ifp = ifp; + mpeif->sc_ifa.ifa_rtrequest = link_rtrequest; + mpeif->sc_ifa.ifa_addr = (struct sockaddr *) ifp->if_sadl; + mpeif->sc_smpls.smpls_len = sizeof(mpeif->sc_smpls); + mpeif->sc_smpls.smpls_family = AF_MPLS; + LIST_INSERT_HEAD(&mpeif_list, mpeif, sc_list); return (0); @@ -114,9 +118,17 @@ int mpe_clone_destroy(struct ifnet *ifp) { struct mpe_softc *mpeif = ifp->if_softc; + int s; LIST_REMOVE(mpeif, sc_list); + if (mpeif->sc_smpls.smpls_label) { + s = splsoftnet(); + rt_ifa_del(&mpeif->sc_ifa, RTF_MPLS | RTF_UP, + smplstosa(&mpeif->sc_smpls)); + splx(s); + } + if_detach(ifp); free(mpeif, M_DEVBUF, 0); return (0); @@ -292,7 +304,7 @@ mpeioctl(struct ifnet *ifp, u_long cmd, case SIOCGETLABEL: ifm = ifp->if_softc; shim.shim_label = - ((ntohl(ifm->sc_shim.shim_label & MPLS_LABEL_MASK)) >> + ((ntohl(ifm->sc_smpls.smpls_label & MPLS_LABEL_MASK)) >> MPLS_LABEL_OFFSET); error = copyout(&shim, ifr->ifr_data, sizeof(shim)); break; @@ -306,11 +318,11 @@ mpeioctl(struct ifnet *ifp, u_long cmd, break; } shim.shim_label = htonl(shim.shim_label << MPLS_LABEL_OFFSET); - if (ifm->sc_shim.shim_label == shim.shim_label) + if (ifm->sc_smpls.smpls_label == shim.shim_label) break; LIST_FOREACH(ifm, &mpeif_list, sc_list) { if (ifm != ifp->if_softc && - ifm->sc_shim.shim_label == shim.shim_label) { + ifm->sc_smpls.smpls_label == shim.shim_label) { error = EEXIST; break; } @@ -319,25 +331,29 @@ mpeioctl(struct ifnet *ifp, u_long cmd, break; ifm = ifp->if_softc; s = splsoftnet(); - if (ifm->sc_shim.shim_label) { + if (ifm->sc_smpls.smpls_label) { /* remove old MPLS route */ - mpe_newlabel(ifp, RTM_DELETE, &ifm->sc_shim); + rt_ifa_del(&ifm->sc_ifa, RTF_MPLS | RTF_UP, + smplstosa(&ifm->sc_smpls)); } /* add new MPLS route */ - error = mpe_newlabel(ifp, RTM_ADD, &shim); + ifm->sc_smpls.smpls_label = shim.shim_label; + error = rt_ifa_add(&ifm->sc_ifa, RTF_MPLS | RTF_UP, + smplstosa(&ifm->sc_smpls)); splx(s); - if (error) + if (error) { + ifm->sc_smpls.smpls_label = 0; break; - ifm->sc_shim.shim_label = shim.shim_label; + } break; case SIOCSIFRDOMAIN: /* must readd the MPLS "route" for our label */ ifm = ifp->if_softc; if (ifr->ifr_rdomainid != ifp->if_rdomain) { - if (ifm->sc_shim.shim_label) { - shim.shim_label = ifm->sc_shim.shim_label; + if (ifm->sc_smpls.smpls_label) { s = splsoftnet(); - error = mpe_newlabel(ifp, RTM_ADD, &shim); + rt_ifa_add(&ifm->sc_ifa, RTF_MPLS | RTF_UP, + smplstosa(&ifm->sc_smpls)); splx(s); } } @@ -433,38 +449,3 @@ mpe_input6(struct mbuf *m, struct ifnet splx(s); } #endif /* INET6 */ - -int -mpe_newlabel(struct ifnet *ifp, int cmd, struct shim_hdr *shim) -{ - struct rtentry *nrt; - struct sockaddr_mpls dst; - struct rt_addrinfo info; - int error; - - bzero(&dst, sizeof(dst)); - dst.smpls_len = sizeof(dst); - dst.smpls_family = AF_MPLS; - dst.smpls_label = shim->shim_label; - - bzero(&info, sizeof(info)); - info.rti_flags = RTF_UP | RTF_MPLS; - info.rti_mpls = MPLS_OP_POP; - info.rti_info[RTAX_DST] = smplstosa(&dst); - info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)ifp->if_sadl; - - error = rtrequest1(cmd, &info, RTP_CONNECTED, &nrt, 0); - rt_missmsg(cmd, &info, error ? 0 : nrt->rt_flags, ifp, error, 0); - if (cmd == RTM_DELETE) { - if (error == 0 && nrt != NULL) { - if (nrt->rt_refcnt <= 0) { - nrt->rt_refcnt++; - rtfree(nrt); - } - } - } - if (cmd == RTM_ADD && error == 0 && nrt != NULL) { - nrt->rt_refcnt--; - } - return (error); -} Index: sys/net/route.c =================================================================== RCS file: /cvs/src/sys/net/route.c,v retrieving revision 1.207 diff -u -p -r1.207 route.c --- sys/net/route.c 12 Feb 2015 11:19:57 -0000 1.207 +++ sys/net/route.c 21 Mar 2015 19:00:13 -0000 @@ -1119,6 +1119,15 @@ rt_ifa_add(struct ifaddr *ifa, int flags info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifa->ifa_ifp->if_rtlabelid, &sa_rl); +#ifdef MPLS + if ((flags & RTF_MPLS) == RTF_MPLS) { + info.rti_mpls = MPLS_OP_POP; + + /* MPLS routes only exist in rdomain 0 */ + rtableid = 0; + } +#endif /* MPLS */ + if ((flags & RTF_HOST) == 0) info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask; @@ -1164,6 +1173,12 @@ rt_ifa_del(struct ifaddr *ifa, int flags u_short rtableid = ifa->ifa_ifp->if_rdomain; u_int8_t prio = RTP_CONNECTED; int error; + +#ifdef MPLS + if ((flags & RTF_MPLS) == RTF_MPLS) + /* MPLS routes only exist in rdomain 0 */ + rtableid = 0; +#endif /* MPLS */ if ((flags & RTF_HOST) == 0 && ifa->ifa_netmask) { m = m_get(M_DONTWAIT, MT_SONAME); Index: sys/netmpls/mpls.h =================================================================== RCS file: /cvs/src/sys/netmpls/mpls.h,v retrieving revision 1.29 diff -u -p -r1.29 mpls.h --- sys/netmpls/mpls.h 15 Jan 2015 23:50:31 -0000 1.29 +++ sys/netmpls/mpls.h 21 Mar 2015 19:00:13 -0000 @@ -141,8 +141,9 @@ extern struct domain mplsdomain; struct mpe_softc { struct ifnet sc_if; /* the interface */ + struct ifaddr sc_ifa; int sc_unit; - struct shim_hdr sc_shim; + struct sockaddr_mpls sc_smpls; LIST_ENTRY(mpe_softc) sc_list; };