[PATCH net-next 0/2] net: better support of blackholes

[PATCH net-next 0/2] net: better support of blackholes

[Eric Dumazet]

First patch is adding a new drop_reason, for packets
that are dropped by route blackhole.

Second patch changes ipv6 to no longer report
local errors for blackhole users.

Eric Dumazet (2):
  net: dropreason: add SKB_DROP_REASON_BLACKHOLE
  ipv6: fix blackhole routes

 include/net/dropreason-core.h | 5 +++++
 net/core/dst.c                | 2 +-
 net/ipv6/route.c              | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.48.1.502.g6dc24dfdaf-goog

[PATCH net-next 1/2] net: dropreason: add SKB_DROP_REASON_BLACKHOLE

[Eric Dumazet]

Use this new drop reason from dst_discard_out().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/dropreason-core.h | 5 +++++
 net/core/dst.c                | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 32a34dfe8cc58fb1afda8922a52249080f1183b5..de42577f16dd199790cea9ac07b326864f2103e3 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -117,6 +117,7 @@
 	FN(ARP_PVLAN_DISABLE)		\
 	FN(MAC_IEEE_MAC_CONTROL)	\
 	FN(BRIDGE_INGRESS_STP_STATE)	\
+	FN(BLACKHOLE)			\
 	FNe(MAX)
 
 /**
@@ -554,6 +555,10 @@ enum skb_drop_reason {
 	 * ingress bridge port does not allow frames to be forwarded.
 	 */
 	SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE,
+	/**
+	 * @SKB_DROP_REASON_BLACKHOLE: blackhole route.
+	 */
+	SKB_DROP_REASON_BLACKHOLE,
 	/**
 	 * @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which
 	 * shouldn't be used as a real 'reason' - only for tracing code gen
diff --git a/net/core/dst.c b/net/core/dst.c
index 9552a90d4772dce49b5fe94d2f1d8da6979d9908..0cbbad4d7c07fa397f66a2d252a636f90dafddee 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -29,7 +29,7 @@
 
 int dst_discard_out(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
-	kfree_skb(skb);
+	kfree_skb_reason(skb, SKB_DROP_REASON_BLACKHOLE);
 	return 0;
 }
 EXPORT_SYMBOL(dst_discard_out);
-- 
2.48.1.502.g6dc24dfdaf-goog

Re: [PATCH net-next 1/2] net: dropreason: add SKB_DROP_REASON_BLACKHOLE

[David Ahern]

On 2/12/25 9:43 AM, Eric Dumazet wrote:
> Use this new drop reason from dst_discard_out().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  include/net/dropreason-core.h | 5 +++++
>  net/core/dst.c                | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

[PATCH net-next 2/2] ipv6: fix blackhole routes

[Eric Dumazet]

For some reason, linux does not really act as a blackhole
for local processes:

ip route add blackhole 100::/64     # RFC 6666
ip route get 100::
RTNETLINK answers: Invalid argument
ping6 -c2 100::
ping6: connect: Invalid argument
ip route del 100::/64

After this patch, a local process no longer has an immediate error,
the blackhole is simply eating the packets as intended.

Also the "route get" command does not fail anymore.

ip route add blackhole 100::/64
ip route get 100::
blackhole 100:: dev lo src ::1 metric 1024 pref medium
ping6 -c2 100::
PING 100:: (100::) 56 data bytes

--- 100:: ping statistics ---
2 packets transmitted, 0 received, 100% packet loss, time 1019ms

ip route del 100::/64

Reported-by: Paul Ripke <stix@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 78362822b9070df138a0724dc76003b63026f9e2..335cdbfe621e2fc4a71badf4ff834870638d5e13 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1048,7 +1048,7 @@ static const int fib6_prop[RTN_MAX + 1] = {
 	[RTN_BROADCAST]	= 0,
 	[RTN_ANYCAST]	= 0,
 	[RTN_MULTICAST]	= 0,
-	[RTN_BLACKHOLE]	= -EINVAL,
+	[RTN_BLACKHOLE]	= 0,
 	[RTN_UNREACHABLE] = -EHOSTUNREACH,
 	[RTN_PROHIBIT]	= -EACCES,
 	[RTN_THROW]	= -EAGAIN,
-- 
2.48.1.502.g6dc24dfdaf-goog

Re: [PATCH net-next 2/2] ipv6: fix blackhole routes

[David Ahern]

On 2/12/25 9:43 AM, Eric Dumazet wrote:
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 78362822b9070df138a0724dc76003b63026f9e2..335cdbfe621e2fc4a71badf4ff834870638d5e13 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1048,7 +1048,7 @@ static const int fib6_prop[RTN_MAX + 1] = {
>  	[RTN_BROADCAST]	= 0,
>  	[RTN_ANYCAST]	= 0,
>  	[RTN_MULTICAST]	= 0,
> -	[RTN_BLACKHOLE]	= -EINVAL,
> +	[RTN_BLACKHOLE]	= 0,
>  	[RTN_UNREACHABLE] = -EHOSTUNREACH,
>  	[RTN_PROHIBIT]	= -EACCES,
>  	[RTN_THROW]	= -EAGAIN,

EINVAL goes back to ef2c7d7b59708 in 2012, so this is a change in user
visible behavior. Also this will make ipv6 deviate from ipv4:

        [RTN_BLACKHOLE] = {
                .error  = -EINVAL,
                .scope  = RT_SCOPE_UNIVERSE,
        },

Re: [PATCH net-next 2/2] ipv6: fix blackhole routes

[Eric Dumazet]

On Wed, Feb 12, 2025 at 7:00 PM David Ahern <dsahern@kernel.org> wrote:
>
> On 2/12/25 9:43 AM, Eric Dumazet wrote:
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 78362822b9070df138a0724dc76003b63026f9e2..335cdbfe621e2fc4a71badf4ff834870638d5e13 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -1048,7 +1048,7 @@ static const int fib6_prop[RTN_MAX + 1] = {
> >       [RTN_BROADCAST] = 0,
> >       [RTN_ANYCAST]   = 0,
> >       [RTN_MULTICAST] = 0,
> > -     [RTN_BLACKHOLE] = -EINVAL,
> > +     [RTN_BLACKHOLE] = 0,
> >       [RTN_UNREACHABLE] = -EHOSTUNREACH,
> >       [RTN_PROHIBIT]  = -EACCES,
> >       [RTN_THROW]     = -EAGAIN,
>
> EINVAL goes back to ef2c7d7b59708 in 2012, so this is a change in user
> visible behavior. Also this will make ipv6 deviate from ipv4:
>
>         [RTN_BLACKHOLE] = {
>                 .error  = -EINVAL,
>                 .scope  = RT_SCOPE_UNIVERSE,
>         },

Should we create a new RTN_SINK (or different name), for both IPv4 and IPv6 ?

Re: [PATCH net-next 2/2] ipv6: fix blackhole routes

[Stanislav Fomichev]

On 02/12, Eric Dumazet wrote:
> On Wed, Feb 12, 2025 at 7:00 PM David Ahern <dsahern@kernel.org> wrote:
> >
> > On 2/12/25 9:43 AM, Eric Dumazet wrote:
> > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > > index 78362822b9070df138a0724dc76003b63026f9e2..335cdbfe621e2fc4a71badf4ff834870638d5e13 100644
> > > --- a/net/ipv6/route.c
> > > +++ b/net/ipv6/route.c
> > > @@ -1048,7 +1048,7 @@ static const int fib6_prop[RTN_MAX + 1] = {
> > >       [RTN_BROADCAST] = 0,
> > >       [RTN_ANYCAST]   = 0,
> > >       [RTN_MULTICAST] = 0,
> > > -     [RTN_BLACKHOLE] = -EINVAL,
> > > +     [RTN_BLACKHOLE] = 0,
> > >       [RTN_UNREACHABLE] = -EHOSTUNREACH,
> > >       [RTN_PROHIBIT]  = -EACCES,
> > >       [RTN_THROW]     = -EAGAIN,
> >
> > EINVAL goes back to ef2c7d7b59708 in 2012, so this is a change in user
> > visible behavior. Also this will make ipv6 deviate from ipv4:
> >
> >         [RTN_BLACKHOLE] = {
> >                 .error  = -EINVAL,
> >                 .scope  = RT_SCOPE_UNIVERSE,
> >         },
> 
> Should we create a new RTN_SINK (or different name), for both IPv4 and IPv6 ?

Sorry for sidelining, but depending on how this discussion goes,
tools/testing/selftests/net/fib_nexthops.sh test might need to be
adjusted (currently fails presumably because of -EINVAL change):

https://netdev-3.bots.linux.dev/vmksft-net/results/990081/2-fib-nexthops-sh/stdout

---
pw-bot: cr

Re: [PATCH net-next 2/2] ipv6: fix blackhole routes

[David Ahern]

On 2/12/25 7:30 PM, Stanislav Fomichev wrote:
> On 02/12, Eric Dumazet wrote:
>> On Wed, Feb 12, 2025 at 7:00 PM David Ahern <dsahern@kernel.org> wrote:
>>>
>>> On 2/12/25 9:43 AM, Eric Dumazet wrote:
>>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>>> index 78362822b9070df138a0724dc76003b63026f9e2..335cdbfe621e2fc4a71badf4ff834870638d5e13 100644
>>>> --- a/net/ipv6/route.c
>>>> +++ b/net/ipv6/route.c
>>>> @@ -1048,7 +1048,7 @@ static const int fib6_prop[RTN_MAX + 1] = {
>>>>       [RTN_BROADCAST] = 0,
>>>>       [RTN_ANYCAST]   = 0,
>>>>       [RTN_MULTICAST] = 0,
>>>> -     [RTN_BLACKHOLE] = -EINVAL,
>>>> +     [RTN_BLACKHOLE] = 0,
>>>>       [RTN_UNREACHABLE] = -EHOSTUNREACH,
>>>>       [RTN_PROHIBIT]  = -EACCES,
>>>>       [RTN_THROW]     = -EAGAIN,
>>>
>>> EINVAL goes back to ef2c7d7b59708 in 2012, so this is a change in user
>>> visible behavior. Also this will make ipv6 deviate from ipv4:
>>>
>>>         [RTN_BLACKHOLE] = {
>>>                 .error  = -EINVAL,
>>>                 .scope  = RT_SCOPE_UNIVERSE,
>>>         },
>>
>> Should we create a new RTN_SINK (or different name), for both IPv4 and IPv6 ?
> 
> Sorry for sidelining, but depending on how this discussion goes,
> tools/testing/selftests/net/fib_nexthops.sh test might need to be
> adjusted (currently fails presumably because of -EINVAL change):
> 
> https://netdev-3.bots.linux.dev/vmksft-net/results/990081/2-fib-nexthops-sh/stdout
> 

yep, I verified that yesterday - there are blackhole selftests. At this
point SINK or whatever the name would have to be a new API, including
how the route is installed if no local failure is really desired.

Re: [PATCH net-next 2/2] ipv6: fix blackhole routes

[Nicolas Dichtel]

Le 12/02/2025 à 19:00, David Ahern a écrit :
> On 2/12/25 9:43 AM, Eric Dumazet wrote:
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 78362822b9070df138a0724dc76003b63026f9e2..335cdbfe621e2fc4a71badf4ff834870638d5e13 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -1048,7 +1048,7 @@ static const int fib6_prop[RTN_MAX + 1] = {
>>  	[RTN_BROADCAST]	= 0,
>>  	[RTN_ANYCAST]	= 0,
>>  	[RTN_MULTICAST]	= 0,
>> -	[RTN_BLACKHOLE]	= -EINVAL,
>> +	[RTN_BLACKHOLE]	= 0,
>>  	[RTN_UNREACHABLE] = -EHOSTUNREACH,
>>  	[RTN_PROHIBIT]	= -EACCES,
>>  	[RTN_THROW]	= -EAGAIN,
> 
> EINVAL goes back to ef2c7d7b59708 in 2012, so this is a change in user
> visible behavior. Also this will make ipv6 deviate from ipv4:
> 
>         [RTN_BLACKHOLE] = {
>                 .error  = -EINVAL,
>                 .scope  = RT_SCOPE_UNIVERSE,
>         },
Yes, if I remember well, to be consistent I mimicked what existed in IPv4. I
never found a good answer to why 'EINVAL' :)

Nicolas

Re: [PATCH net-next 2/2] ipv6: fix blackhole routes

[Simon Horman]

On Wed, Feb 12, 2025 at 04:43:23PM +0000, Eric Dumazet wrote:
> For some reason, linux does not really act as a blackhole
> for local processes:
> 
> ip route add blackhole 100::/64     # RFC 6666
> ip route get 100::
> RTNETLINK answers: Invalid argument
> ping6 -c2 100::
> ping6: connect: Invalid argument
> ip route del 100::/64
> 
> After this patch, a local process no longer has an immediate error,
> the blackhole is simply eating the packets as intended.
> 
> Also the "route get" command does not fail anymore.
> 
> ip route add blackhole 100::/64
> ip route get 100::
> blackhole 100:: dev lo src ::1 metric 1024 pref medium
> ping6 -c2 100::
> PING 100:: (100::) 56 data bytes
> 
> --- 100:: ping statistics ---
> 2 packets transmitted, 0 received, 100% packet loss, time 1019ms

Hi Eric,

Sorry to nit-pick on something that is nothing to do with the change
itself. But could you reformat the above somehow as git will cut off
the commit message at the ("^---") above. Which amongst other things
means the patch will end up without a Signed-off-by line in git.

> 
> ip route del 100::/64
> 
> Reported-by: Paul Ripke <stix@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

...