From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 26496C43381 for ; Fri, 22 Feb 2019 00:32:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DDCCB206B6 for ; Fri, 22 Feb 2019 00:32:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=netronome-com.20150623.gappssmtp.com header.i=@netronome-com.20150623.gappssmtp.com header.b="g60OodNX" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726298AbfBVAc2 (ORCPT ); Thu, 21 Feb 2019 19:32:28 -0500 Received: from mail-qt1-f195.google.com ([209.85.160.195]:33398 "EHLO mail-qt1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726178AbfBVAc2 (ORCPT ); Thu, 21 Feb 2019 19:32:28 -0500 Received: by mail-qt1-f195.google.com with SMTP id z39so737278qtz.0 for ; Thu, 21 Feb 2019 16:32:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :organization:mime-version:content-transfer-encoding; bh=u8yWPrV6eGUmivDFEknaTsdI57cPURD1v0V0TR8Wlss=; b=g60OodNXGrK8xkaYPPycmVb1gfT4LG6I4T8IF6bm5bpjrN0O2OvLmLPQNwwYFCq/5l dw54pEJkTuPXyWnSJM/DP9HRy6LjpnP7VV570HE6+REGSSnFddfHhNONcjtn6du5Kc20 bIRBwGbXdgMQv7zgzW8K5OXdthKPzbtJkmFNyAHqla8mdITgfU9Iekxb2yy2ck9T7c60 JpbM8mFSnexht7TqWLS1x+y1pw5oCtOtVrRzSsnnFTbwI3pdlw9yPsI050c+qHC0YQrc 8c2HoVE/RQLKnbR5EFr+si93C+iSy8sor4DD4DiTzhu4q94nvG5Z7oPHVtqS+a6sAboQ tcfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:organization:mime-version:content-transfer-encoding; bh=u8yWPrV6eGUmivDFEknaTsdI57cPURD1v0V0TR8Wlss=; b=ago0IyFpo5ezFauKYbS0tPsEGTaSbkbEvHK1rPXK/rqfkqdW/QT224BWnEfsAe4Nw6 PJSsu0yPJM6P4Vbf/m/8bolWTYBN3j0QBI3/nPBtKrSTbT3/SbQaztmUJcEPNNasFRqj fMzN80ob3xXYSN2luO/PmEwbXuBRnkQwgGNakEcbkYKDifWNZH6C56fMtSmMbsya7ade Sl9jELMLNTeKfwExKgDJY30S4WNU1trDwg3eDOYtuCd1AdPqdU+H24lHGT9Aw8H3p2mk mh0D8No7pSv6Br7Go+tmHzC39/2kqMdMhXpO8Qstgo0UkOqatPY5TcRguQyCoNQwN4J4 BPgg== X-Gm-Message-State: AHQUAub8z7IjSaMpZMWk3zHnEPpo5htSv9P30wQzBs+VPyImJABCn4dN yOll0x+mQUI/8VauVu7O9haImQ== X-Google-Smtp-Source: AHgI3Ib4gYpJ+nMwDZsvnl3RbLfjMNDwCOGEPKrZRpdE1sRZWXlF8BdgFiTRbJFztI/3UiTQ6NkUDA== X-Received: by 2002:a0c:b89e:: with SMTP id y30mr1105100qvf.52.1550795547304; Thu, 21 Feb 2019 16:32:27 -0800 (PST) Received: from cakuba.netronome.com ([66.60.152.14]) by smtp.gmail.com with ESMTPSA id l129sm187393qkb.44.2019.02.21.16.32.26 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 21 Feb 2019 16:32:27 -0800 (PST) Date: Thu, 21 Feb 2019 16:32:18 -0800 From: Jakub Kicinski To: Toke =?UTF-8?B?SMO4aWxhbmQtSsO4cmdlbnNlbg==?= Cc: David Miller , netdev@vger.kernel.org, Jesper Dangaard Brouer , Daniel Borkmann , Alexei Starovoitov Subject: Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex Message-ID: <20190221163218.72905325@cakuba.netronome.com> In-Reply-To: <874l8wiw3k.fsf@toke.dk> References: <155075021399.13610.12521373406832889226.stgit@alrua-x1> <155075021407.13610.6656977312753058829.stgit@alrua-x1> <20190221134923.53c40b11@cakuba.netronome.com> <874l8wiw3k.fsf@toke.dk> Organization: Netronome Systems, Ltd. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, 22 Feb 2019 00:02:23 +0100, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > Jakub Kicinski writes: >=20 > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke H=C3=B8iland-J=C3=B8rgensen wr= ote: =20 > >> A common pattern when using xdp_redirect_map() is to create a device m= ap > >> 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. > >>=20 > >> This patch adds a second type of device map where the key is interpret= ed as > >> an ifindex and looked up using a hashmap, instead of being used as an = array > >> index. This leads to maps being densely packed, so they can be smaller. > >>=20 > >> The default maps used by xdp_redirect() are changed to use the new map > >> type, which means that xdp_redirect() is no longer limited to ifindex = < 64, > >> but instead to 64 total simultaneous interfaces per network namespace.= This > >> also provides an easy way to compare the performance of devmap and > >> devmap_idx: > >>=20 > >> xdp_redirect_map (devmap): 8394560 pkt/s > >> xdp_redirect (devmap_idx): 8179480 pkt/s > >>=20 > >> Difference: 215080 pkt/s or 3.1 nanoseconds per packet. =20 > > > > Could you share what the ifindex mix was here, to arrive at these > > numbers? How does it compare to using an array but not keying with > > ifindex? =20 >=20 > Just the standard set on my test machine; ifindex 1 through 9, except 8 > in this case. So certainly no more than 1 ifindex in each hash bucket > for those numbers. Oh, I clearly misread your numbers, it's still slower than array, you just don't need the size limit. > >> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen =20 > > =20 > >> +static int dev_map_idx_update_elem(struct bpf_map *map, void *key, vo= id *value, > >> + u64 map_flags) > >> +{ > >> + struct bpf_dtab *dtab =3D container_of(map, struct bpf_dtab, map); > >> + struct bpf_dtab_netdev *dev, *old_dev; > >> + u32 idx =3D *(u32 *)key; > >> + u32 val =3D *(u32 *)value; > >> + u32 bit; > >> + > >> + if (unlikely(map_flags > BPF_EXIST)) > >> + return -EINVAL; > >> + if (unlikely(map_flags =3D=3D BPF_NOEXIST)) > >> + return -EEXIST; > >> + > >> + old_dev =3D __dev_map_idx_lookup_elem(map, idx); > >> + if (!val) { > >> + if (!old_dev) > >> + return 0; =20 > > > > IMHO this is a fairly strange mix of array and hashmap semantics. I > > think you should stick to hashmap behaviour AFA flags and > > update/delete goes. =20 >=20 > Yeah, the double book-keeping is a bit strange, but it allows the actual > forwarding and flush code to be reused between both types of maps. I > think this is worth the slight semantic confusion :) I'm not sure I was clear, let me try again :) Your get_next_key only reports existing indexes if I read the code right, so that's not an array - in an array indexes always exist. What follows inserting 0 should not be equivalent to delete and BPF_NOEXIST should be handled appropriately. =20 Different maps behave differently, I think it's worth trying to limit the divergence in how things behave to the basic array and a hashmap models when possible.