On 5/17/18 4:22 PM, Daniel Borkmann wrote: > On 05/17/2018 06:09 PM, David Ahern wrote: >> 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 <dsah...@gmail.com> >> --- >> net/core/filter.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/net/core/filter.c b/net/core/filter.c >> index 6d0d1560bd70..c47c47a75d4b 100644 >> --- a/net/core/filter.c >> +++ b/net/core/filter.c >> @@ -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; > > 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?
