From: Roopa Prabhu <roopa@cumulusnetworks•com>
To: David Lebrun <david.lebrun@uclouvain•be>
Cc: David Miller <davem@davemloft•net>,
netdev@vger•kernel.org, lorenzo@google•com
Subject: Re: [PATCH net-next v2] ipv6: sr: fix IPv6 initialization failure without lwtunnels
Date: Mon, 14 Nov 2016 06:22:19 -0800 [thread overview]
Message-ID: <5829C89B.7010405@cumulusnetworks.com> (raw)
In-Reply-To: <5828C619.2020008@uclouvain.be>
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
>
next prev parent reply other threads:[~2016-11-14 14:28 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-08 13:57 [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing David Lebrun
2016-11-08 13:57 ` [PATCH net-next v5 1/9] ipv6: implement dataplane support for rthdr type 4 (Segment Routing Header) David Lebrun
2016-11-08 13:57 ` [PATCH net-next v5 2/9] ipv6: sr: add code base for control plane support of SR-IPv6 David Lebrun
2016-11-08 13:57 ` [PATCH net-next v5 3/9] ipv6: sr: add support for SRH encapsulation and injection with lwtunnels David Lebrun
2016-11-08 13:57 ` [PATCH net-next v5 4/9] ipv6: sr: add core files for SR HMAC support David Lebrun
2016-11-08 13:59 ` [PATCH net-next v5 5/9] ipv6: sr: implement API to control SR HMAC structure David Lebrun
2016-11-08 13:59 ` [PATCH net-next v5 6/9] ipv6: sr: add calls to verify and insert HMAC signatures David Lebrun
2016-11-08 13:59 ` [PATCH net-next v5 7/9] ipv6: add source address argument for ipv6_push_nfrag_opts David Lebrun
2016-11-08 13:59 ` [PATCH net-next v5 8/9] ipv6: sr: add support for SRH injection through setsockopt David Lebrun
2016-11-08 13:59 ` [PATCH net-next v5 9/9] ipv6: sr: add documentation file for per-interface sysctls David Lebrun
2016-11-10 1:40 ` [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing David Miller
2016-11-10 8:19 ` David Lebrun
2016-11-10 14:45 ` David Miller
2016-11-10 8:35 ` Lorenzo Colitti
2016-11-10 9:09 ` David Lebrun
2016-11-10 9:14 ` Lorenzo Colitti
2016-11-10 9:26 ` David Lebrun
2016-11-10 9:27 ` Lorenzo Colitti
2016-11-10 9:55 ` [PATCH net-next] ipv6: sr: fix IPv6 initialization failure without lwtunnels David Lebrun
2016-11-10 11:20 ` kbuild test robot
2016-11-10 12:32 ` kbuild test robot
2016-11-10 12:26 ` [PATCH net-next v2] " David Lebrun
2016-11-13 5:20 ` David Miller
2016-11-13 5:23 ` David Miller
2016-11-13 19:59 ` David Lebrun
2016-11-14 14:22 ` Roopa Prabhu [this message]
2016-11-15 10:17 ` David Lebrun
2016-11-15 15:18 ` David Miller
2016-11-16 15:49 ` Roopa Prabhu
2016-11-16 16:31 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5829C89B.7010405@cumulusnetworks.com \
--to=roopa@cumulusnetworks$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=david.lebrun@uclouvain$(echo .)be \
--cc=lorenzo@google$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox