From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3A99745B07; Fri, 11 Oct 2024 13:29:19 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 34D064028E; Fri, 11 Oct 2024 13:29:18 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 37AD240395 for ; Fri, 11 Oct 2024 13:29:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1728646155; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=l8IKntgMC5rnK+smfqiVZzenrD3vRoNqzTGlb+7oGyY=; b=ckk0+iTgXFTe45uSKzkb5Y284gHaulLs687gd+H73Z5ZQKp1WRBByaVUzRc6f1Selnpn7I fabk/8cMQsg7XYW0djQXkj0mxJUScnJTXxMB5qBG1T0flq5VWWJmhN0xJcUa/qmzyeRdgV Linz2ouG7iHa+l22ooYGJRnYnySl/ZA= Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-633-FTG5xUsRN2m3p8CJjJIE9Q-1; Fri, 11 Oct 2024 07:29:14 -0400 X-MC-Unique: FTG5xUsRN2m3p8CJjJIE9Q-1 Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-2fac504fb8cso16410661fa.1 for ; Fri, 11 Oct 2024 04:29:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728646153; x=1729250953; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=l8IKntgMC5rnK+smfqiVZzenrD3vRoNqzTGlb+7oGyY=; b=RBwSxHvcIGEJdi6YHvSh8TvFGM94UbIXrYkiDM7/E91CCIxlOSQQcJeZY5B38fsPTs v6X2gZbt8eiYgQw3zGoSQr/KwXv+zkMkxs7xHDKkH3hCaLOI8L+Rw31npJG4k/PMABdx HL+qtqzGs3nxIswUDj6qecjRD/Q7D7/y7zvVTu8c1Lu1Gubm+XyIIwz9ms+b5/n7y5Ma CAeLpyLNhzAvAOwtbKEiecC0QuNT1AePo7jbfFi9maYRATgTOi/T7kiRLQHPo0dtphob aILw9VeRG4b6SjI/kZ5OUaKProV2UfFKMV4gKhfmTB2X37hqpjeVwcTcigyxit3gEn6S qg8w== X-Gm-Message-State: AOJu0YzIa3FC7ysL6knJHjSq1jwJjf+2cGax4eN1jE/VD6rycJr4Ql8o IzrKf2+kdEnn7l8p6vFp/KvRwkQQpFDQ4vs4JscfAqfHU35FP59MZOgAjzR3ypgYH99gkOCNZbm TX6Ak4z2w+LCohfCNLBANG81iMd1OG5lKKLxvThlr06VtuhQhChliZiWIVLaXjt/Z4kQIduOEbb tWh7vtullPhq/4K40= X-Received: by 2002:a05:6512:a8a:b0:536:54d6:e6d6 with SMTP id 2adb3069b0e04-539da3c5ecbmr1131025e87.17.1728646153134; Fri, 11 Oct 2024 04:29:13 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF+xMEex4fQ7Z4y7kaqYu3yOXYB7GvqNcXRRgwD9BixfHtoiIUrWJn428EzFfEPMqyqJWKXXV6NCVXoqGU0yo0= X-Received: by 2002:a05:6512:a8a:b0:536:54d6:e6d6 with SMTP id 2adb3069b0e04-539da3c5ecbmr1131016e87.17.1728646152579; Fri, 11 Oct 2024 04:29:12 -0700 (PDT) MIME-Version: 1.0 References: <20241008173319.441494-1-vladimir.medvedkin@intel.com> <20241010112621.681773-1-vladimir.medvedkin@intel.com> In-Reply-To: <20241010112621.681773-1-vladimir.medvedkin@intel.com> From: David Marchand Date: Fri, 11 Oct 2024 13:29:01 +0200 Message-ID: Subject: Re: [PATCH v3] fib: network byte order IPv4 lookup To: Vladimir Medvedkin Cc: dev@dpdk.org, rjarry@redhat.com, mb@smartsharesystems.com, stephen@networkplumber.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Oct 10, 2024 at 1:26=E2=80=AFPM Vladimir Medvedkin wrote: > diff --git a/lib/fib/meson.build b/lib/fib/meson.build > index 6795f41a0a..8c03496cdc 100644 > --- a/lib/fib/meson.build > +++ b/lib/fib/meson.build > @@ -25,40 +25,28 @@ if dpdk_conf.has('RTE_ARCH_X86_64') and binutils_ok > # linked into main lib. > > # check if all required flags already enabled (variant a). > - acl_avx512_flags =3D ['__AVX512F__','__AVX512DQ__'] > - acl_avx512_on =3D true > - foreach f:acl_avx512_flags > + fib_avx512_flags =3D ['__AVX512F__','__AVX512DQ__', '__AVX512BW__'] > + fib_avx512_on =3D true > + foreach f:fib_avx512_flags > if cc.get_define(f, args: machine_args) =3D=3D '' > - acl_avx512_on =3D false > + fib_avx512_on =3D false > endif > endforeach Repeating comment on v2 that was lost because of duplicate submission (?): Please reuse the common checks recently merged, see for example: https://git.dpdk.org/dpdk/diff/drivers/event/dlb2/meson.build?id=3Def7a4025= cd714189dc333bb19ea60c2abdeffb7d > > - if acl_avx512_on =3D=3D true > - cflags +=3D ['-DCC_DIR24_8_AVX512_SUPPORT'] > - sources +=3D files('dir24_8_avx512.c') > - # TRIE AVX512 implementation uses avx512bw intrinsics along with > - # avx512f and avx512dq > - if cc.get_define('__AVX512BW__', args: machine_args) !=3D '' > - cflags +=3D ['-DCC_TRIE_AVX512_SUPPORT'] > - sources +=3D files('trie_avx512.c') > - endif > - elif cc.has_multi_arguments('-mavx512f', '-mavx512dq') > + if fib_avx512_on =3D=3D true > + cflags +=3D ['-DCC_DIR24_8_AVX512_SUPPORT', '-DCC_TRIE_AVX512_SU= PPORT'] Nit: now that both dir24_8 and trie share the same requirement, can we go with a simple CC_AVX512_SUPPORT? This is really a nit, I am ok if you prefer to separate both. > + sources +=3D files('dir24_8_avx512.c', 'trie_avx512.c') > + elif cc.has_multi_arguments('-mavx512f', '-mavx512dq', '-mavx512bw') > dir24_8_avx512_tmp =3D static_library('dir24_8_avx512_tmp', > 'dir24_8_avx512.c', > dependencies: static_rte_eal, > - c_args: cflags + ['-mavx512f', '-mavx512dq']) > + c_args: cflags + ['-mavx512f', '-mavx512dq', '-mavx512bw= ']) > objs +=3D dir24_8_avx512_tmp.extract_objects('dir24_8_avx512.c') > - cflags +=3D ['-DCC_DIR24_8_AVX512_SUPPORT'] > - # TRIE AVX512 implementation uses avx512bw intrinsics along with > - # avx512f and avx512dq > - if cc.has_argument('-mavx512bw') > - trie_avx512_tmp =3D static_library('trie_avx512_tmp', > + trie_avx512_tmp =3D static_library('trie_avx512_tmp', > 'trie_avx512.c', > dependencies: static_rte_eal, > - c_args: cflags + ['-mavx512f', \ > - '-mavx512dq', '-mavx512bw']) > - objs +=3D trie_avx512_tmp.extract_objects('trie_avx512.c') > - cflags +=3D ['-DCC_TRIE_AVX512_SUPPORT'] > - endif > + c_args: cflags + ['-mavx512f', '-mavx512dq', '-mavx512bw= ']) > + objs +=3D trie_avx512_tmp.extract_objects('trie_avx512.c') > + cflags +=3D ['-DCC_DIR24_8_AVX512_SUPPORT', '-DCC_TRIE_AVX512_SU= PPORT'] > endif > endif > diff --git a/lib/fib/rte_fib.c b/lib/fib/rte_fib.c > index 4f9fba5a4f..991e48b5ea 100644 > --- a/lib/fib/rte_fib.c > +++ b/lib/fib/rte_fib.c > @@ -42,6 +42,7 @@ EAL_REGISTER_TAILQ(rte_fib_tailq) > struct rte_fib { > char name[RTE_FIB_NAMESIZE]; > enum rte_fib_type type; /**< Type of FIB struct */ > + int flags; /**< Flags */ > struct rte_rib *rib; /**< RIB helper datastructure */ > void *dp; /**< pointer to the dataplane str= uct*/ > rte_fib_lookup_fn_t lookup; /**< FIB lookup function */ > @@ -110,7 +111,7 @@ init_dataplane(struct rte_fib *fib, __rte_unused int = socket_id, > if (fib->dp =3D=3D NULL) > return -rte_errno; > fib->lookup =3D dir24_8_get_lookup_fn(fib->dp, > - RTE_FIB_LOOKUP_DEFAULT); > + RTE_FIB_LOOKUP_DEFAULT, !!(fib->flags & RTE_FIB_F= LAG_LOOKUP_BE)); > fib->modify =3D dir24_8_modify; > return 0; > default: > @@ -214,6 +215,7 @@ rte_fib_create(const char *name, int socket_id, struc= t rte_fib_conf *conf) > rte_strlcpy(fib->name, name, sizeof(fib->name)); > fib->rib =3D rib; > fib->type =3D conf->type; > + fib->flags =3D conf->flags; In addition to Robin comments, I also have a concern on the extensibility aspect. conf->flags must be validated against known flags. Otherwise existing applications may pass wrong stuff and "work fine", until the day we had one more flag. > fib->def_nh =3D conf->default_nh; > ret =3D init_dataplane(fib, socket_id, conf); > if (ret < 0) { --=20 David Marchand