* [PATCH] ipv4: use IS_ENABLED instead of ifdef
@ 2020-11-15 22:45 Florian Klink
2020-11-18 0:01 ` Jakub Kicinski
0 siblings, 1 reply; 7+ messages in thread
From: Florian Klink @ 2020-11-15 22:45 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Florian Klink, Kim Phillips
Checking for ifdef CONFIG_x fails if CONFIG_x=m.
Use IS_ENABLED instead, which is true for both built-ins and modules.
Otherwise, a
> ip -4 route add 1.2.3.4/32 via inet6 fe80::2 dev eth1
fails with the message "Error: IPv6 support not enabled in kernel." if
CONFIG_IPV6 is `m`.
In the spirit of b8127113d01e53adba15b41aefd37b90ed83d631.
Cc: Kim Phillips <kim.phillips@arm.com>
Signed-off-by: Florian Klink <flokli@flokli.de>
---
net/ipv4/fib_frontend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 86a23e4a6a50..b87140a1fa28 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -696,7 +696,7 @@ int fib_gw_from_via(struct fib_config *cfg, struct nlattr *nla,
cfg->fc_gw4 = *((__be32 *)via->rtvia_addr);
break;
case AF_INET6:
-#ifdef CONFIG_IPV6
+#if IS_ENABLED(CONFIG_IPV6)
if (alen != sizeof(struct in6_addr)) {
NL_SET_ERR_MSG(extack, "Invalid IPv6 address in RTA_VIA");
return -EINVAL;
--
2.29.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ipv4: use IS_ENABLED instead of ifdef
2020-11-15 22:45 [PATCH] ipv4: use IS_ENABLED instead of ifdef Florian Klink
@ 2020-11-18 0:01 ` Jakub Kicinski
2020-11-18 0:55 ` David Ahern
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2020-11-18 0:01 UTC (permalink / raw)
To: Florian Klink
Cc: netdev, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Kim Phillips, David Ahern
On Sun, 15 Nov 2020 23:45:09 +0100 Florian Klink wrote:
> Checking for ifdef CONFIG_x fails if CONFIG_x=m.
>
> Use IS_ENABLED instead, which is true for both built-ins and modules.
>
> Otherwise, a
> > ip -4 route add 1.2.3.4/32 via inet6 fe80::2 dev eth1
> fails with the message "Error: IPv6 support not enabled in kernel." if
> CONFIG_IPV6 is `m`.
>
> In the spirit of b8127113d01e53adba15b41aefd37b90ed83d631.
>
> Cc: Kim Phillips <kim.phillips@arm.com>
> Signed-off-by: Florian Klink <flokli@flokli.de>
LGTM, this is the fixes tag right?
Fixes: d15662682db2 ("ipv4: Allow ipv6 gateway with ipv4 routes")
CCing David to give him a chance to ack.
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 86a23e4a6a50..b87140a1fa28 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -696,7 +696,7 @@ int fib_gw_from_via(struct fib_config *cfg, struct nlattr *nla,
> cfg->fc_gw4 = *((__be32 *)via->rtvia_addr);
> break;
> case AF_INET6:
> -#ifdef CONFIG_IPV6
> +#if IS_ENABLED(CONFIG_IPV6)
> if (alen != sizeof(struct in6_addr)) {
> NL_SET_ERR_MSG(extack, "Invalid IPv6 address in RTA_VIA");
> return -EINVAL;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipv4: use IS_ENABLED instead of ifdef
2020-11-18 0:01 ` Jakub Kicinski
@ 2020-11-18 0:55 ` David Ahern
2020-11-18 1:07 ` Jakub Kicinski
0 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2020-11-18 0:55 UTC (permalink / raw)
To: Jakub Kicinski, Florian Klink
Cc: netdev, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Kim Phillips
On 11/17/20 5:01 PM, Jakub Kicinski wrote:
> On Sun, 15 Nov 2020 23:45:09 +0100 Florian Klink wrote:
>> Checking for ifdef CONFIG_x fails if CONFIG_x=m.
>>
>> Use IS_ENABLED instead, which is true for both built-ins and modules.
>>
>> Otherwise, a
>>> ip -4 route add 1.2.3.4/32 via inet6 fe80::2 dev eth1
>> fails with the message "Error: IPv6 support not enabled in kernel." if
>> CONFIG_IPV6 is `m`.
>>
>> In the spirit of b8127113d01e53adba15b41aefd37b90ed83d631.
>>
>> Cc: Kim Phillips <kim.phillips@arm.com>
>> Signed-off-by: Florian Klink <flokli@flokli.de>
>
> LGTM, this is the fixes tag right?
>
> Fixes: d15662682db2 ("ipv4: Allow ipv6 gateway with ipv4 routes")
yep.
>
> CCing David to give him a chance to ack.
Reviewed-by: David Ahern <dsahern@kernel.org>
I looked at this yesterday and got distracted diving into the generated
file to see the difference:
#define CONFIG_IPV6 1
vs
#define CONFIG_IPV6_MODULE 1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipv4: use IS_ENABLED instead of ifdef
2020-11-18 0:55 ` David Ahern
@ 2020-11-18 1:07 ` Jakub Kicinski
2020-11-18 3:09 ` Randy Dunlap
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2020-11-18 1:07 UTC (permalink / raw)
To: David Ahern
Cc: Florian Klink, netdev, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Kim Phillips
On Tue, 17 Nov 2020 17:55:54 -0700 David Ahern wrote:
> On 11/17/20 5:01 PM, Jakub Kicinski wrote:
> > On Sun, 15 Nov 2020 23:45:09 +0100 Florian Klink wrote:
> >> Checking for ifdef CONFIG_x fails if CONFIG_x=m.
> >>
> >> Use IS_ENABLED instead, which is true for both built-ins and modules.
> >>
> >> Otherwise, a
> >>> ip -4 route add 1.2.3.4/32 via inet6 fe80::2 dev eth1
> >> fails with the message "Error: IPv6 support not enabled in kernel." if
> >> CONFIG_IPV6 is `m`.
> >>
> >> In the spirit of b8127113d01e53adba15b41aefd37b90ed83d631.
> >>
> >> Cc: Kim Phillips <kim.phillips@arm.com>
> >> Signed-off-by: Florian Klink <flokli@flokli.de>
> >
> > LGTM, this is the fixes tag right?
> >
> > Fixes: d15662682db2 ("ipv4: Allow ipv6 gateway with ipv4 routes")
>
> yep.
>
> >
> > CCing David to give him a chance to ack.
>
> Reviewed-by: David Ahern <dsahern@kernel.org>
Great, applied, thanks!
> I looked at this yesterday and got distracted diving into the generated
> file to see the difference:
>
> #define CONFIG_IPV6 1
>
> vs
>
> #define CONFIG_IPV6_MODULE 1
Interesting.
drivers/net/ethernet/netronome/nfp/flower/action.c:#ifdef CONFIG_IPV6
Oops.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipv4: use IS_ENABLED instead of ifdef
2020-11-18 1:07 ` Jakub Kicinski
@ 2020-11-18 3:09 ` Randy Dunlap
2020-11-18 8:51 ` Florian Klink
2020-11-18 16:38 ` Jakub Kicinski
0 siblings, 2 replies; 7+ messages in thread
From: Randy Dunlap @ 2020-11-18 3:09 UTC (permalink / raw)
To: Jakub Kicinski, David Ahern
Cc: Florian Klink, netdev, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Kim Phillips
On 11/17/20 5:07 PM, Jakub Kicinski wrote:
> On Tue, 17 Nov 2020 17:55:54 -0700 David Ahern wrote:
>> On 11/17/20 5:01 PM, Jakub Kicinski wrote:
>>> On Sun, 15 Nov 2020 23:45:09 +0100 Florian Klink wrote:
>>>> Checking for ifdef CONFIG_x fails if CONFIG_x=m.
>>>>
>>>> Use IS_ENABLED instead, which is true for both built-ins and modules.
>>>>
>>>> Otherwise, a
>>>>> ip -4 route add 1.2.3.4/32 via inet6 fe80::2 dev eth1
>>>> fails with the message "Error: IPv6 support not enabled in kernel." if
>>>> CONFIG_IPV6 is `m`.
>>>>
>>>> In the spirit of b8127113d01e53adba15b41aefd37b90ed83d631.
>>>>
>>>> Cc: Kim Phillips <kim.phillips@arm.com>
>>>> Signed-off-by: Florian Klink <flokli@flokli.de>
>>>
>>> LGTM, this is the fixes tag right?
>>>
>>> Fixes: d15662682db2 ("ipv4: Allow ipv6 gateway with ipv4 routes")
>>
>> yep.
>>
>>>
>>> CCing David to give him a chance to ack.
>>
>> Reviewed-by: David Ahern <dsahern@kernel.org>
>
> Great, applied, thanks!
>
>> I looked at this yesterday and got distracted diving into the generated
>> file to see the difference:
>>
>> #define CONFIG_IPV6 1
>>
>> vs
>>
>> #define CONFIG_IPV6_MODULE 1
Digging up ancient history. ;)
> Interesting.
>
> drivers/net/ethernet/netronome/nfp/flower/action.c:#ifdef CONFIG_IPV6
>
> Oops.
Notify the maintainer!
--
~Randy
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipv4: use IS_ENABLED instead of ifdef
2020-11-18 3:09 ` Randy Dunlap
@ 2020-11-18 8:51 ` Florian Klink
2020-11-18 16:38 ` Jakub Kicinski
1 sibling, 0 replies; 7+ messages in thread
From: Florian Klink @ 2020-11-18 8:51 UTC (permalink / raw)
To: Randy Dunlap
Cc: Jakub Kicinski, David Ahern, netdev, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Kim Phillips
>>> I looked at this yesterday and got distracted diving into the generated
>>> file to see the difference:
>>>
>>> #define CONFIG_IPV6 1
>>>
>>> vs
>>>
>>> #define CONFIG_IPV6_MODULE 1
>
>Digging up ancient history. ;)
>
>> Interesting.
>>
>> drivers/net/ethernet/netronome/nfp/flower/action.c:#ifdef CONFIG_IPV6
>>
>> Oops.
>
>Notify the maintainer!
Yeah, this is super scary stuff - allmodyes-like configs are quite
common in generic distros, and I assume similar mistakes could have
happened in many other places in the kernel as well.
I wonder if the "ifdef CONFIG_…" pattern should be discouraged, and
"IS_ENABLED(CONFIG_…)" used instead, at least for all tristate config
options.
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipv4: use IS_ENABLED instead of ifdef
2020-11-18 3:09 ` Randy Dunlap
2020-11-18 8:51 ` Florian Klink
@ 2020-11-18 16:38 ` Jakub Kicinski
1 sibling, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2020-11-18 16:38 UTC (permalink / raw)
To: Randy Dunlap
Cc: David Ahern, Florian Klink, netdev, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Kim Phillips
On Tue, 17 Nov 2020 19:09:16 -0800 Randy Dunlap wrote:
> On 11/17/20 5:07 PM, Jakub Kicinski wrote:
> > On Tue, 17 Nov 2020 17:55:54 -0700 David Ahern wrote:
> >> On 11/17/20 5:01 PM, Jakub Kicinski wrote:
> >>> On Sun, 15 Nov 2020 23:45:09 +0100 Florian Klink wrote:
> >>>> Checking for ifdef CONFIG_x fails if CONFIG_x=m.
> >>>>
> >>>> Use IS_ENABLED instead, which is true for both built-ins and modules.
> >>>>
> >>>> Otherwise, a
> >>>>> ip -4 route add 1.2.3.4/32 via inet6 fe80::2 dev eth1
> >>>> fails with the message "Error: IPv6 support not enabled in kernel." if
> >>>> CONFIG_IPV6 is `m`.
> >>>>
> >>>> In the spirit of b8127113d01e53adba15b41aefd37b90ed83d631.
> >>>>
> >>>> Cc: Kim Phillips <kim.phillips@arm.com>
> >>>> Signed-off-by: Florian Klink <flokli@flokli.de>
> >>>
> >>> LGTM, this is the fixes tag right?
> >>>
> >>> Fixes: d15662682db2 ("ipv4: Allow ipv6 gateway with ipv4 routes")
> >>
> >> yep.
> >>
> >>>
> >>> CCing David to give him a chance to ack.
> >>
> >> Reviewed-by: David Ahern <dsahern@kernel.org>
> >
> > Great, applied, thanks!
> >
> >> I looked at this yesterday and got distracted diving into the generated
> >> file to see the difference:
> >>
> >> #define CONFIG_IPV6 1
> >>
> >> vs
> >>
> >> #define CONFIG_IPV6_MODULE 1
>
> Digging up ancient history. ;)
>
> > Interesting.
> >
> > drivers/net/ethernet/netronome/nfp/flower/action.c:#ifdef CONFIG_IPV6
> >
> > Oops.
>
> Notify the maintainer!
The joke was that I was the maintainer when it was added ;)
CCing Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-18 16:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-15 22:45 [PATCH] ipv4: use IS_ENABLED instead of ifdef Florian Klink
2020-11-18 0:01 ` Jakub Kicinski
2020-11-18 0:55 ` David Ahern
2020-11-18 1:07 ` Jakub Kicinski
2020-11-18 3:09 ` Randy Dunlap
2020-11-18 8:51 ` Florian Klink
2020-11-18 16:38 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).