On Wed, Oct 08, 2014 at 06:54:14PM -0300, Rafael Zalamena wrote:
> On Wed, Oct 08, 2014 at 09:22:44AM +0200, Martin Pieuchot wrote:
> > On 07/10/14(Tue) 18:44, Rafael Zalamena wrote:
> > > On Sat, Oct 04, 2014 at 07:39:03PM -0300, Rafael Zalamena wrote:
> > > > On Thu, Oct 02, 2014 at 02:36:12PM +0200, Martin Pieuchot wrote:
> > > > > On 01/10/14(Wed) 21:54, Rafael Zalamena wrote:
> > > > > > --- old chat snip ---
> > > >
> > >
> > > Code changed:
> > > * Replaced old function that used to create routes in favor of rt_ifa_*
> > > * Modified rt_ifa_{add,del} to handle MPLS addresses: when creating an
> > > route to a MPLS interface it means we want to remove labels. Also MPLS
> > > only works on rdomain 0
> >
> > Even if they only work on rdomain 0, I'd prefer not to add code to
> > enforce this behavior. It's like making it harder for people to make it
> > work any rdomain.
> >
> > Other than that, I'm ok with your diff.
> >
>
> I removed the code that hardcoded RTF_MPLS to rdomain 0, now we use a
> function to handle the rdomain switching to install routes.
>
> Index: sys/net/if_mpe.c
> ===================================================================
> RCS file: /home/rzalamena/obsdcvs/src/sys/net/if_mpe.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 if_mpe.c
> --- sys/net/if_mpe.c 22 Jul 2014 11:06:09 -0000 1.35
> +++ sys/net/if_mpe.c 8 Oct 2014 21:48:15 -0000
> @@ -61,7 +61,7 @@ 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 *);
> +int mpe_iflabelroute(struct ifnet *, struct shim_hdr *, int);
>
> LIST_HEAD(, mpe_softc) mpeif_list;
> struct if_clone mpe_cloner =
> @@ -333,10 +333,10 @@ mpeioctl(struct ifnet *ifp, u_long cmd,
> ifm = ifp->if_softc;
> if (ifm->sc_shim.shim_label) {
> /* remove old MPLS route */
> - mpe_newlabel(ifp, RTM_DELETE, &ifm->sc_shim);
> + mpe_iflabelroute(ifp, &ifm->sc_shim, 0);
> }
> /* add new MPLS route */
> - error = mpe_newlabel(ifp, RTM_ADD, &shim);
> + error = mpe_iflabelroute(ifp, &shim, 1);
> if (error)
> break;
> ifm->sc_shim.shim_label = shim.shim_label;
> @@ -346,8 +346,7 @@ mpeioctl(struct ifnet *ifp, u_long cmd,
> 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);
> + mpe_iflabelroute(ifp, &ifm->sc_shim, 1);
> }
> }
> /* return with ENOTTY so that the parent handler finishes */
> @@ -443,37 +442,29 @@ mpe_input6(struct mbuf *m, struct ifnet
> }
> #endif /* INET6 */
>
> +/*
> + * Install or remove mpe interface label routes using rdomain 0.
> + */
> int
> -mpe_newlabel(struct ifnet *ifp, int cmd, struct shim_hdr *shim)
> +mpe_iflabelroute(struct ifnet *ifp, struct shim_hdr *shim, int add)
> {
> - 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--;
> - }
> + int error;
> + struct sockaddr_mpls smpls;
> + u_short rdomain = ifp->if_rdomain;
> +
> + ifp->if_rdomain = 0;
> +
> + memset(&smpls, 0, sizeof(smpls));
> + smpls.smpls_family = AF_MPLS;
> + smpls.smpls_label = shim->shim_label;
> + smpls.smpls_len = sizeof(smpls);
> + if (add)
> + error = rt_ifa_add(ifp->if_lladdr, RTF_MPLS | RTF_UP,
> + smplstosa(&smpls));
> + else
> + error = rt_ifa_del(ifp->if_lladdr, RTF_MPLS | RTF_UP,
> + smplstosa(&smpls));
> +
> + ifp->if_rdomain = rdomain;
> return (error);
> }
> Index: sys/net/route.c
> ===================================================================
> RCS file: /home/rzalamena/obsdcvs/src/sys/net/route.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 route.c
> --- sys/net/route.c 2 Oct 2014 12:21:20 -0000 1.185
> +++ sys/net/route.c 8 Oct 2014 21:48:15 -0000
> @@ -1100,6 +1100,9 @@ rt_ifa_add(struct ifaddr *ifa, int flags
> 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;
> +
> if ((flags & RTF_HOST) == 0)
> info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
>
Any other feedbacks?
