netdev - Re: [PATCH bpf-next 3/4] bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for bpf_fib

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <16cc33fe-4759-0a7b-1e03-0d77d2f79351@linux.dev>
Date:   Fri, 17 Feb 2023 09:13:11 -0800
From:   Martin KaFai Lau <martin.lau@...ux.dev>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     bpf@...r.kernel.org, 'Alexei Starovoitov ' <ast@...nel.org>,
        'Andrii Nakryiko ' <andrii@...nel.org>,
        netdev@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH bpf-next 3/4] bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for
 bpf_fib_lookup

On 2/17/23 8:00 AM, Daniel Borkmann wrote:
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 1503f61336b6..6c1956e36c97 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
> [...]
>> @@ -5838,21 +5836,28 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct 
>> bpf_fib_lookup *params,
>>       if (likely(nhc->nhc_gw_family != AF_INET6)) {
>>           if (nhc->nhc_gw_family)
>>               params->ipv4_dst = nhc->nhc_gw.ipv4;
>> -
>> -        neigh = __ipv4_neigh_lookup_noref(dev,
>> -                         (__force u32)params->ipv4_dst);
>>       } else {
>>           struct in6_addr *dst = (struct in6_addr *)params->ipv6_dst;
>>           params->family = AF_INET6;
>>           *dst = nhc->nhc_gw.ipv6;
>> -        neigh = __ipv6_neigh_lookup_noref_stub(dev, dst);
>>       }
>> +    if (flags & BPF_FIB_LOOKUP_SKIP_NEIGH)
>> +        goto set_fwd_params;
>> +
>> +    if (params->family == AF_INET6)
> 
> Nit, would have probably more intuitive to keep the same test also here
> (nhc->nhc_gw_family != AF_INET6), but either way, lgtm.

Ack.

> 
> Are you still required to fill the params->smac in bpf_fib_set_fwd_params()
> in that case, meaning, shouldn't bpf_redirect_neigh() take care of it as well
> from neigh_output()? Looks unnecessary and could be moved out too.

Good point. will move it out from bpf_fib_set_fwd_params also. Thanks for the 
review.