From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [bpf-next V1-RFC PATCH 01/14] xdp: base API for new XDP rx-queue info concept Date: Mon, 18 Dec 2017 06:23:40 -0700 Message-ID: <2effe097-6802-2020-075d-47cc3576f78f@gmail.com> References: <151316391502.14967.13292358380181773729.stgit@firesoul> <151316396600.14967.5648145904814220715.stgit@firesoul> <3b781865-be2a-740b-8403-fe47fea929bc@gmail.com> <20171218115501.3f1fcf36@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Daniel Borkmann , Alexei Starovoitov , netdev@vger.kernel.org, gospo@broadcom.com, bjorn.topel@intel.com, michael.chan@broadcom.com To: Jesper Dangaard Brouer Return-path: Received: from mail-pf0-f171.google.com ([209.85.192.171]:38771 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758906AbdLRNXm (ORCPT ); Mon, 18 Dec 2017 08:23:42 -0500 Received: by mail-pf0-f171.google.com with SMTP id u25so9668713pfg.5 for ; Mon, 18 Dec 2017 05:23:42 -0800 (PST) In-Reply-To: <20171218115501.3f1fcf36@redhat.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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) { ... }