[PATCH net-next v4 0/2] mpls: multipath support

[PATCH net-next v4 0/2] mpls: multipath support

[Roopa Prabhu]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch adds support for MPLS multipath routes.

Includes following changes to support multipath:
- splits struct mpls_route into 'struct mpls_route + struct mpls_nh'.

- struct mpls_nh represents a mpls nexthop label forwarding entry

- Adds support to parse/fill RTA_MULTIPATH netlink attribute for
multipath routes similar to ipv4/v6 fib

- In the process of restructuring, this patch also consistently changes all
labels to u8

$ip -f mpls route add 100 nexthop as 200 via inet 10.1.1.2 dev swp1 \
		nexthop as 700 via inet 10.1.1.6 dev swp2 \
		nexthop as 800 via inet 40.1.1.2 dev swp3

$ip  -f mpls route show
100 
	nexthop as to 200 via inet 10.1.1.2  dev swp1
	nexthop as to 700 via inet 10.1.1.6  dev swp2
	nexthop as to 800 via inet 40.1.1.2  dev swp3

Roopa Prabhu (1):
  mpls: multipath support

Robert Shearman (1):
  mpls: flow-based multipath selection

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>

----
v2:
	- Incorporate some feedback from Robert:
		use dynamic allocation (list) instead of static allocation
		for nexthops
v3:
	- Move back to arrays (same as v1), also suggested by Eric Biederman

v4:
	- address a few comments from Eric Biederman
	Plan to address the following pending comments in incremental patches after this
	infrastructure changes go in.
	- Move VIA size to 16 bytes
	- use ipv6 flow label in ecmp calculations
	- dead route handling during multipath route selection (I had planned this in
	an incremental patch initially).

 include/net/mpls_iptunnel.h |   2 +-
 net/mpls/af_mpls.c          | 668 ++++++++++++++++++++++++++++++++++----------
 net/mpls/internal.h         |  57 +++-
 3 files changed, 572 insertions(+), 155 deletions(-)

-- 
1.9.1

Re: [PATCH net-next v4 0/2] mpls: multipath support

[David Miller]

From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Sun, 18 Oct 2015 18:20:43 -0700

> This patch adds support for MPLS multipath routes.

Eric, can you please review these changes?

Thanks.

Re: [PATCH net-next v4 0/2] mpls: multipath support

[Eric W. Biederman]

Roopa Prabhu <roopa@cumulusnetworks.com> writes:

> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch adds support for MPLS multipath routes.
>
> Includes following changes to support multipath:
> - splits struct mpls_route into 'struct mpls_route + struct mpls_nh'.
>
> - struct mpls_nh represents a mpls nexthop label forwarding entry
>
> - Adds support to parse/fill RTA_MULTIPATH netlink attribute for
> multipath routes similar to ipv4/v6 fib
>
> - In the process of restructuring, this patch also consistently changes all
> labels to u8
>
> $ip -f mpls route add 100 nexthop as 200 via inet 10.1.1.2 dev swp1 \
> 		nexthop as 700 via inet 10.1.1.6 dev swp2 \
> 		nexthop as 800 via inet 40.1.1.2 dev swp3
>
> $ip  -f mpls route show
> 100 
> 	nexthop as to 200 via inet 10.1.1.2  dev swp1
> 	nexthop as to 700 via inet 10.1.1.6  dev swp2
> 	nexthop as to 800 via inet 40.1.1.2  dev swp3
>
> Roopa Prabhu (1):
>   mpls: multipath support
>
> Robert Shearman (1):
>   mpls: flow-based multipath selection
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> ----
> v2:
> 	- Incorporate some feedback from Robert:
> 		use dynamic allocation (list) instead of static allocation
> 		for nexthops
> v3:
> 	- Move back to arrays (same as v1), also suggested by Eric Biederman
>
> v4:
> 	- address a few comments from Eric Biederman
> 	Plan to address the following pending comments in incremental patches after this
> 	infrastructure changes go in.
> 	- Move VIA size to 16 bytes
> 	- use ipv6 flow label in ecmp calculations
> 	- dead route handling during multipath route selection (I had planned this in
> 	an incremental patch initially).

I don't see anything problematic in the patches the worst
I found is dead code and we can delete that later so
for purposes of moving forward I say:

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

That said we really need dead path handling.  Without handling paths
that go dead this functionality really is pretty much broken.  So if you
can't get that by the merge window we will need to apply a patch to
disable processing of the RTA_MULTIPATH netlink attribute.

Eric

Re: [PATCH net-next v4 0/2] mpls: multipath support

[David Miller]

From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Sun, 18 Oct 2015 18:20:43 -0700

> This patch adds support for MPLS multipath routes.

Please address Rob's feedback and respin, thanks!

Re: [PATCH net-next v4 0/2] mpls: multipath support

[roopa]

On 10/21/15, 9:04 PM, Eric W. Biederman wrote:
> Roopa Prabhu <roopa@cumulusnetworks.com> writes:
>
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch adds support for MPLS multipath routes.
>>
>> Includes following changes to support multipath:
>> - splits struct mpls_route into 'struct mpls_route + struct mpls_nh'.
>>
>> - struct mpls_nh represents a mpls nexthop label forwarding entry
>>
>> - Adds support to parse/fill RTA_MULTIPATH netlink attribute for
>> multipath routes similar to ipv4/v6 fib
>>
>> - In the process of restructuring, this patch also consistently changes all
>> labels to u8
>>
>> $ip -f mpls route add 100 nexthop as 200 via inet 10.1.1.2 dev swp1 \
>> 		nexthop as 700 via inet 10.1.1.6 dev swp2 \
>> 		nexthop as 800 via inet 40.1.1.2 dev swp3
>>
>> $ip  -f mpls route show
>> 100 
>> 	nexthop as to 200 via inet 10.1.1.2  dev swp1
>> 	nexthop as to 700 via inet 10.1.1.6  dev swp2
>> 	nexthop as to 800 via inet 40.1.1.2  dev swp3
>>
>> Roopa Prabhu (1):
>>   mpls: multipath support
>>
>> Robert Shearman (1):
>>   mpls: flow-based multipath selection
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> ----
>> v2:
>> 	- Incorporate some feedback from Robert:
>> 		use dynamic allocation (list) instead of static allocation
>> 		for nexthops
>> v3:
>> 	- Move back to arrays (same as v1), also suggested by Eric Biederman
>>
>> v4:
>> 	- address a few comments from Eric Biederman
>> 	Plan to address the following pending comments in incremental patches after this
>> 	infrastructure changes go in.
>> 	- Move VIA size to 16 bytes
>> 	- use ipv6 flow label in ecmp calculations
>> 	- dead route handling during multipath route selection (I had planned this in
>> 	an incremental patch initially).
> I don't see anything problematic in the patches the worst
> I found is dead code and we can delete that later so
> for purposes of moving forward I say:
>
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> That said we really need dead path handling.  Without handling paths
> that go dead this functionality really is pretty much broken.  So if you
> can't get that by the merge window we will need to apply a patch to
> disable processing of the RTA_MULTIPATH netlink attribute.
I will submit these before the merge window. Since dead routes were not handled for the non-multipath routes, it always was an incremental feature to cover on my TODO list. Understand your concern, I will submit them soon.

thanks.