On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
> Jakub Kicinski <jakub.kicin...@netronome.com> writes:
> > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index b63bc77af2d1..629661db36ee 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -7527,6 +7527,12 @@ static int fixup_bpf_calls(struct bpf_verifier_env
> >> *env)
> >> prog->dst_needed = 1;
> >> if (insn->imm == BPF_FUNC_get_prandom_u32)
> >> bpf_user_rnd_init_once();
> >> + if (insn->imm == BPF_FUNC_redirect) {
> >> + int err = dev_map_alloc_default_map();
> >> +
> >> + if (err)
> >> + return err;
> >> + }
> >> if (insn->imm == BPF_FUNC_override_return)
> >> prog->kprobe_override = 1;
> >> if (insn->imm == BPF_FUNC_tail_call) {
> >
> >> +int dev_map_alloc_default_map(void)
> >> +{
> >> + struct net *net = current->nsproxy->net_ns;
> >> + struct bpf_dtab *dtab, *old_dtab;
> >> + struct net_device *netdev;
> >> + union bpf_attr attr = {};
> >> + u32 idx;
> >> + int err;
> >
> > BPF programs don't obey by netns boundaries. The fact the program is
> > verified in one ns doesn't mean this is the only ns it will be used in :(
> > Meaning if any program is using the redirect map you may need a secret
> > map in every ns.. no?
>
> Ah, yes, good point. Totally didn't think about the fact that load and
> attach are decoupled. Hmm, guess I'll just have to move the call to
> alloc_default_map() to the point where the program is attached to an
> interface, then...
Possibly.. and you also need to handle the case where interface with a
program attached is moved, no?
> I trust it's safe to skip the allocation in case the program is
> offloaded to hardware, right? I.e., an offloaded program will never need
> to fall back to the kernel helper?
We will cross that bridge when we get there ;) I'd definitely want the
ability to do a redirect to a non-offloaded netdev (e.g. redirect to a
veth) via some fallback, but the plan is to try to only add support for
the map version of redirect on offload, anyway.
