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;
};
