| Message ID | 1497058292-10099-3-git-send-email-kjlx@templeofstupid.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Fri, 9 Jun 2017 18:31:31 -0700 Krister Johansen <kjlx@templeofstupid.com> wrote: > diff --git a/include/utils.h b/include/utils.h > index bfbc9e6..60ffde4 100644 > --- a/include/utils.h > +++ b/include/utils.h > @@ -87,6 +87,9 @@ struct ipx_addr { > #ifndef AF_MPLS > # define AF_MPLS 28 > #endif > +#ifndef IPPROTO_MPLS > +#define IPPROTO_MPLS 137 > +#endif > I am a little concerned that this definition may end up being different between kernel and iproute2. It looks like utils.h already has lots of duplicate definitions of things that are in standard include directory. Most of these like IPSEC and DECNET are old, but MPLS might get changed in the process of going into glibc. Is there anyway to get this from the kernel headers which are cloned into include/linux/ already?
On Wed, Jun 14, 2017 at 10:02:11AM -0700, Stephen Hemminger wrote: > On Fri, 9 Jun 2017 18:31:31 -0700 > Krister Johansen <kjlx@templeofstupid.com> wrote: > > > diff --git a/include/utils.h b/include/utils.h > > index bfbc9e6..60ffde4 100644 > > --- a/include/utils.h > > +++ b/include/utils.h > > @@ -87,6 +87,9 @@ struct ipx_addr { > > #ifndef AF_MPLS > > # define AF_MPLS 28 > > #endif > > +#ifndef IPPROTO_MPLS > > +#define IPPROTO_MPLS 137 > > +#endif > > > > I am a little concerned that this definition may end up being different > between kernel and iproute2. It looks like utils.h already has lots of duplicate > definitions of things that are in standard include directory. > > Most of these like IPSEC and DECNET are old, but MPLS might get changed > in the process of going into glibc. > > Is there anyway to get this from the kernel headers which are cloned > into include/linux/ already? I did try to fix this up as part of bringing this patch up to date, since it was one of the concerns that David raised too. I believe the problem that I ran into was that IPPROTO_MPLS wasn't defined in all versions of the headers where I tried to include them, and by bringing in in.h, I also managed to get a bunch of errors around re-definition of other symbols. That said, I don't believe that 137 as the IPPROTO_MPLS value should change anytime soon. It's defined in RFC 4023. https://tools.ietf.org/html/rfc4023 However, if this is still seems problematic, I can take another shot at attempting to clean this up further. -K
On 6/14/17 11:11 AM, Krister Johansen wrote: > I did try to fix this up as part of bringing this patch up to date, > since it was one of the concerns that David raised too. I believe the > problem that I ran into was that IPPROTO_MPLS wasn't defined in all > versions of the headers where I tried to include them, and by bringing > in in.h, I also managed to get a bunch of errors around re-definition of > other symbols. > > That said, I don't believe that 137 as the IPPROTO_MPLS value should > change anytime soon. It's defined in RFC 4023. > > https://tools.ietf.org/html/rfc4023 > > However, if this is still seems problematic, I can take another shot at > attempting to clean this up further. IPPROTO_MPLS is defined in include/linux/in.h in iproute2.
On Wed, Jun 14, 2017 at 11:16:20AM -0600, David Ahern wrote: > On 6/14/17 11:11 AM, Krister Johansen wrote: > > I did try to fix this up as part of bringing this patch up to date, > > since it was one of the concerns that David raised too. I believe the > > problem that I ran into was that IPPROTO_MPLS wasn't defined in all > > versions of the headers where I tried to include them, and by bringing > > in in.h, I also managed to get a bunch of errors around re-definition of > > other symbols. > > > > That said, I don't believe that 137 as the IPPROTO_MPLS value should > > change anytime soon. It's defined in RFC 4023. > > > > https://tools.ietf.org/html/rfc4023 > > > > However, if this is still seems problematic, I can take another shot at > > attempting to clean this up further. > > IPPROTO_MPLS is defined in include/linux/in.h in iproute2. Right. I poked at this some more yesterday afternoon. The IPPROTO enum in include/linux/in.h is wrapped in an #if __UAPI_DEF_IN_IPPROTO. That's defined in include/linux/libc-compat.h. If _NETINET_IN_H is defined, then __UAPI_DEF_IN_IPPROTO is undefined. In that case, we pick up the the IPPROTO defs from netinet/in.h. The addition of IPPROTO_MPLS to utils.h is to cover the case where we're using a system netinet/in.h that's older and doesn't yet have an IPPROTO_MPLS defintion. There seems to be prescedent for this, since utils.h also includes IPPROTO_ESP, IPPROTO_AH, and IPPROTO_COMP. I spent some time trying to remove includes of netinet/in.h from the code in this patch, however, it's also included by other system headers that are included throughout iproute2. I'm not saying removing this is impossible, but it certainly seems like it's beyond the scope of this particular patch. -K
diff --git a/include/utils.h b/include/utils.h index bfbc9e6..60ffde4 100644 --- a/include/utils.h +++ b/include/utils.h @@ -87,6 +87,9 @@ struct ipx_addr { #ifndef AF_MPLS # define AF_MPLS 28 #endif +#ifndef IPPROTO_MPLS +#define IPPROTO_MPLS 137 +#endif __u32 get_addr32(const char *name); int get_addr_1(inet_prefix *dst, const char *arg, int family); diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c index 2f74d9b..cf3a9ef 100644 --- a/ip/link_iptnl.c +++ b/ip/link_iptnl.c @@ -16,6 +16,7 @@ #include <sys/socket.h> #include <arpa/inet.h> +#include <linux/in.h> #include <linux/ip.h> #include <linux/if_tunnel.h> #include "rt_names.h" @@ -47,9 +48,8 @@ static void print_usage(FILE *f, int sit) type ); if (sit) { - fprintf(f, - " [ mode { ip6ip | ipip | any } ]\n" - " [ isatap ]\n"); + fprintf(f, " [ mode { ip6ip | ipip | mplsip | any } ]\n"); + fprintf(f, " [ isatap ]\n"); } fprintf(f, " [ external ]\n"); fprintf(f, " [ fwmark MARK ]\n"); @@ -243,6 +243,9 @@ get_failed: strcmp(*argv, "ipip") == 0 || strcmp(*argv, "ip4ip4") == 0) proto = IPPROTO_IPIP; + else if (strcmp(*argv, "mpls/ipv4") == 0 || + strcmp(*argv, "mplsip") == 0) + proto = IPPROTO_MPLS; else if (strcmp(*argv, "any/ipv4") == 0 || strcmp(*argv, "any") == 0) proto = 0; diff --git a/ip/tunnel.c b/ip/tunnel.c index 7956d71..d359eb9 100644 --- a/ip/tunnel.c +++ b/ip/tunnel.c @@ -54,6 +54,9 @@ const char *tnl_strproto(__u8 proto) case IPPROTO_ESP: strcpy(buf, "esp"); break; + case IPPROTO_MPLS: + strcpy(buf, "mpls"); + break; case 0: strcpy(buf, "any"); break; diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in index 3cc2f5d..994b539 100644 --- a/man/man8/ip-link.8.in +++ b/man/man8/ip-link.8.in @@ -662,7 +662,7 @@ the following additional arguments are supported: ] [ .I " [no]encap-remcsum " ] [ -.I " mode " { ip6ip | ipip | any } " +.I " mode " { ip6ip | ipip | mplsip | any } " ] .in +8 @@ -700,10 +700,11 @@ encapsulation. applicable for Generic UDP Encapsulation. .sp -.BI mode " { ip6ip | ipip | any } " +.BI mode " { ip6ip | ipip | mplsip | any } " - specifies mode in which device should run. "ip6ip" indicates -IPv6-Over-IPv4, "ipip" indicates "IPv4-Over-IPv4", "any" indicates either -IPv6 or IPv4 Over IPv4. Only supported for SIT where the default is "ip6ip". +IPv6-Over-IPv4, "ipip" indicates "IPv4-Over-IPv4", "mplsip" indicates +MPLS-Over-IPv4, "any" indicates IPv6, IPv4 or MPLS Over IPv4. Only +supported for SIT where the default is "ip6ip". .in -8
Original-Author: Simon Horman <simon.horman@netronome.com> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com> --- include/utils.h | 3 +++ ip/link_iptnl.c | 9 ++++++--- ip/tunnel.c | 3 +++ man/man8/ip-link.8.in | 9 +++++---- 4 files changed, 17 insertions(+), 7 deletions(-)