bpf: Add MTU check to fib lookup helper

PATCH In review David Ahern 3 participants 9 messages May 17, 2018 – May 20, 2018

Subsystems (5)

Networking Network drivers BPF IPv4/IPv6 +1 more subsystems

lore

Share & tools

lore

David Ahern

author

2018-05-17 16:09 UTC #0 lore raw

cover bpf: Add MTU check to fib lookup helper

Packets that exceed the egress MTU can not be forwarded in the fast path. Add IPv4 and IPv6 MTU helpers that take a FIB lookup result (versus the typical dst path) and add the calls to bpf_ipv{4,6}_fib_lookup.

David Ahern (3):
net/ipv4: Add helper to return path MTU based on fib result net/ipv6: Add helper to return path MTU based on fib result
bpf: Add mtu checking to FIB forwarding helper

 include/net/ip6_fib.h   |  6 ++++++
 include/net/ip6_route.h |  3 +++
 include/net/ip_fib.h    |  2 ++
 net/core/filter.c       | 10 ++++++++++
 net/ipv4/route.c        | 31 +++++++++++++++++++++++++++++++
 net/ipv6/route.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 100 insertions(+)

Signature · 1 line

2.11.0

David Ahern

author

2018-05-17 16:09 UTC #1 lore raw

PATCH 1/3 net/ipv4: Add helper to return path MTU based on fib result

Determine path MTU from a FIB lookup result. Logic is a distillation of ip_dst_mtu_maybe_forward.

Signed-off-by David Ahern <[email protected]>

---
 include/net/ip_fib.h |  2 ++
 net/ipv4/route.c     | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

include/net/ip_fib.h +2 -0

index 81d0f2107ff1..69c91d1934c1 100644
@@ -449,4 +449,6 @@ static inline void fib_proc_exit(struct net *net)
 }
 #endif
 
+u32 ip_mtu_from_fib_result(struct fib_result *res, __be32 daddr);
+
 #endif  /* _NET_FIB_H */

net/ipv4/route.c +31 -0

index 29268efad247..ac3b22bc51b2 100644
@@ -1352,6 +1352,37 @@ static struct fib_nh_exception *find_exception(struct fib_nh *nh, __be32 daddr)
 	return NULL;
 }
 
+/* MTU selection:
+ * 1. mtu on route is locked - use it
+ * 2. mtu from nexthop exception
+ * 3. mtu from egress device
+ */
+
+u32 ip_mtu_from_fib_result(struct fib_result *res, __be32 daddr)
+{
+	struct fib_info *fi = res->fi;
+	struct fib_nh *nh = &fi->fib_nh[res->nh_sel];
+	struct net_device *dev = nh->nh_dev;
+	u32 mtu = 0;
+
+	if (dev_net(dev)->ipv4.sysctl_ip_fwd_use_pmtu ||
+	    fi->fib_metrics->metrics[RTAX_LOCK - 1] & (1 << RTAX_MTU))
+		mtu = fi->fib_mtu;
+
+	if (likely(!mtu)) {
+		struct fib_nh_exception *fnhe;
+
+		fnhe = find_exception(nh, daddr);
+		if (fnhe && !time_after_eq(jiffies, fnhe->fnhe_expires))
+			mtu = fnhe->fnhe_pmtu;
+	}
+
+	if (likely(!mtu))
+		mtu = min(READ_ONCE(dev->mtu), IP_MAX_MTU);
+
+	return mtu - lwtunnel_headroom(nh->nh_lwtstate, mtu);
+}
+
 static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
 			      __be32 daddr, const bool do_cache)
 {

Signature · 1 line

2.11.0

David Ahern

author

2018-05-17 16:09 UTC #2 lore raw

PATCH 2/3 net/ipv6: Add helper to return path MTU based on fib result

In reply to David Ahern

Determine path MTU from a FIB lookup result. Logic is based on ip6_dst_mtu_forward plus lookup of nexthop exception.

Signed-off-by David Ahern <[email protected]>

---
 include/net/ip6_fib.h   |  6 ++++++
 include/net/ip6_route.h |  3 +++
 net/ipv6/route.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+)

include/net/ip6_fib.h +6 -0

index cc70f6da8462..7897efe80727 100644
@@ -412,6 +412,12 @@ static inline struct net_device *fib6_info_nh_dev(const struct fib6_info *f6i)
 	return f6i->fib6_nh.nh_dev;
 }
 
+static inline
+struct lwtunnel_state *fib6_info_nh_lwt(const struct fib6_info *f6i)
+{
+	return f6i->fib6_nh.nh_lwtstate;
+}
+
 void inet6_rt_notify(int event, struct fib6_info *rt, struct nl_info *info,
 		     unsigned int flags);

include/net/ip6_route.h +3 -0

index 4cf1ef935ed9..7b9c82de11cc 100644
@@ -300,6 +300,9 @@ static inline unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst)
 	return mtu;
 }
 
+u32 ip6_mtu_from_fib6(struct fib6_info *f6i, struct in6_addr *daddr,
+		      struct in6_addr *saddr);
+
 struct neighbour *ip6_neigh_lookup(const struct in6_addr *gw,
 				   struct net_device *dev, struct sk_buff *skb,
 				   const void *daddr);

net/ipv6/route.c +48 -0

54 lines not on this page · raw message

Signature · 1 line

2.11.0

David Ahern

author

2018-05-17 16:09 UTC #3 lore raw

PATCH 3/3 bpf: Add mtu checking to FIB forwarding helper

In reply to David Ahern

Add check that egress MTU can handle packet to be forwarded. If the MTU is less than the packet lenght, return 0 meaning the packet is expected to continue up the stack for help - eg., fragmenting the packet or sending an ICMP.

Signed-off-by David Ahern <[email protected]>

---
 net/core/filter.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

net/core/filter.c +10 -0

index 6d0d1560bd70..c47c47a75d4b 100644
@@ -4098,6 +4098,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct fib_nh *nh;
 	struct flowi4 fl4;
 	int err;
+	u32 mtu;
 
 	dev = dev_get_by_index_rcu(net, params->ifindex);
 	if (unlikely(!dev))

@@ -4149,6 +4150,10 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	if (res.fi->fib_nhs > 1)
 		fib_select_path(net, &res, &fl4, NULL);
 
+	mtu = ip_mtu_from_fib_result(&res, params->ipv4_dst);
+	if (params->tot_len > mtu)
+		return 0;
+
 	nh = &res.fi->fib_nh[res.nh_sel];
 
 	/* do not handle lwt encaps right now */

@@ -4188,6 +4193,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct flowi6 fl6;
 	int strict = 0;
 	int oif;
+	u32 mtu;
 
 	/* link local addresses are never forwarded */
 	if (rt6_need_strict(dst) || rt6_need_strict(src))

@@ -4250,6 +4256,10 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 						       fl6.flowi6_oif, NULL,
 						       strict);
 
+	mtu = ip6_mtu_from_fib6(f6i, dst, src);
+	if (params->tot_len > mtu)
+		return 0;
+
 	if (f6i->fib6_nh.nh_lwtstate)
 		return 0;

Signature · 1 line

2.11.0

Daniel Borkmann

maintainer

2018-05-17 22:22 UTC #4 lore raw

Re: [PATCH bpf-next 3/3] bpf: Add mtu checking to FIB forwarding helper

Quoted 42 lines · David Ahern

Signed-off-by David Ahern <[email protected]>

---
 net/core/filter.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

net/core/filter.c +6 -0

index 6d0d1560bd70..c47c47a75d4b 100644
@@ -4098,6 +4098,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct fib_nh *nh;
 	struct flowi4 fl4;
 	int err;
+	u32 mtu;
 
 	dev = dev_get_by_index_rcu(net, params->ifindex);
 	if (unlikely(!dev))

@@ -4149,6 +4150,10 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	if (res.fi->fib_nhs > 1)
 		fib_select_path(net, &res, &fl4, NULL);
 
+	mtu = ip_mtu_from_fib_result(&res, params->ipv4_dst);
+	if (params->tot_len > mtu)
+		return 0;
+
 	nh = &res.fi->fib_nh[res.nh_sel];
 
 	/* do not handle lwt encaps right now */

@@ -4188,6 +4193,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct flowi6 fl6;
 	int strict = 0;
 	int oif;
+	u32 mtu;
 
 	/* link local addresses are never forwarded */
 	if (rt6_need_strict(dst) || rt6_need_strict(src))

net/core/filter.c static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, · line 4264

Quoted 9 lines

net/core/filter.c +4 -0

@@ -4250,6 +4256,10 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 						       fl6.flowi6_oif, NULL,
 						       strict);
 
+	mtu = ip6_mtu_from_fib6(f6i, dst, src);
+	if (params->tot_len > mtu)
+		return 0;
+
 	if (f6i->fib6_nh.nh_lwtstate)
 		return 0;

Could you elaborate how this interacts in tc BPF use case where you have e.g. GSO packets and tot_len from aggregated packets would definitely be larger than MTU (e.g. see is_skb_forwardable() as one example on such checks)? Should this be an opt-in via a new flag for the helper?

Thanks,
Daniel

David Ahern

author

2018-05-18 00:34 UTC #5 lore raw

Re: [PATCH bpf-next 3/3] bpf: Add mtu checking to FIB forwarding helper

Quoted 46 lines · Daniel Borkmann

Quoted 45 lines · David Ahern

Signed-off-by David Ahern <[email protected]>

---
 net/core/filter.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

net/core/filter.c +6 -0

index 6d0d1560bd70..c47c47a75d4b 100644
@@ -4098,6 +4098,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct fib_nh *nh;
 	struct flowi4 fl4;
 	int err;
+	u32 mtu;
 
 	dev = dev_get_by_index_rcu(net, params->ifindex);
 	if (unlikely(!dev))

@@ -4149,6 +4150,10 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	if (res.fi->fib_nhs > 1)
 		fib_select_path(net, &res, &fl4, NULL);
 
+	mtu = ip_mtu_from_fib_result(&res, params->ipv4_dst);
+	if (params->tot_len > mtu)
+		return 0;
+
 	nh = &res.fi->fib_nh[res.nh_sel];
 
 	/* do not handle lwt encaps right now */

@@ -4188,6 +4193,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct flowi6 fl6;
 	int strict = 0;
 	int oif;
+	u32 mtu;
 
 	/* link local addresses are never forwarded */
 	if (rt6_need_strict(dst) || rt6_need_strict(src))

@@ -4250,6 +4256,10 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 						       fl6.flowi6_oif, NULL,
 						       strict);

Quoted 11 lines

net/core/filter.c static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, · line 4264
Quoted 6 lines
net/core/filter.c +4 -0
+	mtu = ip6_mtu_from_fib6(f6i, dst, src);
+	if (params->tot_len > mtu)
+		return 0;
+
 	if (f6i->fib6_nh.nh_lwtstate)
 		return 0;
Could you elaborate how this interacts in tc BPF use case where you have e.g. GSO packets and tot_len from aggregated packets would definitely be larger than MTU (e.g. see is_skb_forwardable() as one example on such checks)? Should this be an opt-in via a new flag for the helper?

It should not be opt-in for XDP.

I could add a flag to the internal call -- bpf_skb_fib_lookup sets the flag to skip the MTU check in bpf_ipv4_fib_lookup and bpf_ipv6_fib_lookup.

For the skb case do you want bpf_skb_fib_lookup call is_skb_forwardable or leave that to the BPF program?

Daniel Borkmann

maintainer

2018-05-18 14:01 UTC #6 lore raw

Re: [PATCH bpf-next 3/3] bpf: Add mtu checking to FIB forwarding helper

Quoted 59 lines · David Ahern

Quoted 46 lines · Daniel Borkmann
Quoted 45 lines · David Ahern
Add check that egress MTU can handle packet to be forwarded. If the MTU is less than the packet lenght, return 0 meaning the packet is expected to continue up the stack for help - eg., fragmenting the packet or sending an ICMP.
Signed-off-by David Ahern <[email protected]>
---
 net/core/filter.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
net/core/filter.c +6 -0
index 6d0d1560bd70..c47c47a75d4b 100644
@@ -4098,6 +4098,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct fib_nh *nh;
 	struct flowi4 fl4;
 	int err;
+	u32 mtu;
 
 	dev = dev_get_by_index_rcu(net, params->ifindex);
 	if (unlikely(!dev))
@@ -4149,6 +4150,10 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	if (res.fi->fib_nhs > 1)
 		fib_select_path(net, &res, &fl4, NULL);
 
+	mtu = ip_mtu_from_fib_result(&res, params->ipv4_dst);
+	if (params->tot_len > mtu)
+		return 0;
+
 	nh = &res.fi->fib_nh[res.nh_sel];
 
 	/* do not handle lwt encaps right now */
@@ -4188,6 +4193,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	struct flowi6 fl6;
 	int strict = 0;
 	int oif;
+	u32 mtu;
 
 	/* link local addresses are never forwarded */
 	if (rt6_need_strict(dst) || rt6_need_strict(src))
@@ -4250,6 +4256,10 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 						       fl6.flowi6_oif, NULL,
 						       strict);
 
Quoted 11 lines
net/core/filter.c static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, · line 4264
Quoted 6 lines
net/core/filter.c +4 -0
+	mtu = ip6_mtu_from_fib6(f6i, dst, src);
+	if (params->tot_len > mtu)
+		return 0;
+
 	if (f6i->fib6_nh.nh_lwtstate)
 		return 0;
Could you elaborate how this interacts in tc BPF use case where you have e.g. GSO packets and tot_len from aggregated packets would definitely be larger than MTU (e.g. see is_skb_forwardable() as one example on such checks)? Should this be an opt-in via a new flag for the helper?

Quoted 1 line

It should not be opt-in for XDP.

Yes, correct, for XDP it should not.

Quoted 5 lines

I could add a flag to the internal call -- bpf_skb_fib_lookup sets the flag to skip the MTU check in bpf_ipv4_fib_lookup and bpf_ipv6_fib_lookup.
For the skb case do you want bpf_skb_fib_lookup call is_skb_forwardable or leave that to the BPF program?

I think it probably makes sense to add an internal (unexposed) flag or bool where we propagate skb_is_gso(skb) from bpf_skb_fib_lookup() call-site and have similar logic where we first check this bool and if false do the MTU check (so it still can get enforced for control packets). Thus probably nothing of that implementation detail needs to be exposed to the program author.

kbuild test robot

build bot

2018-05-20 06:42 UTC #7 lore raw

Re: [PATCH bpf-next 3/3] bpf: Add mtu checking to FIB forwarding helper

In reply to David Ahern

I love your patch! Yet something to improve:

Hi David,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/David-Ahern/bpf-Add-MTU-check-to-fib-lookup-helper/20180520-103417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.2.0

reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64

All errors (new ones prefixed by >>):

net/core/filter.o: In function `bpf_ipv6_fib_lookup':

>> filter.c:(.text+0x12072): undefined reference to `ip6_mtu_from_fib6'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

kbuild test robot

build bot

2018-05-20 11:14 UTC #8 lore raw

Re: [PATCH bpf-next 3/3] bpf: Add mtu checking to FIB forwarding helper

In reply to David Ahern

I love your patch! Yet something to improve:

Hi David,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/David-Ahern/bpf-Add-MTU-check-to-fib-lookup-helper/20180520-103417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-ws0-05200859 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

net/core/filter.o: In function `bpf_ipv6_fib_lookup':

>> net/core/filter.c:4259: undefined reference to `ip6_mtu_from_fib6'

vim +4259 net/core/filter.c

4182
4183 #if IS_ENABLED(CONFIG_IPV6)
4184 static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
4185 u32 flags)
4186 {
4187 struct in6_addr *src = (struct in6_addr *) params->ipv6_src;
4188 struct in6_addr *dst = (struct in6_addr *) params->ipv6_dst;
4189 struct neighbour *neigh;
4190 struct net_device *dev;
4191 struct inet6_dev *idev;
4192 struct fib6_info *f6i;
4193 struct flowi6 fl6;
4194 int strict = 0;
4195 int oif;
4196 u32 mtu;
4197
4198 /* link local addresses are never forwarded */
4199 if (rt6_need_strict(dst) || rt6_need_strict(src))
4200 return 0;
4201
4202 dev = dev_get_by_index_rcu(net, params->ifindex);
4203 if (unlikely(!dev))
4204 return -ENODEV;
4205
4206 idev = __in6_dev_get_safely(dev);
4207 if (unlikely(!idev || !net->ipv6.devconf_all->forwarding))
4208 return 0;
4209
4210 if (flags & BPF_FIB_LOOKUP_OUTPUT) {
4211 fl6.flowi6_iif = 1;
4212 oif = fl6.flowi6_oif = params->ifindex;
4213 } else {
4214 oif = fl6.flowi6_iif = params->ifindex;
4215 fl6.flowi6_oif = 0;
4216 strict = RT6_LOOKUP_F_HAS_SADDR;
4217 }
4218 fl6.flowlabel = params->flowlabel;
4219 fl6.flowi6_scope = 0;
4220 fl6.flowi6_flags = 0;
4221 fl6.mp_hash = 0;
4222
4223 fl6.flowi6_proto = params->l4_protocol;
4224 fl6.daddr = *dst;
4225 fl6.saddr = *src;
4226 fl6.fl6_sport = params->sport;
4227 fl6.fl6_dport = params->dport;
4228
4229 if (flags & BPF_FIB_LOOKUP_DIRECT) {
4230 u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
4231 struct fib6_table *tb;
4232
4233 tb = ipv6_stub->fib6_get_table(net, tbid);
4234 if (unlikely(!tb))
4235 return 0;
4236
4237 f6i = ipv6_stub->fib6_table_lookup(net, tb, oif, &fl6, strict);
4238 } else {
4239 fl6.flowi6_mark = 0;
4240 fl6.flowi6_secid = 0;
4241 fl6.flowi6_tun_key.tun_id = 0;
4242 fl6.flowi6_uid = sock_net_uid(net, NULL);
4243
4244 f6i = ipv6_stub->fib6_lookup(net, oif, &fl6, strict);
4245 }
4246
4247 if (unlikely(IS_ERR_OR_NULL(f6i) || f6i == net->ipv6.fib6_null_entry))
4248 return 0;
4249
4250 if (unlikely(f6i->fib6_flags & RTF_REJECT ||
4251 f6i->fib6_type != RTN_UNICAST))
4252 return 0;
4253
4254 if (f6i->fib6_nsiblings && fl6.flowi6_oif == 0)
4255 f6i = ipv6_stub->fib6_multipath_select(net, f6i, &fl6,
4256 fl6.flowi6_oif, NULL,
4257 strict);
4258

> 4259		mtu = ip6_mtu_from_fib6(f6i, dst, src);

4260 if (params->tot_len > mtu)
4261 return 0;
4262
4263 if (f6i->fib6_nh.nh_lwtstate)
4264 return 0;
4265
4266 if (f6i->fib6_flags & RTF_GATEWAY)
4267 *dst = f6i->fib6_nh.nh_gw;
4268
4269 dev = f6i->fib6_nh.nh_dev;
4270 params->rt_metric = f6i->fib6_metric;
4271
4272 /* xdp and cls_bpf programs are run in RCU-bh so rcu_read_lock_bh is
4273 * not needed here. Can not use __ipv6_neigh_lookup_noref here
4274 * because we need to get nd_tbl via the stub
4275 */
4276 neigh = ___neigh_lookup_noref(ipv6_stub->nd_tbl, neigh_key_eq128,
4277 ndisc_hashfn, dst, dev);
4278 if (neigh)
4279 return bpf_fib_set_fwd_params(params, neigh, dev);
4280
4281 return 0;
4282 }
4283 #endif
4284

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation