From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Fastabend Subject: Re: [bpf-next V4 PATCH 01/14] xdp: base API for new XDP rx-queue info concept Date: Thu, 4 Jan 2018 20:54:00 -0800 Message-ID: <77b400ae-1407-55fb-4356-30262ed0e121@gmail.com> References: <151497504273.18176.10177133999720101758.stgit@firesoul> <151497511370.18176.16970502050847272706.stgit@firesoul> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, dsahern@gmail.com, gospo@broadcom.com, bjorn.topel@intel.com, michael.chan@broadcom.com To: Jesper Dangaard Brouer , Daniel Borkmann , Alexei Starovoitov Return-path: Received: from mail-pg0-f65.google.com ([74.125.83.65]:34773 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751194AbeAEEyT (ORCPT ); Thu, 4 Jan 2018 23:54:19 -0500 Received: by mail-pg0-f65.google.com with SMTP id j4so1613507pgp.1 for ; Thu, 04 Jan 2018 20:54:19 -0800 (PST) In-Reply-To: <151497511370.18176.16970502050847272706.stgit@firesoul> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 01/03/2018 02:25 AM, Jesper Dangaard Brouer wrote: > This patch only introduce the core data structures and API functions. > All XDP enabled drivers must use the API before this info can used. > > There is a need for XDP to know more about the RX-queue a given XDP > frames have arrived on. For both the XDP bpf-prog and kernel side. > > Instead of extending xdp_buff each time new info is needed, the patch > creates a separate read-mostly struct xdp_rxq_info, that contains this > info. We stress this data/cache-line is for read-only info. This is > NOT for dynamic per packet info, use the data_meta for such use-cases. > > The performance advantage is this info can be setup at RX-ring init > time, instead of updating N-members in xdp_buff. A possible (driver > level) micro optimization is that xdp_buff->rxq assignment could be > done once per XDP/NAPI loop. The extra pointer deref only happens for > program needing access to this info (thus, no slowdown to existing > use-cases). > > Signed-off-by: Jesper Dangaard Brouer > --- > include/linux/filter.h | 2 + > include/net/xdp.h | 47 ++++++++++++++++++++++++++++++++++ > net/core/Makefile | 2 + > net/core/xdp.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 117 insertions(+), 1 deletion(-) > create mode 100644 include/net/xdp.h > create mode 100644 net/core/xdp.c > > diff --git a/include/linux/filter.h b/include/linux/filter.h > index 2b0df2703671..425056c7f96c 100644 > --- a/include/linux/filter.h > +++ b/include/linux/filter.h > @@ -20,6 +20,7 @@ > #include > #include > > +#include > #include Perhaps just 'struct xdp_rxq_info' is need here instead of the full include. At least that is the pattern used for sk_buff and sock. (by the way sorry for the late v4 feedback) > > #include > @@ -503,6 +504,7 @@ struct xdp_buff { > void *data_end; > void *data_meta; > void *data_hard_start; > + struct xdp_rxq_info *rxq; > }; > > /* Compute the linear packet data range [data, data_end) which > diff --git a/include/net/xdp.h b/include/net/xdp.h > new file mode 100644 > index 000000000000..86c41631a908 > --- /dev/null > +++ b/include/net/xdp.h [...] > + > +/* Returns 0 on success, negative on failure */ > +int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq, > + struct net_device *dev, u32 queue_index) > +{ > + if (xdp_rxq->reg_state == REG_STATE_UNUSED) { > + WARN(1, "Driver promised not to register this"); > + return -EINVAL; > + } > + > + if (xdp_rxq->reg_state == REG_STATE_REGISTERED) { > + WARN(1, "Missing unregister, handled but fix driver"); > + xdp_rxq_info_unreg(xdp_rxq); > + } > + > + if (!dev) { Seems a bit paranoid, driver passing a NULL dev would be badly broken. And probably not important but could make above tests unlikely(). > + WARN(1, "Missing net_device from driver"); > + return -ENODEV; > + } > + > + /* State either UNREGISTERED or NEW */ > + xdp_rxq_info_init(xdp_rxq); > + xdp_rxq->dev = dev; > + xdp_rxq->queue_index = queue_index; > + > + xdp_rxq->reg_state = REG_STATE_REGISTERED; > + return 0; > +} > +EXPORT_SYMBOL_GPL(xdp_rxq_info_reg); > + > +void xdp_rxq_info_unused(struct xdp_rxq_info *xdp_rxq) > +{ > + xdp_rxq->reg_state = REG_STATE_UNUSED; > +} > +EXPORT_SYMBOL_GPL(xdp_rxq_info_unused); >