Re: iproute2 mpls max labels

[Magnus Bergroth <bergroth@nordu.net>]

> Eric W. Biederman <mailto:ebiederm@xmission.com>
> 21 juli 2016 22:00
> Roopa Prabhu <roopa@cumulusnetworks.com> 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.

<<Magnus