| Message ID | 166807341463.2904467.10141806642379634063.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] rxrpc: Fix missing IPV6 #ifdef | expand |
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 > /* > >
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
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
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
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
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 } /*
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(+)