From mboxrd@z Thu Jan 1 00:00:00 1970 From: Magnus Bergroth Subject: Re: iproute2 mpls max labels Date: Thu, 21 Jul 2016 23:08:18 +0200 Message-ID: <579139C2.1050804@nordu.net> References: <578A7BF0.2020107@nordu.net> <57911A26.3080203@cumulusnetworks.com> <8737n23goi.fsf@x220.int.ebiederm.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Roopa Prabhu , netdev@vger.kernel.org, Robert Shearman To: "Eric W. Biederman" Return-path: Received: from e-mailfilter02.sunet.se ([192.36.171.202]:52576 "EHLO e-mailfilter02.sunet.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754402AbcGUVIh (ORCPT ); Thu, 21 Jul 2016 17:08:37 -0400 In-Reply-To: <8737n23goi.fsf@x220.int.ebiederm.org> Sender: netdev-owner@vger.kernel.org List-ID: > Eric W. Biederman > 21 juli 2016 22:00 > Roopa Prabhu writes: > >> On 7/16/16, 11:24 AM, Magnus Bergroth wrote: >>> Wanted to use more than the default maximum of 8 mpls labels. Max labels >>> seems to be hardcode to 8 in two places. >>> >>> --- iproute2-4.6.0/lib/utils.c 2016-05-18 20:56:02.000000000 +0200 >>> +++ iproute2-4.6.0-bergroth/lib/utils.c 2016-07-16 20:12:10.714958071 >>> +0200 >>> @@ -476,7 +476,7 @@ >>> addr->bytelen = 4; >>> addr->bitlen = 20; >>> /* How many bytes do I need? */ >>> - for (i = 0; i < 8; i++) { >>> + for (i = 0; i < MPLS_MAX_LABELS; i++) { >>> if (ntohl(addr->data[i]) & MPLS_LS_S_MASK) { >>> addr->bytelen = (i + 1)*4; >>> break; >>> >>> >>> --- iproute2-4.6.0/include/utils.h 2016-05-18 20:56:02.000000000 +0200 >>> +++ iproute2-4.6.0-bergroth/include/utils.h 2016-07-15 >>> 11:55:57.297681742 +0200 >>> @@ -54,6 +54,9 @@ >>> #define NEXT_ARG_FWD() do { argv++; argc--; } while(0) >>> #define PREV_ARG() do { argv--; argc++; } while(0) >>> >>> +/* Maximum number of labels the mpls helpers support */ >>> +#define MPLS_MAX_LABELS 8 >>> + >>> typedef struct >>> { >>> __u16 flags; >>> @@ -61,7 +64,7 @@ >>> __s16 bitlen; >>> /* These next two fields match rtvia */ >>> __u16 family; >>> - __u32 data[8]; >>> + __u32 data[MPLS_MAX_LABELS]; >>> } inet_prefix; >>> > > This structure is not MPLS specific so that is not appropriate to use > MPLS_MAX_LABELS when definiting the structure. Likewise changing this > structure and limiting the changes to mpls parts of the code is not > appropriate. > >>> #define PREFIXLEN_SPECIFIED 1 >>> @@ -88,9 +91,6 @@ >>> # define AF_MPLS 28 >>> #endif >>> >>> -/* Maximum number of labels the mpls helpers support */ >>> -#define MPLS_MAX_LABELS 8 >>> - >>> __u32 get_addr32(const char *name); >>> int get_addr_1(inet_prefix *dst, const char *arg, int family); >>> int get_prefix_1(inet_prefix *dst, char *arg, int family); >>> >>> >> I did not realize it is hardcoded to 8 in iproute2. Because kernel has a hard coded limit of >> 2. >> I think we need to fix it in a few places: >> a) we should move the kernel #define to a uapi header file which iproute2 can use >> b) there has been a general ask to bump the kernel MAX_LABELS from 2 and I don't see >> a problem with it yet. so, we could bump it to 8. >> >> Were you planning to post patches for one or both of the above ?. >> >> I can post them too. Let me know. > > a) I just looked and the kernel netlink protocol does not have a limit. > The kernel does have a limit but the netlink protocol does not so > there is no point in exporting a limit in a uapi header, it will > just be out of date and wrong. > > b) I can see in principle bumping up the kernels MAX_LABELS past two > although I haven't heard those requests, or understand the use cases. > I don't recall seeing any ducumentation on cases where it is > desirable to push a lot of labels at once. (Do hardware > implementations support pushing a lot of labels at once?) > > Bumping past 8 seems quite a lot. That starts feeling like people > trying to break other peoples mpls stacks. That is asking for more > packet space for labels than ipv6 uses for addresses and ipv6 is way > oversized. The commonly agreed wisdom is the world only needs 40 to > 48 bits to route on to reach the entire world. I think that 8 would be more than enough for most use cases, even 6 or 4 would be sufficient. I'm looking at doing MPLS source routing based on a label-stack. Each router in the network will get a set of static routes that pop the label and sends it out to the next router based on the label that gets poped. I have no problem compiling a special build with the MAX_LABELS set to my need. I just noticed that changing only the MAX_LABELS wasn't enough to get more than 8 labels to work with iproute2 after changing the kernel "MAX_NEW_LABELS 2" in include/net/mpls_iptunnel.h and net/mpls/internal.h to a higher number. <