[PATCH net-next v5 0/2] af_mpls: fix undefined reference to ip6_route_output with CONFIG_IPV6=n

[PATCH net-next v5 0/2] af_mpls: fix undefined reference to ip6_route_output with CONFIG_IPV6=n

[Roopa Prabhu]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch series uses ipv6_stub_impl.ipv6_dst_lookup instead of 
ip6_route_output. Follows the vxlan drivers usage of
ipv6_stub_impl.ipv6_dst_lookup.

There is no sk in the af_mpls context from where
ipv6_stub_impl.ipv6_dst_lookup is used. sk appears to be needed
to get the namespace 'net' and is optional otherwise. This patch series
changes ipv6_stub_impl.ipv6_dst_lookup to take net argument. sk remains
optional.

The case of CONFIG_IPV6=m and MPLS_ROUTING=y is covered by checking
if ipv6_stub is not NULL. I have tested this case for proper return
values to the user. (I dont see an ipv6_stub null check in
the vxlan driver. I will test it separately and submit a patch
for vxlan driver if needed).

v1 - v2: use IS_BUILTIN

v2 - v3: Use new Kconfig option that depends on (IPV6 || IPV6=n) as
	 suggested by Dave. Also uses IS_ERR as suggested by Thomas.

v3 - v4: Include missed case of (MPLS_ROUTING=y && IPV6=m) reported by
         Dave.

v4 - v5: Use ipv6_stub_impl.ipv6_dst_lookup as suggested by Hannes


Dave, v4 uses a new Kconfig option and v5 uses ipv6_stub_impl.ipv6_dst_lookup
which looks like was added for vxlan driver for similar use case. Thanks and
apologies for the iterations on this.

Roopa Prabhu (2):
  ipv6: change ipv6_stub_impl.ipv6_dst_lookup to take net argument
  af_mpls: fix undefined reference to ip6_route_output

 drivers/net/vxlan.c    |    2 +-
 include/net/addrconf.h |    4 ++--
 include/net/ipv6.h     |    3 ++-
 net/ipv6/icmp.c        |    6 +++---
 net/ipv6/ip6_output.c  |   15 ++++++++-------
 net/mpls/af_mpls.c     |   39 +++++++++++++++++++++++++++++++--------
 net/tipc/udp_media.c   |    3 ++-
 7 files changed, 49 insertions(+), 23 deletions(-)

-- 
1.7.10.4

Re: [PATCH net-next v5 0/2] af_mpls: fix undefined reference to ip6_route_output with CONFIG_IPV6=n

[David Miller]

From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Tue, 28 Jul 2015 15:27:39 -0700

> v4 - v5: Use ipv6_stub_impl.ipv6_dst_lookup as suggested by Hannes

I think this might not work.

The ipv6_stub pointer is NULL until the ipv6 module is loaded.

VXLAN can safely call through ipv6_stub->foo() because it _knows_ the
ipv6 module has been loaded, and will not go away, because it is
working with an ipv6 socket.

You don't have that set of preconditions here, and thus it is pretty
easy to generate a scenerio where ipv6_stub will be NULL at your call
sites.

This is starting to get really messy, to be honest.  And we've been
harboring this build failure for several days now.

Re: [PATCH net-next v5 0/2] af_mpls: fix undefined reference to ip6_route_output with CONFIG_IPV6=n

[roopa]

On 7/29/15, 10:42 PM, David Miller wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> Date: Tue, 28 Jul 2015 15:27:39 -0700
>
>> v4 - v5: Use ipv6_stub_impl.ipv6_dst_lookup as suggested by Hannes
> I think this might not work.
>
> The ipv6_stub pointer is NULL until the ipv6 module is loaded.
>
> VXLAN can safely call through ipv6_stub->foo() because it _knows_ the
> ipv6 module has been loaded, and will not go away, because it is
> working with an ipv6 socket.
>
> You don't have that set of preconditions here, and thus it is pretty
> easy to generate a scenerio where ipv6_stub will be NULL at your call
> sites.

v4 seems to be a better/safe option in that case. I can resubmit v4 if 
that is acceptable.

>
> This is starting to get really messy, to be honest.  And we've been
> harboring this build failure for several days now.
>
please revert the commit that introduced the IPV6 dependency. I will 
resubmit if needed.
I don't feel good about the build failure either.

Thanks and apologies.

commit 01faef2cebae02685e2bcfc9bbee8416d5ec19fc
Author: Roopa Prabhu <roopa@cumulusnetworks.com>
Date:   Tue Jul 21 09:16:24 2015 -0700

     mpls: make RTA_OIF optional

     If user did not specify an oif, try and get it from the via address.
     If failed to get device, return with -ENODEV.

Re: [PATCH net-next v5 0/2] af_mpls: fix undefined reference to ip6_route_output with CONFIG_IPV6=n

[Hannes Frederic Sowa]

On Thu, 2015-07-30 at 06:22 -0700, roopa wrote:
> On 7/29/15, 10:42 PM, David Miller wrote:
> > From: Roopa Prabhu <roopa@cumulusnetworks.com>
> > Date: Tue, 28 Jul 2015 15:27:39 -0700
> > 
> > > v4 - v5: Use ipv6_stub_impl.ipv6_dst_lookup as suggested by Hannes
> > I think this might not work.
> > 
> > The ipv6_stub pointer is NULL until the ipv6 module is loaded.
> > 
> > VXLAN can safely call through ipv6_stub->foo() because it _knows_ 
> > the
> > ipv6 module has been loaded, and will not go away, because it is
> > working with an ipv6 socket.
> > 
> > You don't have that set of preconditions here, and thus it is pretty
> > easy to generate a scenerio where ipv6_stub will be NULL at your 
> > call
> > sites.
> 
> v4 seems to be a better/safe option in that case. I can resubmit v4 if 
> that is acceptable.
> 
> > 
> > This is starting to get really messy, to be honest.  And we've been
> > harboring this build failure for several days now.
> > 
> please revert the commit that introduced the IPV6 dependency. I will 
> resubmit if needed.
> I don't feel good about the build failure either.
> 
> Thanks and apologies.

Of course you have to guard the NULL pointer dereference. Something simple like that will do:

--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -107,7 +107,15 @@ int inet6addr_notifier_call_chain(unsigned long val, void *v)
 }
 EXPORT_SYMBOL(inet6addr_notifier_call_chain);
 
-const struct ipv6_stub *ipv6_stub __read_mostly;
+static int eafnosupport_ipv6_dst_lookup(struct sock *u1, struct dst_entry **u2,
+                                       struct flowi6 *u3)
+{
+       return -EAFNOSUPPORT;
+}
+
+const struct ipv6_stub *ipv6_stub __read_mostly = &(struct ipv6_stub){
+       .ipv6_dst_lookup = eafnosupport_ipv6_dst_lookup,
+};
 EXPORT_SYMBOL_GPL(ipv6_stub);
 
 /* IPv6 Wildcard Address and Loopback Address defined by RFC2553 */


Bye,
Hannes

Re: [PATCH net-next v5 0/2] af_mpls: fix undefined reference to ip6_route_output with CONFIG_IPV6=n

[roopa]

On 7/30/15, 7:57 AM, Hannes Frederic Sowa wrote:
> On Thu, 2015-07-30 at 06:22 -0700, roopa wrote:
>> On 7/29/15, 10:42 PM, David Miller wrote:
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>> Date: Tue, 28 Jul 2015 15:27:39 -0700
>>>
>>>> v4 - v5: Use ipv6_stub_impl.ipv6_dst_lookup as suggested by Hannes
>>> I think this might not work.
>>>
>>> The ipv6_stub pointer is NULL until the ipv6 module is loaded.
>>>
>>> VXLAN can safely call through ipv6_stub->foo() because it _knows_
>>> the
>>> ipv6 module has been loaded, and will not go away, because it is
>>> working with an ipv6 socket.
>>>
>>> You don't have that set of preconditions here, and thus it is pretty
>>> easy to generate a scenerio where ipv6_stub will be NULL at your
>>> call
>>> sites.
>> v4 seems to be a better/safe option in that case. I can resubmit v4 if
>> that is acceptable.
>>
>>> This is starting to get really messy, to be honest.  And we've been
>>> harboring this build failure for several days now.
>>>
>> please revert the commit that introduced the IPV6 dependency. I will
>> resubmit if needed.
>> I don't feel good about the build failure either.
>>
>> Thanks and apologies.
> Of course you have to guard the NULL pointer dereference. Something simple like that will do:
>
> --- a/net/ipv6/addrconf_core.c
> +++ b/net/ipv6/addrconf_core.c
> @@ -107,7 +107,15 @@ int inet6addr_notifier_call_chain(unsigned long val, void *v)
>   }
>   EXPORT_SYMBOL(inet6addr_notifier_call_chain);
>   
> -const struct ipv6_stub *ipv6_stub __read_mostly;
> +static int eafnosupport_ipv6_dst_lookup(struct sock *u1, struct dst_entry **u2,
> +                                       struct flowi6 *u3)
> +{
> +       return -EAFNOSUPPORT;
> +}
> +
> +const struct ipv6_stub *ipv6_stub __read_mostly = &(struct ipv6_stub){
> +       .ipv6_dst_lookup = eafnosupport_ipv6_dst_lookup,
> +};
>   EXPORT_SYMBOL_GPL(ipv6_stub);
>   
>   /* IPv6 Wildcard Address and Loopback Address defined by RFC2553 */
>
I have submitted a v6 with this. I had a NULL check in my v5 for 
ipv6_stub and i had thought
that should suffice...but was only concerned about ipv6 module unload.
If v6 is no good, i plan to re-post my v4 patch as v7.

thanks.

Re: [PATCH net-next v5 0/2] af_mpls: fix undefined reference to ip6_route_output with CONFIG_IPV6=n

[David Miller]

From: roopa <roopa@cumulusnetworks.com>
Date: Thu, 30 Jul 2015 13:38:14 -0700

> but was only concerned about ipv6 module unload.

It is not possible to unload the ipv6 module.