netdev - Re: [bpf-next V1-RFC PATCH 01/14] xdp: base API for new XDP rx-queue info concept

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:   Mon, 18 Dec 2017 06:23:40 -0700
From:   David Ahern <dsahern@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        netdev@...r.kernel.org, gospo@...adcom.com, bjorn.topel@...el.com,
        michael.chan@...adcom.com
Subject: Re: [bpf-next V1-RFC PATCH 01/14] xdp: base API for new XDP rx-queue
 info concept

On 12/18/17 3:55 AM, Jesper Dangaard Brouer wrote:
> 
> Handling return-errors in the drivers complicated the driver code, as it
> involves unraveling and deallocating other RX-rings etc (that were
> already allocated) if the reg fails. (Also notice next patch will allow
> dev == NULL, if right ptype is set).
> 
> I'm not completely rejecting you idea, as this is a good optimization
> trick, which is to move validation checks to setup-time, thus allowing
> less validation checks at runtime.  I sort-of actually already did
> this, as I allow bpf to deref dev without NULL check.  I would argue
> this is good enough, as we will crash in a predictable way, as above
> WARN will point to which driver violated the API.
> 
> If people think it is valuable I can change this API to return an err?

Saeed's suggested API in a comment on patch 12 also removes most of the
WARN_ONs as it sets the device and index:

xdp_rxq_info_reg(netdev, rxq_index)
{
    rxqueue = dev->_rx + rxq_index;
    xdp_rxq = rxqueue.xdp_rxq;
        xdp_rxq_info_init(xdp_rxq);
    xdp_rxq.dev = netdev;
    xdp_rxq.queue_index = rxq_index;
}

xdp_rxq_info_unreg(netdev, rxq_index)
{
...
}