netdev - Re: [PATCH bpf-next 0/3] xdp: Add devmap

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: <87bly4zg8n.fsf@toke.dk>
Date:   Mon, 08 Jul 2019 17:40:56 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Jonathan Lemon <jlemon@...gsvamp.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>, netdev@...r.kernel.org,
        David Miller <davem@...emloft.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Björn Töpel <bjorn.topel@...il.com>
Subject: Re: [PATCH bpf-next 0/3] xdp: Add devmap_hash map type

"Jonathan Lemon" <jlemon@...gsvamp.com> writes:

> On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
>
>> This series adds a new map type, devmap_hash, that works like the 
>> existing
>> devmap type, but using a hash-based indexing scheme. This is useful 
>> for the use
>> case where a devmap is indexed by ifindex (for instance for use with 
>> the routing
>> table lookup helper). For this use case, the regular devmap needs to 
>> be sized
>> after the maximum ifindex number, not the number of devices in it. A 
>> hash-based
>> indexing scheme makes it possible to size the map after the number of 
>> devices it
>> should contain instead.
>
> This device hash map is sized at NETDEV_HASHENTRIES == 2^8 == 256. Is
> this actually smaller than an array? What ifindex values are you
> seeing?

Well, not in all cases, certainly. But machines with lots of virtual
interfaces (e.g., container hosts) can easily exceed that. Also, for a
devmap we charge the full size of max_entries * struct bpf_dtab_netdev
towards the locked memory cost on map creation. And since sizeof(struct
bpf_dtab_netdev) is 64, the size of the hashmap only corresponds to 32
entries...

But more importantly, it's a UI issue: Say you want to create a simple
program that uses the fib_lookup helper (something like the xdp_fwd
example under samples/bpf/). You know that you only want to route
between a couple of interfaces, so you naturally create a devmap that
can hold, say, 8 entries (just to be sure). This works fine on your
initial test, where the machine only has a couple of physical interfaces
brought up at boot. But then you try to run the same program on your
production server, where the interfaces you need to use just happen to
have ifindexes higher than 8, and now it breaks for no discernible
reason. Or even worse, if you remove and re-add an interface, you may no
longer be able to insert it into your map because the ifindex changed...

-Toke