Re: mpe patch: use rt_ifa_{add,del}
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 - 1.35
> +++ sys/net/if_mpe.c 8 Oct 2014 21:48:15 -
> @@ -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);
>
Re: mpe patch: use rt_ifa_{add,del}
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.c22 Jul 2014 11:06:09 - 1.35
+++ sys/net/if_mpe.c8 Oct 2014 21:48:15 -
@@ -61,7 +61,7 @@ int mpeioctl(struct ifnet *, u_long, cad
void mpestart(struct ifnet *);
intmpe_clone_create(struct if_clone *, int);
intmpe_clone_destroy(struct ifnet *);
-intmpe_newlabel(struct ifnet *, int, struct shim_hdr *);
+intmpe_iflabelroute(struct ifnet *, struct shim_hdr *, int);
LIST_HEAD(, mpe_softc) mpeif_list;
struct if_clonempe_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
dif
Re: mpe patch: use rt_ifa_{add,del}
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.
>
> Here is the new diff based on mpi@ feedback:
> Index: sys/net/if_mpe.c
> ===
> RCS file: /cvs/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 - 1.35
> +++ sys/net/if_mpe.c 7 Oct 2014 21:24:47 -
> @@ -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 =
> @@ -280,6 +279,7 @@ mpeioctl(struct ifnet *ifp, u_long cmd,
> int error;
> struct mpe_softc*ifm;
> struct ifreq*ifr;
> + struct sockaddr_mpls smpls;
> struct shim_hdr shim;
>
> ifr = (struct ifreq *)data;
> @@ -331,12 +331,19 @@ mpeioctl(struct ifnet *ifp, u_long cmd,
> if (error)
> break;
> ifm = ifp->if_softc;
> + memset(&smpls, 0, sizeof(smpls));
> + smpls.smpls_family = AF_MPLS;
> + smpls.smpls_len = sizeof(smpls);
> if (ifm->sc_shim.shim_label) {
> /* remove old MPLS route */
> - mpe_newlabel(ifp, RTM_DELETE, &ifm->sc_shim);
> + smpls.smpls_label = ifm->sc_shim.shim_label;
> + rt_ifa_del(ifp->if_lladdr, RTF_MPLS | RTF_UP,
> + smplstosa(&smpls));
> }
> /* add new MPLS route */
> - error = mpe_newlabel(ifp, RTM_ADD, &shim);
> + smpls.smpls_label = shim.shim_label;
> + error = rt_ifa_add(ifp->if_lladdr, RTF_MPLS | RTF_UP,
> + smplstosa(&smpls));
> if (error)
> break;
> ifm->sc_shim.shim_label = shim.shim_label;
> @@ -346,8 +353,12 @@ 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);
> + memset(&smpls, 0, sizeof(smpls));
> + smpls.smpls_label = ifm->sc_shim.shim_label;
> + smpls.smpls_family = AF_MPLS;
> + smpls.smpls_len = sizeof(smpls);
> + error = rt_ifa_add(ifp->if_lladdr,
> + RTF_MPLS | RTF_UP, smplstosa(&smpls));
> }
> }
> /* return with ENOTTY so that the parent handler finishes */
> @@ -442,38 +453,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
>
Re: mpe patch: use rt_ifa_{add,del}
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
Here is the new diff based on mpi@ feedback:
Index: sys/net/if_mpe.c
===
RCS file: /cvs/src/sys/net/if_mpe.c,v
retrieving revision 1.35
diff -u -p -r1.35 if_mpe.c
--- sys/net/if_mpe.c22 Jul 2014 11:06:09 - 1.35
+++ sys/net/if_mpe.c7 Oct 2014 21:24:47 -
@@ -61,7 +61,6 @@ int mpeioctl(struct ifnet *, u_long, cad
void mpestart(struct ifnet *);
intmpe_clone_create(struct if_clone *, int);
intmpe_clone_destroy(struct ifnet *);
-intmpe_newlabel(struct ifnet *, int, struct shim_hdr *);
LIST_HEAD(, mpe_softc) mpeif_list;
struct if_clonempe_cloner =
@@ -280,6 +279,7 @@ mpeioctl(struct ifnet *ifp, u_long cmd,
int error;
struct mpe_softc*ifm;
struct ifreq*ifr;
+ struct sockaddr_mpls smpls;
struct shim_hdr shim;
ifr = (struct ifreq *)data;
@@ -331,12 +331,19 @@ mpeioctl(struct ifnet *ifp, u_long cmd,
if (error)
break;
ifm = ifp->if_softc;
+ memset(&smpls, 0, sizeof(smpls));
+ smpls.smpls_family = AF_MPLS;
+ smpls.smpls_len = sizeof(smpls);
if (ifm->sc_shim.shim_label) {
/* remove old MPLS route */
- mpe_newlabel(ifp, RTM_DELETE, &ifm->sc_shim);
+ smpls.smpls_label = ifm->sc_shim.shim_label;
+ rt_ifa_del(ifp->if_lladdr, RTF_MPLS | RTF_UP,
+ smplstosa(&smpls));
}
/* add new MPLS route */
- error = mpe_newlabel(ifp, RTM_ADD, &shim);
+ smpls.smpls_label = shim.shim_label;
+ error = rt_ifa_add(ifp->if_lladdr, RTF_MPLS | RTF_UP,
+ smplstosa(&smpls));
if (error)
break;
ifm->sc_shim.shim_label = shim.shim_label;
@@ -346,8 +353,12 @@ 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);
+ memset(&smpls, 0, sizeof(smpls));
+ smpls.smpls_label = ifm->sc_shim.shim_label;
+ smpls.smpls_family = AF_MPLS;
+ smpls.smpls_len = sizeof(smpls);
+ error = rt_ifa_add(ifp->if_lladdr,
+ RTF_MPLS | RTF_UP, smplstosa(&smpls));
}
}
/* return with ENOTTY so that the parent handler finishes */
@@ -442,38 +453,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.185
diff -u -p -r1.185 route.c
--- sys/net/route.c 2 Oct 2014 12:21:20 - 1.185
+++ sys/net/route.c 7 Oct 2014 21:24:47 -
@@ -1100,6 +1100,11 @@ rt_ifa_add(struct ifaddr *ifa, int flags
info.rti
Re: mpe patch: use rt_ifa_{add,del}
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.c22 Jul 2014 11:06:09 - 1.35
+++ net/if_mpe.c4 Oct 2014 22:21:17 -
@@ -61,7 +61,6 @@ int mpeioctl(struct ifnet *, u_long, cad
void mpestart(struct ifnet *);
intmpe_clone_create(struct if_clone *, int);
intmpe_clone_destroy(struct ifnet *);
-intmpe_newlabel(struct ifnet *, int, struct shim_hdr *);
LIST_HEAD(, mpe_softc) mpeif_list;
struct if_clonempe_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));
Re: mpe patch: use rt_ifa_{add,del}
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;
> - LIS
mpe patch: use rt_ifa_{add,del}
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
* 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 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 *);
intmpe_clone_create(struct if_clone *, int);
intmpe_clone_destroy(struct ifnet *);
-intmpe_newlabel(struct ifnet *, int, struct shim_hdr *);
LIST_HEAD(, mpe_softc) mpeif_list;
struct if_clonempe_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);
+
