Re: [PATCH net-next] ipv4/fib: send RTM_DELROUTE notify when flush fib

Ido Schimmel
2023-08-09T07:06:24Z

On Fri, Aug 04, 2023 at 04:09:08PM +0800, Hangbin Liu wrote:
> I reconsidered this issue this week. As we can't get the device status in
> fib_table_flush(). How about adding another flag to track the deleted src
> entries. e.g. RTNH_F_UNRESOLVED. Which is only used in ipmr currently.
> When the src route address is deleted, the route entry also could be
> considered as unresolved. With this idea, the patch could be like:
>
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 51c13cf9c5ae..5c41d34ab447 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -420,7 +420,7 @@ struct rtnexthop {
> #define RTNH_F_ONLINK 4 /* Gateway is forced on link */
> #define RTNH_F_OFFLOAD 8 /* Nexthop is offloaded */
> #define RTNH_F_LINKDOWN 16 /* carrier-down on nexthop */
> -#define RTNH_F_UNRESOLVED 32 /* The entry is unresolved (ipmr) */
> +#define RTNH_F_UNRESOLVED 32 /* The entry is unresolved (ipmr/dead src) */
> #define RTNH_F_TRAP 64 /* Nexthop is trapping packets */
>
> #define RTNH_COMPARE_MASK (RTNH_F_DEAD | RTNH_F_LINKDOWN | \

I'm not sure we need to reinterpret the meaning of this uAPI flag. The
FIB info structure currently looks like this:

struct fib_info {
struct hlist_node fib_hash; /* 0 16 */
struct hlist_node fib_lhash; /* 16 16 */
struct list_head nh_list; /* 32 16 */
struct net * fib_net; /* 48 8 */
refcount_t fib_treeref; /* 56 4 */
refcount_t fib_clntref; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
unsigned int fib_flags; /* 64 4 */
unsigned char fib_dead; /* 68 1 */
unsigned char fib_protocol; /* 69 1 */
unsigned char fib_scope; /* 70 1 */
unsigned char fib_type; /* 71 1 */
__be32 fib_prefsrc; /* 72 4 */
u32 fib_tb_id; /* 76 4 */
u32 fib_priority; /* 80 4 */

/* XXX 4 bytes hole, try to pack */

struct dst_metrics * fib_metrics; /* 88 8 */
int fib_nhs; /* 96 4 */
bool fib_nh_is_v6; /* 100 1 */
bool nh_updated; /* 101 1 */

/* XXX 2 bytes hole, try to pack */

struct nexthop * nh; /* 104 8 */
struct callback_head rcu __attribute__((__aligned__(8))); /* 112 16 */
/* --- cacheline 2 boundary (128 bytes) --- */
struct fib_nh fib_nh[]; /* 128 0 */

/* size: 128, cachelines: 2, members: 21 */
/* sum members: 122, holes: 2, sum holes: 6 */
/* forced alignments: 1 */
} __attribute__((__aligned__(8)));

We can instead represent fib_nh_is_v6 and nh_updated using a single bit:

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index a378eff827c7..a91f8a28689a 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -152,8 +152,8 @@ struct fib_info {
#define fib_rtt fib_metrics->metrics[RTAX_RTT-1]
#define fib_advmss fib_metrics->metrics[RTAX_ADVMSS-1]
int fib_nhs;
- bool fib_nh_is_v6;
- bool nh_updated;
+ u8 fib_nh_is_v6:1,
+ nh_updated:1;
struct nexthop *nh;
struct rcu_head rcu;
struct fib_nh fib_nh[];

And then add another bit there to mark a FIB info that is deleted
because of preferred source address deletion.

I suggest testing with the FIB tests in tools/testing/selftests/net/.

> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 65ba18a91865..a7ef21a6d271 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -1883,7 +1883,7 @@ int fib_sync_down_addr(struct net_device *dev, __be32 local)
> fi->fib_tb_id != tb_id)
> continue;
> if (fi->fib_prefsrc == local) {
> - fi->fib_flags |= RTNH_F_DEAD;
> + fi->fib_flags |= (RTNH_F_DEAD | RTNH_F_UNRESOLVED);
> ret++;
> }
> }
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 74d403dbd2b4..88c593967063 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -2026,6 +2026,7 @@ void fib_table_flush_external(struct fib_table *tb)
> int fib_table_flush(struct net *net, struct fib_table *tb, bool flush_all)
> {
> struct trie *t = (struct trie *)tb->tb_data;
> + struct nl_info info = { .nl_net = net };
> struct key_vector *pn = t->kv;
> unsigned long cindex = 1;
> struct hlist_node *tmp;
> @@ -2088,6 +2089,11 @@ int fib_table_flush(struct net *net, struct fib_table *tb, bool flush_all)
>
> fib_notify_alias_delete(net, n->key, &n->leaf, fa,
> NULL);
> + if (fi->fib_flags & RTNH_F_UNRESOLVED) {
> + fi->fib_flags &= ~RTNH_F_UNRESOLVED;
> + rtmsg_fib(RTM_DELROUTE, htonl(n->key), fa,
> + KEYLENGTH - fa->fa_slen, tb->tb_id, &info, 0);
> + }
> hlist_del_rcu(&fa->fa_list);
> fib_release_info(fa->fa_info);
> alias_free_mem_rcu(fa);
>
> Thanks
> Hangbin