netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] mpls: multipath support
@ 2015-08-11 21:45 Roopa Prabhu
  2015-08-12 17:30 ` Robert Shearman
  0 siblings, 1 reply; 4+ messages in thread
From: Roopa Prabhu @ 2015-08-11 21:45 UTC (permalink / raw)
  To: davem; +Cc: ebiederm, rshearma, netdev

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch series adds multipath support to mpls routes.

resembles ipv4 multipath support. The multipath route nexthop
selection algorithm is the same code as in ipv4 fib code.

I understand that the multipath algorithm in ipv4 is undergoing
some changes and will move mpls to similar algo if applicable once
those get merged.

mpls multipath support can be moved under CONFIG_MPLS_ROUTE_MULTIPATH if
needed similar to CONFIG_IP_ROUTE_MULTIPATH. I started with that
but that resulted in too many #ifdef CONFIG_MPLS_ROUTE_MULTIPATH
throughout the af_mpls code. If there is a strong reason
to introduce a config option, I will respin v2 with
CONFIG_MPLS_ROUTE_MULTIPATH. These multipath patches do not introduce
any UAPI changes.

example iproute2 usage:
$ip -f mpls route add 100 nexthop as 200 via inet 10.1.1.2 dev swp1 \
	nexthop as 300 via inet 10.1.1.6 dev swp2

$ip -f mpls route show
100 
	nexthop as to 200 via inet 10.1.1.2  dev swp1
	nexthop as to 300 via inet 10.1.1.6  dev swp2


Roopa Prabhu (3):
  mpls: move mpls_route nexthop fields to a new nhlfe struct
  mpls: consistently use u8 to store number of labels
  mpls: add multipath route support

 include/net/mpls_iptunnel.h |    2 +-
 net/mpls/af_mpls.c          |  519 ++++++++++++++++++++++++++++++++-----------
 net/mpls/internal.h         |   44 +++-
 3 files changed, 437 insertions(+), 128 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next 0/3] mpls: multipath support
  2015-08-11 21:45 [PATCH net-next 0/3] mpls: multipath support Roopa Prabhu
@ 2015-08-12 17:30 ` Robert Shearman
  2015-08-13  2:07   ` roopa
  0 siblings, 1 reply; 4+ messages in thread
From: Robert Shearman @ 2015-08-12 17:30 UTC (permalink / raw)
  To: Roopa Prabhu, davem; +Cc: ebiederm, netdev

On 11/08/15 22:45, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch series adds multipath support to mpls routes.
>
> resembles ipv4 multipath support. The multipath route nexthop
> selection algorithm is the same code as in ipv4 fib code.
>
> I understand that the multipath algorithm in ipv4 is undergoing
> some changes and will move mpls to similar algo if applicable once
> those get merged.

Is it necessary for the mpls patch selection algorithm to closely 
resemble the ipv4 one? A flow based algorithm would be much better for 
traffic that is sensitive to re-ordering (e.g TCP, L2VPN) and IMHO we 
should do this from the start for MPLS.

I've also been looking at implementing this functionality. I've got a 
set of patches for this that I can send if you'd like.

>
> mpls multipath support can be moved under CONFIG_MPLS_ROUTE_MULTIPATH if
> needed similar to CONFIG_IP_ROUTE_MULTIPATH. I started with that
> but that resulted in too many #ifdef CONFIG_MPLS_ROUTE_MULTIPATH
> throughout the af_mpls code. If there is a strong reason
> to introduce a config option, I will respin v2 with
> CONFIG_MPLS_ROUTE_MULTIPATH. These multipath patches do not introduce
> any UAPI changes.

I agree with not having such a config option - I can't see a strong 
reason for it.

Thanks,
Rob

>
> example iproute2 usage:
> $ip -f mpls route add 100 nexthop as 200 via inet 10.1.1.2 dev swp1 \
> 	nexthop as 300 via inet 10.1.1.6 dev swp2
>
> $ip -f mpls route show
> 100
> 	nexthop as to 200 via inet 10.1.1.2  dev swp1
> 	nexthop as to 300 via inet 10.1.1.6  dev swp2
>
>
> Roopa Prabhu (3):
>    mpls: move mpls_route nexthop fields to a new nhlfe struct
>    mpls: consistently use u8 to store number of labels
>    mpls: add multipath route support
>
>   include/net/mpls_iptunnel.h |    2 +-
>   net/mpls/af_mpls.c          |  519 ++++++++++++++++++++++++++++++++-----------
>   net/mpls/internal.h         |   44 +++-
>   3 files changed, 437 insertions(+), 128 deletions(-)
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next 0/3] mpls: multipath support
  2015-08-12 17:30 ` Robert Shearman
@ 2015-08-13  2:07   ` roopa
  2015-08-13 13:46     ` Robert Shearman
  0 siblings, 1 reply; 4+ messages in thread
From: roopa @ 2015-08-13  2:07 UTC (permalink / raw)
  To: Robert Shearman; +Cc: davem, ebiederm, netdev

On 8/12/15, 10:30 AM, Robert Shearman wrote:
> On 11/08/15 22:45, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch series adds multipath support to mpls routes.
>>
>> resembles ipv4 multipath support. The multipath route nexthop
>> selection algorithm is the same code as in ipv4 fib code.
>>
>> I understand that the multipath algorithm in ipv4 is undergoing
>> some changes and will move mpls to similar algo if applicable once
>> those get merged.
>
> Is it necessary for the mpls patch selection algorithm to closely 
> resemble the ipv4 one? 
No, It is not necessary. I picked that because it was already there. And 
I see that ipv4 is also getting some new multipath algorithms
(https://marc.info/?l=linux-api&m=143457208315573&w=2). I wanted to move 
to the new RT_MP infra if that becomes applicable in the future.

> A flow based algorithm would be much better for traffic that is 
> sensitive to re-ordering (e.g TCP, L2VPN) and IMHO we should do this 
> from the start for MPLS.
>
> I've also been looking at implementing this functionality. I've got a 
> set of patches for this that I can send if you'd like.

Definitely. But, It seems like you can also submit incremental patches 
to mine. You can replace the current algo with a hash based with your 
patches.
If that does not work for you and if you want me to merge with this 
series that works too.

Thanks!,
Roopa

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next 0/3] mpls: multipath support
  2015-08-13  2:07   ` roopa
@ 2015-08-13 13:46     ` Robert Shearman
  0 siblings, 0 replies; 4+ messages in thread
From: Robert Shearman @ 2015-08-13 13:46 UTC (permalink / raw)
  To: roopa; +Cc: davem, ebiederm, netdev

On 13/08/15 03:07, roopa wrote:
> On 8/12/15, 10:30 AM, Robert Shearman wrote:
>> On 11/08/15 22:45, Roopa Prabhu wrote:
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This patch series adds multipath support to mpls routes.
>>>
>>> resembles ipv4 multipath support. The multipath route nexthop
>>> selection algorithm is the same code as in ipv4 fib code.
>>>
>>> I understand that the multipath algorithm in ipv4 is undergoing
>>> some changes and will move mpls to similar algo if applicable once
>>> those get merged.
>>
>> Is it necessary for the mpls patch selection algorithm to closely
>> resemble the ipv4 one?
> No, It is not necessary. I picked that because it was already there. And
> I see that ipv4 is also getting some new multipath algorithms
> (https://marc.info/?l=linux-api&m=143457208315573&w=2). I wanted to move
> to the new RT_MP infra if that becomes applicable in the future.

The MPLS code doesn't have the binary compatibility requirement that the 
IPv4 path does, so there isn't so much of a need for the algorithm to be 
configurable, provided the default is reasonable. Unless you have a use 
case in mind that would particularly suited to the round-robin algorithm?

>
>> A flow based algorithm would be much better for traffic that is
>> sensitive to re-ordering (e.g TCP, L2VPN) and IMHO we should do this
>> from the start for MPLS.
>>
>> I've also been looking at implementing this functionality. I've got a
>> set of patches for this that I can send if you'd like.
>
> Definitely. But, It seems like you can also submit incremental patches
> to mine. You can replace the current algo with a hash based with your
> patches.

With a flow-based algorithm if there's no need to support weighted paths 
then there's no need to iterate through the nexthops to work out which 
one should be used and, therefore, there is a performance benefit.

So my patches implement a flow-based path selection without support for 
weighted paths. This is similar way to how the IPv6 path selection 
works. The user can still do UCMP with this mechanism, but they have to 
add the same nexthop multiple times. I don't know if this trade-off is 
worth it, but the benefit is that we can always add support for weighted 
paths in the future, whereas removing support for weighted paths would 
be harder due to compatibility concerns.

Therefore, if I rebased my patches on top of yours I would be removing 
code managing the weighting that you will have just added. Not sure if 
that is desirable.

> If that does not work for you and if you want me to merge with this
> series that works too.

I think that would work better. I'll send you a patch against your 
current series.

Thanks,
Rob

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-08-13 13:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-11 21:45 [PATCH net-next 0/3] mpls: multipath support Roopa Prabhu
2015-08-12 17:30 ` Robert Shearman
2015-08-13  2:07   ` roopa
2015-08-13 13:46     ` Robert Shearman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).