Netdev List
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Yuyang Huang <sigefriedhyy@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Bobby Eshleman <bobbyeshleman@meta.com>,
	Chris J Arges <carges@cloudflare.com>,
	David Ahern <dsahern@kernel.org>, David Wei <dw@davidwei.uk>,
	Dimitri Daskalakis <dimitri.daskalakis1@gmail.com>,
	Donald Hunter <donald.hunter@gmail.com>,
	Eric Dumazet <edumazet@google.com>, Gal Pressman <gal@nvidia.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Shuah Khan <shuah@kernel.org>, Simon Horman <horms@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>,
	Willem de Bruijn <willemb@google.com>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH net-next v3 1/2] ipv6: report why a route was deleted in RTM_DELROUTE
Date: Tue, 28 Jul 2026 13:13:18 +0300	[thread overview]
Message-ID: <20260728101318.GA849975@shredder> (raw)
In-Reply-To: <20260725031214.43059-2-sigefriedhyy@gmail.com>

On Sat, Jul 25, 2026 at 12:12:13PM +0900, Yuyang Huang wrote:
> Add RTA_DEL_REASON (u32) to RTM_DELROUTE notifications to report the
> cause of kernel-initiated IPv6 route deletions, so user space can tell
> routes that expired from routes explicitly withdrawn by Router
> Advertisements.
> 
> The two causes call for different treatment in user space. An expired
> RA route means the router failed to refresh it before its lifetime ran
> out, which points at a misconfigured or unreliable router; user space
> may want to react, for example by disabling IPv6 on that network
> entirely. An RA withdrawal (zero lifetime) is valid, RFC-compliant
> operation and needs no such recovery. Today both surface as an
> identical RTM_DELROUTE.
> 
> Plumb the deletion reason through the IPv6 FIB and notification paths:
> - Update `fib6_del`, `fib6_del_route`, and `inet6_rt_notify` signatures
>   to explicitly pass the `del_reason` down the call chain.
> - Update `rt6_fill_node()` to emit `RTA_DEL_REASON` attribute when the
>   reason is not UNSPEC.
> - Record the cause in the kernel-initiated IPv6 deletion paths:
>   - RTA_DEL_REASON_EXPIRED: for routes cleaned up by the FIB6 garbage
>     collector.
>   - RTA_DEL_REASON_RA_WITHDRAWN: for default routes, prefix routes, or
>     route information option routes withdrawn by Router Advertisements.
> - Update the `rt-route` Netlink YAML spec to define the new attribute,
>   notifications, and multicast groups.
> 
> Signed-off-by: Yuyang Huang <sigefriedhyy@gmail.com>
> ---
>  Documentation/netlink/specs/rt-route.yaml | 67 ++++++++++++++++++++++-
>  include/net/ip6_fib.h                     |  4 +-
>  include/net/ip6_route.h                   |  2 +
>  include/uapi/linux/rtnetlink.h            | 17 ++++++
>  net/ipv6/addrconf.c                       |  3 +-
>  net/ipv6/ip6_fib.c                        | 19 ++++---
>  net/ipv6/ndisc.c                          |  7 ++-
>  net/ipv6/route.c                          | 66 ++++++++++++++--------
>  8 files changed, 148 insertions(+), 37 deletions(-)

There are too many changes in this patch. The meaningful changes are
mixed in with a lot of churn. I suggest splitting it as follows:

1. Add the rta_del_reason enum and ip6_del_rt_reason(). Call it from
ip6_del_rt() with RTA_DEL_REASON_UNSPEC. No functional changes.

2. Propagate the deletion reason argument from ip6_del_rt_reason() all
the way to fib6_del_route(). Purely a mechanical change with a lot of
churn.

3. Change the relevant callers to specify the deletion reason instead of
RTA_DEL_REASON_UNSPEC. This is the meaningful change.

Now the only thing that is left is to expose the deletion reason to user
space.

4. Add a deletion reason argument to rt6_fill_node() and pass UNSPEC.
Purely a mechanical change with a lot of churn.

5. Change rt6_fill_node() (and rt6_nlmsg_size()) to expose the reason to
user space assuming it is not UNSPEC. At this point it is still not
visible to user space since all the callers pass UNSPEC.

6. Add inet6_rt_del_notify() (see more below) and call it from
fib6_del_route() with the deletion reason. The inet6_rt_notify() callers
do not need to change.

7. Add the multicast groups and the notifications to rt-route.yaml (see
more below).

8. Add the deletion reason to rt-route.yaml.

9. Add the selftest.

> 
> diff --git a/Documentation/netlink/specs/rt-route.yaml b/Documentation/netlink/specs/rt-route.yaml
> index 33195db96746..d7ef80fc9c71 100644
> --- a/Documentation/netlink/specs/rt-route.yaml
> +++ b/Documentation/netlink/specs/rt-route.yaml
> @@ -78,6 +78,15 @@ definitions:
>        -
>          name: rta-used
>          type: u32
> +  -
> +    name: del-reason
> +    type: enum
> +    name-prefix: rta-del-reason-
> +    enum-name: rta-del-reason
> +    entries:
> +      - unspec
> +      - expired
> +      - ra-withdrawn
>  
>  attribute-sets:
>    -
> @@ -185,6 +194,10 @@ attribute-sets:
>          type: u32
>          byte-order: big-endian
>          display-hint: hex
> +      -
> +        name: del-reason
> +        type: u32
> +        enum: del-reason
>    -
>      name: metrics
>      name-prefix: rtax-
> @@ -299,6 +312,7 @@ operations:
>              - dport
>              - nh-id
>              - flowlabel
> +            - del-reason
>        dump:
>          request:
>            value: 26
> @@ -313,7 +327,37 @@ operations:
>        do:
>          request:
>            value: 24
> -          attributes: *all-route-attrs
> +          # del-reason is notification-only, the kernel rejects it in
> +          # requests, so it is deliberately absent from this list.
> +          attributes: &route-req-attrs
> +            - dst
> +            - src
> +            - iif
> +            - oif
> +            - gateway
> +            - priority
> +            - prefsrc
> +            - metrics
> +            - multipath
> +            - flow
> +            - cacheinfo
> +            - table
> +            - mark
> +            - mfc-stats
> +            - via
> +            - newdst
> +            - pref
> +            - encap-type
> +            - encap
> +            - expires
> +            - pad
> +            - uid
> +            - ttl-propagate
> +            - ip-proto
> +            - sport
> +            - dport

At least sport and dport are only used in RTM_GETROUTE, but it does seem
like they won't be rejected if encoded in an RTM_NEWROUTE request, so I
guess it's OK to put them here.

> +            - nh-id
> +            - flowlabel
>      -
>        name: delroute
>        doc: Delete an existing route
> @@ -321,4 +365,23 @@ operations:
>        do:
>          request:
>            value: 25
> -          attributes: *all-route-attrs
> +          attributes: *route-req-attrs
> +    -
> +      name: newroute-ntf
> +      doc: Notification about a created route.
> +      value: 24
> +      notify: getroute
> +    -
> +      name: delroute-ntf
> +      doc: Notification about a deleted route.
> +      value: 25
> +      notify: getroute
> +
> +mcast-groups:
> +  list:
> +    -
> +      name: rtnlgrp-ipv4-route
> +      value: 7
> +    -
> +      name: rtnlgrp-ipv6-route
> +      value: 11

This hunk (adding the mcast groups and notifications) is not related to
the deletion reason. Please split it to a separate patch. There are too
many changes in this patch as-is.

> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index 9cd27e1b9b69..7ca0e2124e79 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -468,7 +468,7 @@ void fib6_clean_all_skip_notify(struct net *net,
>  
>  int fib6_add(struct fib6_node *root, struct fib6_info *rt,
>  	     struct nl_info *info, struct netlink_ext_ack *extack);
> -int fib6_del(struct fib6_info *rt, struct nl_info *info);
> +int fib6_del(struct fib6_info *rt, struct nl_info *info, u32 del_reason);
>  
>  static inline
>  void rt6_get_prefsrc(const struct rt6_info *rt, struct in6_addr *addr)
> @@ -531,7 +531,7 @@ static inline void fib6_rt_update(struct net *net, struct fib6_info *rt,
>  }
>  #endif
>  void inet6_rt_notify(int event, struct fib6_info *rt, struct nl_info *info,
> -		     unsigned int flags);
> +		     unsigned int flags, u32 del_reason);

Why not use enum rta_del_reason like we do with kfree_skb_reason()?

Also, only one caller calls it from the deletion path, the rest pass
RTM_NEWROUTE and it's weird to see something like:

inet6_rt_notify(RTM_NEWROUTE, rt, info, nlflags, RTA_DEL_REASON_UNSPEC);

Maybe create inet6_rt_del_notify(rt, info, del_reason) and have it call
__inet6_rt_notify()? Then we don't need any changes in the addition
path.

>  
>  void fib6_age_exceptions(struct fib6_info *rt, struct fib6_gc_args *gc_args,
>  			 unsigned long now);
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index 09ffe0f13ce7..98f1684fe378 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -128,6 +128,8 @@ int ip6_route_add(struct fib6_config *cfg, gfp_t gfp_flags,
>  int ip6_ins_rt(struct net *net, struct fib6_info *f6i);
>  #if IS_ENABLED(CONFIG_IPV6)
>  int ip6_del_rt(struct net *net, struct fib6_info *f6i, bool skip_notify);
> +int ip6_del_rt_reason(struct net *net, struct fib6_info *f6i, bool skip_notify,
> +		      u32 del_reason);

Please use the enum throughout the patch instead of a plain u32.

  reply	other threads:[~2026-07-28 10:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-25  3:12 [PATCH net-next v3 0/2] ipv6: report why a route was deleted in RTM_DELROUTE Yuyang Huang
2026-07-25  3:12 ` [PATCH net-next v3 1/2] " Yuyang Huang
2026-07-28 10:13   ` Ido Schimmel [this message]
2026-07-28 12:50     ` Yuyang Huang
2026-07-25  3:12 ` [PATCH net-next v3 2/2] selftests: net: verify RTA_DEL_REASON on route deletion Yuyang Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260728101318.GA849975@shredder \
    --to=idosch@nvidia.com \
    --cc=bobbyeshleman@meta.com \
    --cc=carges@cloudflare.com \
    --cc=davem@davemloft.net \
    --cc=dimitri.daskalakis1@gmail.com \
    --cc=donald.hunter@gmail.com \
    --cc=dsahern@kernel.org \
    --cc=dw@davidwei.uk \
    --cc=edumazet@google.com \
    --cc=gal@nvidia.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    --cc=sigefriedhyy@gmail.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox