On 01/10/14(Wed) 21:54, Rafael Zalamena wrote: > This new diff aims to simplify the mpe(4) device and also to improve > the old code that handled the installation of MPLS interface routes. > > I followed what mpi@ said: > > On Tue, Sep 30, 2014 at 11:00:25AM +0200, Martin Pieuchot wrote: > > Hello Rafael, > > > > On 14/09/14(Sun) 23:49, Rafael Zalamena wrote: > > > The following patch is just a preparation for the code that is coming to > > > implement the wire network interface (the VPLS datapath) to work on > > > OpenBSD. > > > > > > This code turns the mpe code that handles route and labels into some > > > general > > > use functions that will be called by mpe and wire. > > > > Would it be possible to use the new rt_ifa_add() and rt_ifa_del() instead > > of > > keeping what is basically a copy of the old rtinit()? > > > > In your case you want to use the lladdr's ifa and you can check for > > RTF_MPLS in the flags to add the corresponding MPLS_OP_POP value. > > > > --- patch snipped --- > > Code change: > * Moved label address from softc to lladdr ifa
I'm afraid this is not what we want. The rest of your diff looks fine but moving the storage to be represented as a 'destination address' might make sense, but not attached on the lladdr ifa. > * Changed rt_ifa_add to default RTF_MPLS routes to do a POP and only > use rdomain 0 (MPLS only works on domain 0, and it doesn't make sense > other actions when creating MPLS route to an interface) > * Removed old code that installed mpe MPLS routes > * Conflicting labels verification is now done by routing (see rt_ifa_add()) This looks ok. > This was tested in the setup described in: > http://2011.eurobsdcon.org/papers/jeker/MPLS.pdf > > Here is the diff: > diff --git sys/net/if_mpe.c sys/net/if_mpe.c > index 74039dc..8580ef3 100644 > --- sys/net/if_mpe.c > +++ sys/net/if_mpe.c > @@ -61,7 +61,6 @@ int mpeioctl(struct ifnet *, u_long, caddr_t); > 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 = > @@ -84,13 +83,19 @@ mpe_clone_create(struct if_clone *ifc, int unit) > { > struct ifnet *ifp; > struct mpe_softc *mpeif; > + struct sockaddr_mpls *smpls; > int s; > > if ((mpeif = malloc(sizeof(*mpeif), > M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL) > return (ENOMEM); > > - mpeif->sc_shim.shim_label = 0; > + smpls = malloc(sizeof(*smpls), M_IFADDR, M_NOWAIT | M_ZERO); > + if (smpls == NULL) { > + free(mpeif, M_DEVBUF, 0); > + return (ENOMEM); > + } > + > mpeif->sc_unit = unit; > ifp = &mpeif->sc_if; > snprintf(ifp->if_xname, sizeof ifp->if_xname, "mpe%d", unit); > @@ -110,6 +115,10 @@ mpe_clone_create(struct if_clone *ifc, int unit) > bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t)); > #endif > > + smpls->smpls_family = AF_MPLS; > + smpls->smpls_len = sizeof(*smpls); > + ifp->if_lladdr->ifa_dstaddr = smplstosa(smpls); > + > s = splnet(); > LIST_INSERT_HEAD(&mpeif_list, mpeif, sc_list); > splx(s); > @@ -127,6 +136,7 @@ mpe_clone_destroy(struct ifnet *ifp) > LIST_REMOVE(mpeif, sc_list); > splx(s); > > + free(ifp->if_lladdr->ifa_dstaddr, M_IFADDR, 0); > if_detach(ifp); > free(mpeif, M_DEVBUF, 0); > return (0); > @@ -278,7 +288,7 @@ int > mpeioctl(struct ifnet *ifp, u_long cmd, caddr_t data) > { > int error; > - struct mpe_softc *ifm; > + struct sockaddr_mpls *smpls; > struct ifreq *ifr; > struct shim_hdr shim; > > @@ -303,14 +313,13 @@ mpeioctl(struct ifnet *ifp, u_long cmd, caddr_t data) > ifp->if_mtu = ifr->ifr_mtu; > break; > case SIOCGETLABEL: > - ifm = ifp->if_softc; > + smpls = satosmpls(ifp->if_lladdr->ifa_dstaddr); > shim.shim_label = > - ((ntohl(ifm->sc_shim.shim_label & MPLS_LABEL_MASK)) >> > + ((ntohl(smpls->smpls_label & MPLS_LABEL_MASK)) >> > MPLS_LABEL_OFFSET); > error = copyout(&shim, ifr->ifr_data, sizeof(shim)); > break; > case SIOCSETLABEL: > - ifm = ifp->if_softc; > if ((error = copyin(ifr->ifr_data, &shim, sizeof(shim)))) > break; > if (shim.shim_label > MPLS_LABEL_MAX || > @@ -319,36 +328,29 @@ mpeioctl(struct ifnet *ifp, u_long cmd, caddr_t data) > break; > } > shim.shim_label = htonl(shim.shim_label << MPLS_LABEL_OFFSET); > - if (ifm->sc_shim.shim_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) { > - error = EEXIST; > - break; > - } > - } > - if (error) > + > + smpls = satosmpls(ifp->if_lladdr->ifa_dstaddr); > + if (smpls->smpls_label == shim.shim_label) > break; > - ifm = ifp->if_softc; > - if (ifm->sc_shim.shim_label) { > - /* remove old MPLS route */ > - mpe_newlabel(ifp, RTM_DELETE, &ifm->sc_shim); > + if (smpls->smpls_label) { > + rt_ifa_del(ifp->if_lladdr, RTF_MPLS | RTF_UP, > + smplstosa(smpls)); > + smpls->smpls_label = 0; > } > - /* add new MPLS route */ > - error = mpe_newlabel(ifp, RTM_ADD, &shim); > - if (error) > - break; > - ifm->sc_shim.shim_label = shim.shim_label; > + > + smpls->smpls_label = shim.shim_label; > + error = rt_ifa_add(ifp->if_lladdr, RTF_MPLS | RTF_UP, > + smplstosa(smpls)); > + if (error != 0) > + smpls->smpls_label = 0; > 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; > - error = mpe_newlabel(ifp, RTM_ADD, &shim); > - } > + smpls = satosmpls(ifp->if_lladdr->ifa_dstaddr); > + if (smpls->smpls_label) > + rt_ifa_add(ifp->if_lladdr, RTF_MPLS | RTF_UP, > + smplstosa(smpls)); > } > /* return with ENOTTY so that the parent handler finishes */ > return (ENOTTY); > @@ -442,38 +444,3 @@ mpe_input6(struct mbuf *m, struct ifnet *ifp, struct > sockaddr_mpls *smpls, > 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); > -} > diff --git sys/net/route.c sys/net/route.c > index 4ac1bd1..9559d89 100644 > --- sys/net/route.c > +++ sys/net/route.c > @@ -1100,6 +1100,11 @@ rt_ifa_add(struct ifaddr *ifa, int flags, struct > sockaddr *dst) > info.rti_info[RTAX_LABEL] = > rtlabel_id2sa(ifa->ifa_ifp->if_rtlabelid, &sa_rl); > > + if ((flags & RTF_MPLS) == RTF_MPLS) { > + info.rti_mpls = MPLS_OP_POP; > + rtableid = 0; > + } > + > if ((flags & RTF_HOST) == 0) > info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask; > > diff --git sys/netmpls/mpls.h sys/netmpls/mpls.h > index 2903aa4..b9085b0 100644 > --- sys/netmpls/mpls.h > +++ sys/netmpls/mpls.h > @@ -149,7 +149,6 @@ extern struct domain mplsdomain; > struct mpe_softc { > struct ifnet sc_if; /* the interface */ > int sc_unit; > - struct shim_hdr sc_shim; > LIST_ENTRY(mpe_softc) sc_list; > }; > >
