| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 9 Aug 2023 10:06:07 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Hangbin Liu <liuhangbin@...il.com>
Cc: David Ahern <dsahern@...nel.org>,
Stephen Hemminger <stephen@...workplumber.org>,
netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Thomas Haller <thaller@...hat.com>
Subject: Re: [PATCH net-next] ipv4/fib: send RTM_DELROUTE notify when flush
fib
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
Powered by blists - more mailing lists