[PATCH net-next] rxrpc: Fix missing IPV6 #ifdef

[PATCH net-next] rxrpc: Fix missing IPV6 #ifdef

[David Howells]

Fix rxrpc_encap_err_rcv() to make the call to ipv6_icmp_error conditional
on IPV6 support being enabled.

Fixes: b6c66c4324e7 ("rxrpc: Use the core ICMP/ICMP6 parsers")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---

 net/rxrpc/local_object.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index a178f71e5082..25cdfcf7b415 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -33,7 +33,9 @@ static void rxrpc_encap_err_rcv(struct sock *sk, struct sk_buff *skb, int err,
 {
 	if (ip_hdr(skb)->version == IPVERSION)
 		return ip_icmp_error(sk, skb, err, port, info, payload);
+#ifdef CONFIG_AF_RXRPC_IPV6
 	return ipv6_icmp_error(sk, skb, err, port, info, payload);
+#endif
 }
 
 /*

Re: [PATCH net-next] rxrpc: Fix missing IPV6 #ifdef

[Hideaki Yoshifuji]

Hi,

2022年11月10日(木) 18:44 David Howells <dhowells@redhat.com>:
>
> Fix rxrpc_encap_err_rcv() to make the call to ipv6_icmp_error conditional
> on IPV6 support being enabled.
>
> Fixes: b6c66c4324e7 ("rxrpc: Use the core ICMP/ICMP6 parsers")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Marc Dionne <marc.dionne@auristor.com>
> cc: linux-afs@lists.infradead.org
> cc: netdev@vger.kernel.org
> ---
>
>  net/rxrpc/local_object.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
> index a178f71e5082..25cdfcf7b415 100644
> --- a/net/rxrpc/local_object.c
> +++ b/net/rxrpc/local_object.c
> @@ -33,7 +33,9 @@ static void rxrpc_encap_err_rcv(struct sock *sk, struct sk_buff *skb, int err,
>  {
>         if (ip_hdr(skb)->version == IPVERSION)
>                 return ip_icmp_error(sk, skb, err, port, info, payload);
> +#ifdef CONFIG_AF_RXRPC_IPV6
>         return ipv6_icmp_error(sk, skb, err, port, info, payload);
> +#endif
>  }
>

Because this introduces a missing return literally without CONFIG_AF_RXRPC_IPV6,
I would prefer #ifdef / #else for the whole function.

--yoshfuji

>  /*
>
>

Re: [PATCH net-next] rxrpc: Fix missing IPV6 #ifdef

[David Howells]

Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com> wrote:

> Because this introduces a missing return literally without
> CONFIG_AF_RXRPC_IPV6, I would prefer #ifdef / #else for the whole function.

They're both void functions, so actually, there's nothing to return.

David

Re: [PATCH net-next] rxrpc: Fix missing IPV6 #ifdef

[Andrew Lunn]

On Thu, Nov 10, 2022 at 09:43:34AM +0000, David Howells wrote:
> Fix rxrpc_encap_err_rcv() to make the call to ipv6_icmp_error conditional
> on IPV6 support being enabled.
> 
> Fixes: b6c66c4324e7 ("rxrpc: Use the core ICMP/ICMP6 parsers")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Marc Dionne <marc.dionne@auristor.com>
> cc: linux-afs@lists.infradead.org
> cc: netdev@vger.kernel.org
> ---
> 
>  net/rxrpc/local_object.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
> index a178f71e5082..25cdfcf7b415 100644
> --- a/net/rxrpc/local_object.c
> +++ b/net/rxrpc/local_object.c
> @@ -33,7 +33,9 @@ static void rxrpc_encap_err_rcv(struct sock *sk, struct sk_buff *skb, int err,
>  {
>  	if (ip_hdr(skb)->version == IPVERSION)
>  		return ip_icmp_error(sk, skb, err, port, info, payload);
> +#ifdef CONFIG_AF_RXRPC_IPV6
>  	return ipv6_icmp_error(sk, skb, err, port, info, payload);
> +#endif

Can this be if (IS_ENABLED(CONFIG_AF_RXRPC_IPV6) {} rather than
#ifdef? It gives better build testing.

	Andrew

Re: [PATCH net-next] rxrpc: Fix missing IPV6 #ifdef

[David Howells]

Andrew Lunn <andrew@lunn.ch> wrote:

> > +#ifdef CONFIG_AF_RXRPC_IPV6
> >  	return ipv6_icmp_error(sk, skb, err, port, info, payload);
> > +#endif
> 
> Can this be if (IS_ENABLED(CONFIG_AF_RXRPC_IPV6) {} rather than
> #ifdef? It gives better build testing.

Sure.  Does it actually make that much of a difference?  I guess the
declaration is there even if IPV6 is disabled.

David

Re: [PATCH net-next] rxrpc: Fix missing IPV6 #ifdef

[Andrew Lunn]

On Thu, Nov 10, 2022 at 02:09:45PM +0000, David Howells wrote:
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > > +#ifdef CONFIG_AF_RXRPC_IPV6
> > >  	return ipv6_icmp_error(sk, skb, err, port, info, payload);
> > > +#endif
> > 
> > Can this be if (IS_ENABLED(CONFIG_AF_RXRPC_IPV6) {} rather than
> > #ifdef? It gives better build testing.
> 
> Sure.  Does it actually make that much of a difference?  I guess the
> declaration is there even if IPV6 is disabled.

And that is the point. Even if this feature is disabled, the type
checking will be performed, the number of parameters etc. Somebody who
modifies ipv6_icmp_error() is going to find problems with a single
compilation, rather than having to use a big collection of random
configs.

So this is a review comment i often make. Avoid #ifdef if you can, use
IS_ENABLED() inside an if statement.

	Andrew