| Message ID | 20250212164323.2183023-3-edumazet@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: better support of blackholes | expand |
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, },
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 ?
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
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.
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> ...
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
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,
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(-)