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 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.
> 

I tried to use ifp->if_addrlist, but it looks like this KASSERT in rtsock.c
doesn't like this.
rtsock.c:1322
                TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
                        KASSERT(ifa->ifa_addr->sa_family != AF_LINK);

See sys/net/rtsock.c: revision 1.144
"
Do not put any link-layer address on the per-ifp lists or on the RB-
Tree.
 
Since interfaces only support one link-layer address accessible via the
if_sadl member, there's no need to have it elsewhere.  This improves
various address lookups because the first element of the list, the link-
layer address, won't necessarily be discarded.

Finally remove the empty netmask associated to every link-layer address.
This hack was needed to (ab)use the address & netmask comparison code to
do a strcmp() on the interface name embedded in the sdl_data field.

ok henning@, claudio@
"

So I moved the ifa back to softc.

> >  * 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)

Also changed rt_ifa_del() to remove routes from rdomain 0 as well.

> >  * Removed old code that installed mpe MPLS routes
> >  * Conflicting labels verification is now done by routing (see rt_ifa_add())

I had to put back the old verification for installed routes, since the
rt_ifa_add checks failed to find already existing routes when changing
domains or having mpe interfaces down.

> 
> This looks ok.
> 
> > This was tested in the setup described in:
> > http://2011.eurobsdcon.org/papers/jeker/MPLS.pdf
> > 
> > Here is the diff:
> > --- snipped old diff ---

Updated diff:

Index: 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
--- net/if_mpe.c        22 Jul 2014 11:06:09 -0000      1.35
+++ net/if_mpe.c        4 Oct 2014 22:21:17 -0000
@@ -61,7 +61,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 =
@@ -84,13 +83,19 @@ mpe_clone_create(struct if_clone *ifc, i
 {
        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,12 @@ mpe_clone_create(struct if_clone *ifc, i
        bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
 #endif
 
+       smpls->smpls_family = AF_MPLS;
+       smpls->smpls_len = sizeof(*smpls);
+       mpeif->sc_ifa.ifa_ifp = ifp;
+       mpeif->sc_ifa.ifa_addr = (struct sockaddr *) ifp->if_sadl;
+       mpeif->sc_ifa.ifa_dstaddr = smplstosa(smpls);
+
        s = splnet();
        LIST_INSERT_HEAD(&mpeif_list, mpeif, sc_list);
        splx(s);
@@ -128,6 +139,7 @@ mpe_clone_destroy(struct ifnet *ifp)
        splx(s);
 
        if_detach(ifp);
+       free(mpeif->sc_ifa.ifa_dstaddr, M_IFADDR, 0);
        free(mpeif, M_DEVBUF, 0);
        return (0);
 }
@@ -278,8 +290,9 @@ int
 mpeioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 {
        int                      error;
-       struct mpe_softc        *ifm;
+       struct mpe_softc        *ifm, *ifmn;
        struct ifreq            *ifr;
+       struct sockaddr_mpls    *smpls;
        struct shim_hdr          shim;
 
        ifr = (struct ifreq *)data;
@@ -304,13 +317,15 @@ mpeioctl(struct ifnet *ifp, u_long cmd, 
                break;
        case SIOCGETLABEL:
                ifm = ifp->if_softc;
+               smpls = satosmpls(ifm->sc_ifa.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;
+               smpls = satosmpls(ifm->sc_ifa.ifa_dstaddr);
                if ((error = copyin(ifr->ifr_data, &shim, sizeof(shim))))
                        break;
                if (shim.shim_label > MPLS_LABEL_MAX ||
@@ -319,36 +334,40 @@ 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 (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) {
-                               error = EEXIST;
-                               break;
-                       }
-               }
-               if (error)
+               LIST_FOREACH(ifmn, &mpeif_list, sc_list) {
+                       if (ifm == ifmn)
+                               continue;
+                       if (satosmpls(ifm->sc_ifa.ifa_dstaddr)->smpls_label !=
+                           shim.shim_label)
+                               continue;
+
+                       error = EEXIST;
                        break;
-               ifm = ifp->if_softc;
-               if (ifm->sc_shim.shim_label) {
-                       /* remove old MPLS route */
-                       mpe_newlabel(ifp, RTM_DELETE, &ifm->sc_shim);
                }
-               /* add new MPLS route */
-               error = mpe_newlabel(ifp, RTM_ADD, &shim);
                if (error)
                        break;
-               ifm->sc_shim.shim_label = shim.shim_label;
+               if (smpls->smpls_label) {
+                       rt_ifa_del(&ifm->sc_ifa, RTF_MPLS | RTF_UP,
+                           smplstosa(smpls));
+                       smpls->smpls_label = 0;
+               }
+
+               smpls->smpls_label = shim.shim_label;
+               error = rt_ifa_add(&ifm->sc_ifa, 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;
+               smpls = satosmpls(ifm->sc_ifa.ifa_dstaddr);
                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);
-                       }
+                       if (smpls->smpls_label)
+                               rt_ifa_add(&ifm->sc_ifa, RTF_MPLS | RTF_UP,
+                                   smplstosa(smpls));
                }
                /* return with ENOTTY so that the parent handler finishes */
                return (ENOTTY);
@@ -442,38 +461,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: 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
--- net/route.c 2 Oct 2014 12:21:20 -0000       1.185
+++ net/route.c 4 Oct 2014 22:21:17 -0000
@@ -1100,6 +1100,11 @@ 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;
+               rtableid = 0;
+       }
+
        if ((flags & RTF_HOST) == 0)
                info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
 
@@ -1144,6 +1149,9 @@ rt_ifa_del(struct ifaddr *ifa, int flags
        u_short                  rtableid = ifa->ifa_ifp->if_rdomain;
        u_int8_t                 prio = RTP_CONNECTED;
        int                      error;
+
+       if ((flags & RTF_MPLS) == RTF_MPLS)
+               rtableid = 0;
 
        if ((flags & RTF_HOST) == 0 && ifa->ifa_netmask) {
                m = m_get(M_DONTWAIT, MT_SONAME);
Index: netmpls/mpls.h
===================================================================
RCS file: /home/rzalamena/obsdcvs//src/sys/netmpls/mpls.h,v
retrieving revision 1.28
diff -u -p -r1.28 mpls.h
--- netmpls/mpls.h      24 Apr 2013 10:20:15 -0000      1.28
+++ netmpls/mpls.h      4 Oct 2014 22:21:17 -0000
@@ -148,8 +148,8 @@ 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;
        LIST_ENTRY(mpe_softc)   sc_list;
 };
 

Reply via email to