Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Jakub Kicinski <jakub.kicinski@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...

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?

-Toke