Re: [PATCH net-next v2] ipv6: sr: fix IPv6 initialization failure without lwtunnels

[Roopa Prabhu <roopa@cumulusnetworks.com>]

On 11/13/16, 11:59 AM, David Lebrun wrote:
> On 11/13/2016 06:23 AM, David Miller wrote:
>> This seems like such a huge mess, quite frankly.
>>
>> IPV6-SR has so many strange dependencies, a weird Kconfig option that is
>> simply controlling what a responsible sysadmin should be allow to do if
>> he chooses anyways.
>>
>> Every distribution is going to say "¯\_(ツ)_/¯" and just turn the thing
>> on in their builds.
> Indeed, the issue is that seg6_iptunnel.o was included in obj-y instead
> of ipv6-y, triggering the bug when CONFIG_IPV6=m. Fixed with the
> following modification to the patch (tested with allyesconfig and
> allmodconfig):
>
> diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
> index 8979d53..a233136 100644
> --- a/net/ipv6/Makefile
> +++ b/net/ipv6/Makefile
> @@ -53,6 +53,6 @@ obj-$(subst m,y,$(CONFIG_IPV6)) += inet6_hashtables.o
>
>  ifneq ($(CONFIG_IPV6),)
>  obj-$(CONFIG_NET_UDP_TUNNEL) += ip6_udp_tunnel.o
> -obj-$(CONFIG_LWTUNNEL) += seg6_iptunnel.o
> +ipv6-$(CONFIG_LWTUNNEL) += seg6_iptunnel.o
>  obj-y += mcast_snoop.o
>  endif


>
> I agree with you that the way to combine the dependencies is strange,
> even if they are very few. The part of the IPv6-SR patch that is enabled
> by default depends on two things: IPV6 and LWTUNNEL. The problem is that
> LWTUNNEL does not depend on IPV6 and is not necessarily enabled. To fix
> the bug reported by Lorenzo, I propose to select one the three following
> solutions:
>
> 1. Make LWTUNNEL always enabled (removing the option).
>    Pros: remove an option
>    Cons: add always-enabled code
>
> 2. Create an option IPV6_SEG6_LWTUNNEL, which would select LWTUNNEL and
> enable the compilation of seg6_iptunnel.o.
>    Pros: logically dissociate the part of IPv6-SR that depends on
> LWTUNNEL from the core patch and simplifies compilation
>    Cons: add an option

I prefer option b). most LWTUNNEL encaps are done this way.

seg6 and seg6_iptunnel is new segment routing code and can be under CONFIG_IPV6_SEG6 which depends on CONFIG_LWTUNNEL and CONFIG_IPV6. CONFIG_IPV6_SEG6_HMAC could then depend on CONFIG_IPV6_SEG6



>
> 3. Apply the proposed patch with the fix
>    Pros: do not modify options
>    Cons: weird conditional compilation
>
> What do you think ?
>
> David
>