netdev - Re: [PATCH RESEND v11 2/4] xdp: extend xdp_redirect

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]

Date:   Fri, 14 May 2021 08:21:56 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     Hangbin Liu <liuhangbin@...il.com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Jiri Benc <jbenc@...hat.com>,
        Eelco Chaudron <echaudro@...hat.com>, ast@...nel.org,
        Daniel Borkmann <daniel@...earbox.net>,
        Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        David Ahern <dsahern@...il.com>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Björn Töpel <bjorn.topel@...il.com>,
        Martin KaFai Lau <kafai@...com>, brouer@...hat.com
Subject: Re: [PATCH RESEND v11 2/4] xdp: extend xdp_redirect_map with
 broadcast support

On Thu, 13 May 2021 13:01:16 -0700
John Fastabend <john.fastabend@...il.com> wrote:

> Hangbin Liu wrote:
> > This patch adds two flags BPF_F_BROADCAST and BPF_F_EXCLUDE_INGRESS to
> > extend xdp_redirect_map for broadcast support.
> > 
> > With BPF_F_BROADCAST the packet will be broadcasted to all the interfaces
> > in the map. with BPF_F_EXCLUDE_INGRESS the ingress interface will be
> > excluded when do broadcasting.
> > 
> > When getting the devices in dev hash map via dev_map_hash_get_next_key(),
> > there is a possibility that we fall back to the first key when a device
> > was removed. This will duplicate packets on some interfaces. So just walk
> > the whole buckets to avoid this issue. For dev array map, we also walk the
> > whole map to find valid interfaces.
> > 
> > Function bpf_clear_redirect_map() was removed in
> > commit ee75aef23afe ("bpf, xdp: Restructure redirect actions").
> > Add it back as we need to use ri->map again.
> > 
> > With test topology:
> >   +-------------------+             +-------------------+
> >   | Host A (i40e 10G) |  ---------- | eno1(i40e 10G)    |
> >   +-------------------+             |                   |
> >                                     |   Host B          |
> >   +-------------------+             |                   |
> >   | Host C (i40e 10G) |  ---------- | eno2(i40e 10G)    |
> >   +-------------------+             |                   |
> >                                     |          +------+ |
> >                                     | veth0 -- | Peer | |
> >                                     | veth1 -- |      | |
> >                                     | veth2 -- |  NS  | |
> >                                     |          +------+ |
> >                                     +-------------------+
> > 
> > On Host A:
> >  # pktgen/pktgen_sample03_burst_single_flow.sh -i eno1 -d $dst_ip -m $dst_mac -s 64
> > 
> > On Host B(Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz, 128G Memory):
> > Use xdp_redirect_map and xdp_redirect_map_multi in samples/bpf for testing.
> > All the veth peers in the NS have a XDP_DROP program loaded. The
> > forward_map max_entries in xdp_redirect_map_multi is modify to 4.
> > 
> > Testing the performance impact on the regular xdp_redirect path with and
> > without patch (to check impact of additional check for broadcast mode):
> > 
> > 5.12 rc4         | redirect_map        i40e->i40e      |    2.0M |  9.7M
> > 5.12 rc4         | redirect_map        i40e->veth      |    1.7M | 11.8M
> > 5.12 rc4 + patch | redirect_map        i40e->i40e      |    2.0M |  9.6M
> > 5.12 rc4 + patch | redirect_map        i40e->veth      |    1.7M | 11.7M
> > 
> > Testing the performance when cloning packets with the redirect_map_multi
> > test, using a redirect map size of 4, filled with 1-3 devices:
> > 
> > 5.12 rc4 + patch | redirect_map multi  i40e->veth (x1) |    1.7M | 11.4M
> > 5.12 rc4 + patch | redirect_map multi  i40e->veth (x2) |    1.1M |  4.3M
> > 5.12 rc4 + patch | redirect_map multi  i40e->veth (x3) |    0.8M |  2.6M
> > 
> > Acked-by: Toke Høiland-Jørgensen <toke@...hat.com>
> > Acked-by: Martin KaFai Lau <kafai@...com>
> > Signed-off-by: Hangbin Liu <liuhangbin@...il.com>  
> 
> [...]
> 
> LGTM thanks for sticking with it.

+1

> Acked-by: John Fastabend <john.fastabend@...il.com>

Acked-by: Jesper Dangaard Brouer <brouer@...hat.com>

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer