On Tue, Jul 28, 2026 at 7:13 PM Ido Schimmel <[email protected]> wrote:
> 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. > 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. > 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. > 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. > Please use the enum throughout the patch instead of a plain u32. Thank you very much for all the detailed reviews! Will adjust all the items properly in patchset v4. Yuyang
