netdev - Re: [PATCH bpf-next v5 3/6] xdp: Add devmap_hash map type for looking up devices by hashed index

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: <20190729181612.7bdbe16a@carbon>
Date:   Mon, 29 Jul 2019 18:16:12 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>, netdev@...r.kernel.org,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Björn Töpel <bjorn.topel@...il.com>,
        Yonghong Song <yhs@...com>, brouer@...hat.com
Subject: Re: [PATCH bpf-next v5 3/6] xdp: Add devmap_hash map type for
 looking up devices by hashed index

On Fri, 26 Jul 2019 18:06:55 +0200
Toke Høiland-Jørgensen <toke@...hat.com> wrote:

> From: Toke Høiland-Jørgensen <toke@...hat.com>
> 
> A common pattern when using xdp_redirect_map() is to create a device map
> where the lookup key is simply ifindex. Because device maps are arrays,
> this leaves holes in the map, and the map has to be sized to fit the
> largest ifindex, regardless of how many devices actually are actually
> needed in the map.
> 
> This patch adds a second type of device map where the key is looked up
> using a hashmap, instead of being used as an array index. This allows maps
> to be densely packed, so they can be smaller.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> Acked-by: Yonghong Song <yhs@...com>

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

[...]
> +static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
> +						    int idx)
> +{
> +	return &dtab->dev_index_head[idx & (dtab->n_buckets - 1)];
> +}

I was about to complain about, that you are not using a pre-calculated
MASK value, instead of doing the -1 operation each time.  But I looked
at the ASM code, and the LEA operation used does the -1 operation in
the same instruction, so I guess this makes no performance difference.

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