[v13,1/7] fib: make lookup function type configurable
Checks
Commit Message
Add type argument to dir24_8_get_lookup_fn()
Now it supports 3 different lookup implementations:
RTE_FIB_DIR24_8_SCALAR_MACRO
RTE_FIB_DIR24_8_SCALAR_INLINE
RTE_FIB_DIR24_8_SCALAR_UNI
Add new rte_fib_set_lookup_fn() - user can change lookup
function type runtime.
Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
lib/librte_fib/dir24_8.c | 84 +++++++++++++++++++++++---------------
lib/librte_fib/dir24_8.h | 2 +-
lib/librte_fib/rte_fib.c | 21 +++++++++-
lib/librte_fib/rte_fib.h | 32 +++++++++++++++
lib/librte_fib/rte_fib_version.map | 1 +
5 files changed, 106 insertions(+), 34 deletions(-)
Comments
On 19/10/2020 16:05, Vladimir Medvedkin wrote:
> Add type argument to dir24_8_get_lookup_fn()
> Now it supports 3 different lookup implementations:
> RTE_FIB_DIR24_8_SCALAR_MACRO
> RTE_FIB_DIR24_8_SCALAR_INLINE
> RTE_FIB_DIR24_8_SCALAR_UNI
>
> Add new rte_fib_set_lookup_fn() - user can change lookup
> function type runtime.
>
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
On Mon, Oct 19, 2020 at 5:05 PM Vladimir Medvedkin
<vladimir.medvedkin@intel.com> wrote:
>
> Add type argument to dir24_8_get_lookup_fn()
> Now it supports 3 different lookup implementations:
> RTE_FIB_DIR24_8_SCALAR_MACRO
> RTE_FIB_DIR24_8_SCALAR_INLINE
> RTE_FIB_DIR24_8_SCALAR_UNI
>
> Add new rte_fib_set_lookup_fn() - user can change lookup
> function type runtime.
>
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
We create a fib object with a type: either RTE_FIB_DUMMY or
RTE_FIB_DIR24_8 (separate topic, we probably do not need
RTE_FIB_TYPE_MAX).
This lookup API is dir24_8 specific.
If we won't abstract the lookup selection type, why not change this
API as dir24_8 specific?
I.e. s/rte_fib_set_lookup_fn/rte_fib_dir24_8_set_lookup_fn/g
The same would apply to FIB6 trie implementation.
Hi David,
On 22/10/2020 12:52, David Marchand wrote:
> On Mon, Oct 19, 2020 at 5:05 PM Vladimir Medvedkin
> <vladimir.medvedkin@intel.com> wrote:
>>
>> Add type argument to dir24_8_get_lookup_fn()
>> Now it supports 3 different lookup implementations:
>> RTE_FIB_DIR24_8_SCALAR_MACRO
>> RTE_FIB_DIR24_8_SCALAR_INLINE
>> RTE_FIB_DIR24_8_SCALAR_UNI
>>
>> Add new rte_fib_set_lookup_fn() - user can change lookup
>> function type runtime.
>>
>> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
>> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>
> We create a fib object with a type: either RTE_FIB_DUMMY or
> RTE_FIB_DIR24_8 (separate topic, we probably do not need
> RTE_FIB_TYPE_MAX).
RTE_FIB_TYPE_MAX is used for early sanity check. I can remove it
(relying on that init_dataplane() will return error for improper type),
if you think that it is better to get rid of it.
>
> This lookup API is dir24_8 specific.
> If we won't abstract the lookup selection type, why not change this
> API as dir24_8 specific?
> I.e. s/rte_fib_set_lookup_fn/rte_fib_dir24_8_set_lookup_fn/g
>
> The same would apply to FIB6 trie implementation.
Good point. In future I want to add more data plane algorithms such as
DXR or Poptrie for example. In this case I don't really want to have
separate function for every supported algorithm, i.e. I think it is
better to have single rte_fib_set_lookup_fn(). But on the other hand it
needs to be generic in this case. In future releases I want to get rid
of different dir24_8's scalar implementations (MACRO/INLINE/UNI). After
this we can change types to algorithm agnostic names:
RTE_FIB_SCALAR,
RTE_FIB_VECTOR_AVX512
>
On Thu, Oct 22, 2020 at 5:12 PM Medvedkin, Vladimir
<vladimir.medvedkin@intel.com> wrote:
>
> Hi David,
>
> On 22/10/2020 12:52, David Marchand wrote:
> > On Mon, Oct 19, 2020 at 5:05 PM Vladimir Medvedkin
> > <vladimir.medvedkin@intel.com> wrote:
> >>
> >> Add type argument to dir24_8_get_lookup_fn()
> >> Now it supports 3 different lookup implementations:
> >> RTE_FIB_DIR24_8_SCALAR_MACRO
> >> RTE_FIB_DIR24_8_SCALAR_INLINE
> >> RTE_FIB_DIR24_8_SCALAR_UNI
> >>
> >> Add new rte_fib_set_lookup_fn() - user can change lookup
> >> function type runtime.
> >>
> >> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> >> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> >
> > We create a fib object with a type: either RTE_FIB_DUMMY or
> > RTE_FIB_DIR24_8 (separate topic, we probably do not need
> > RTE_FIB_TYPE_MAX).
> RTE_FIB_TYPE_MAX is used for early sanity check. I can remove it
> (relying on that init_dataplane() will return error for improper type),
> if you think that it is better to get rid of it.
Applications could start using it.
If you don't need it, don't expose it.
A validation on type <= RTE_FIB_DIR24_8 should be enough.
>
> >
> > This lookup API is dir24_8 specific.
> > If we won't abstract the lookup selection type, why not change this
> > API as dir24_8 specific?
> > I.e. s/rte_fib_set_lookup_fn/rte_fib_dir24_8_set_lookup_fn/g
> >
> > The same would apply to FIB6 trie implementation.
>
> Good point. In future I want to add more data plane algorithms such as
> DXR or Poptrie for example. In this case I don't really want to have
> separate function for every supported algorithm, i.e. I think it is
> better to have single rte_fib_set_lookup_fn(). But on the other hand it
> needs to be generic in this case. In future releases I want to get rid
> of different dir24_8's scalar implementations (MACRO/INLINE/UNI). After
> this we can change types to algorithm agnostic names:
> RTE_FIB_SCALAR,
> RTE_FIB_VECTOR_AVX512
Is there a real benefit from those 3 scalar lookup implementations for dir24_8 ?
Hello,
On 23/10/2020 11:29, David Marchand wrote:
> On Thu, Oct 22, 2020 at 5:12 PM Medvedkin, Vladimir
> <vladimir.medvedkin@intel.com> wrote:
>>
>> Hi David,
>>
>> On 22/10/2020 12:52, David Marchand wrote:
>>> On Mon, Oct 19, 2020 at 5:05 PM Vladimir Medvedkin
>>> <vladimir.medvedkin@intel.com> wrote:
>>>>
>>>> Add type argument to dir24_8_get_lookup_fn()
>>>> Now it supports 3 different lookup implementations:
>>>> RTE_FIB_DIR24_8_SCALAR_MACRO
>>>> RTE_FIB_DIR24_8_SCALAR_INLINE
>>>> RTE_FIB_DIR24_8_SCALAR_UNI
>>>>
>>>> Add new rte_fib_set_lookup_fn() - user can change lookup
>>>> function type runtime.
>>>>
>>>> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
>>>> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>>>
>>> We create a fib object with a type: either RTE_FIB_DUMMY or
>>> RTE_FIB_DIR24_8 (separate topic, we probably do not need
>>> RTE_FIB_TYPE_MAX).
>> RTE_FIB_TYPE_MAX is used for early sanity check. I can remove it
>> (relying on that init_dataplane() will return error for improper type),
>> if you think that it is better to get rid of it.
>
> Applications could start using it.
> If you don't need it, don't expose it.
>
> A validation on type <= RTE_FIB_DIR24_8 should be enough.
>
Will remove it in v14, thanks!
>
>>
>>>
>>> This lookup API is dir24_8 specific.
>>> If we won't abstract the lookup selection type, why not change this
>>> API as dir24_8 specific?
>>> I.e. s/rte_fib_set_lookup_fn/rte_fib_dir24_8_set_lookup_fn/g
>>>
>>> The same would apply to FIB6 trie implementation.
>>
>> Good point. In future I want to add more data plane algorithms such as
>> DXR or Poptrie for example. In this case I don't really want to have
>> separate function for every supported algorithm, i.e. I think it is
>> better to have single rte_fib_set_lookup_fn(). But on the other hand it
>> needs to be generic in this case. In future releases I want to get rid
>> of different dir24_8's scalar implementations (MACRO/INLINE/UNI). After
>> this we can change types to algorithm agnostic names:
>> RTE_FIB_SCALAR,
>> RTE_FIB_VECTOR_AVX512
>
> Is there a real benefit from those 3 scalar lookup implementations for dir24_8 ?
>
Initially I've sent 3 different implementations to get responses from
the community what implementation should I leave. Test results on
different IA CPU's shows that MACRO based implementation perform
slightly faster. So I think there is no benefit from keeping other
implementations.
>
@@ -45,13 +45,6 @@ struct dir24_8_tbl {
#define ROUNDUP(x, y) RTE_ALIGN_CEIL(x, (1 << (32 - y)))
-enum lookup_type {
- MACRO,
- INLINE,
- UNI
-};
-enum lookup_type test_lookup = MACRO;
-
static inline void *
get_tbl24_p(struct dir24_8_tbl *dp, uint32_t ip, uint8_t nh_sz)
{
@@ -252,35 +245,62 @@ dir24_8_lookup_bulk_uni(void *p, const uint32_t *ips,
}
}
+static inline rte_fib_lookup_fn_t
+get_scalar_fn(enum rte_fib_dir24_8_nh_sz nh_sz)
+{
+ switch (nh_sz) {
+ case RTE_FIB_DIR24_8_1B:
+ return dir24_8_lookup_bulk_1b;
+ case RTE_FIB_DIR24_8_2B:
+ return dir24_8_lookup_bulk_2b;
+ case RTE_FIB_DIR24_8_4B:
+ return dir24_8_lookup_bulk_4b;
+ case RTE_FIB_DIR24_8_8B:
+ return dir24_8_lookup_bulk_8b;
+ default:
+ return NULL;
+ }
+}
+
+static inline rte_fib_lookup_fn_t
+get_scalar_fn_inlined(enum rte_fib_dir24_8_nh_sz nh_sz)
+{
+ switch (nh_sz) {
+ case RTE_FIB_DIR24_8_1B:
+ return dir24_8_lookup_bulk_0;
+ case RTE_FIB_DIR24_8_2B:
+ return dir24_8_lookup_bulk_1;
+ case RTE_FIB_DIR24_8_4B:
+ return dir24_8_lookup_bulk_2;
+ case RTE_FIB_DIR24_8_8B:
+ return dir24_8_lookup_bulk_3;
+ default:
+ return NULL;
+ }
+}
+
rte_fib_lookup_fn_t
-dir24_8_get_lookup_fn(struct rte_fib_conf *fib_conf)
+dir24_8_get_lookup_fn(void *p, enum rte_fib_dir24_8_lookup_type type)
{
- enum rte_fib_dir24_8_nh_sz nh_sz = fib_conf->dir24_8.nh_sz;
+ enum rte_fib_dir24_8_nh_sz nh_sz;
+ struct dir24_8_tbl *dp = p;
- if (test_lookup == MACRO) {
- switch (nh_sz) {
- case RTE_FIB_DIR24_8_1B:
- return dir24_8_lookup_bulk_1b;
- case RTE_FIB_DIR24_8_2B:
- return dir24_8_lookup_bulk_2b;
- case RTE_FIB_DIR24_8_4B:
- return dir24_8_lookup_bulk_4b;
- case RTE_FIB_DIR24_8_8B:
- return dir24_8_lookup_bulk_8b;
- }
- } else if (test_lookup == INLINE) {
- switch (nh_sz) {
- case RTE_FIB_DIR24_8_1B:
- return dir24_8_lookup_bulk_0;
- case RTE_FIB_DIR24_8_2B:
- return dir24_8_lookup_bulk_1;
- case RTE_FIB_DIR24_8_4B:
- return dir24_8_lookup_bulk_2;
- case RTE_FIB_DIR24_8_8B:
- return dir24_8_lookup_bulk_3;
- }
- } else
+ if (dp == NULL)
+ return NULL;
+
+ nh_sz = dp->nh_sz;
+
+ switch (type) {
+ case RTE_FIB_DIR24_8_SCALAR_MACRO:
+ return get_scalar_fn(nh_sz);
+ case RTE_FIB_DIR24_8_SCALAR_INLINE:
+ return get_scalar_fn_inlined(nh_sz);
+ case RTE_FIB_DIR24_8_SCALAR_UNI:
return dir24_8_lookup_bulk_uni;
+ default:
+ return NULL;
+ }
+
return NULL;
}
@@ -22,7 +22,7 @@ void
dir24_8_free(void *p);
rte_fib_lookup_fn_t
-dir24_8_get_lookup_fn(struct rte_fib_conf *conf);
+dir24_8_get_lookup_fn(void *p, enum rte_fib_dir24_8_lookup_type type);
int
dir24_8_modify(struct rte_fib *fib, uint32_t ip, uint8_t depth,
@@ -107,7 +107,8 @@ init_dataplane(struct rte_fib *fib, __rte_unused int socket_id,
fib->dp = dir24_8_create(dp_name, socket_id, conf);
if (fib->dp == NULL)
return -rte_errno;
- fib->lookup = dir24_8_get_lookup_fn(conf);
+ fib->lookup = dir24_8_get_lookup_fn(fib->dp,
+ RTE_FIB_DIR24_8_SCALAR_MACRO);
fib->modify = dir24_8_modify;
return 0;
default:
@@ -317,3 +318,21 @@ rte_fib_get_rib(struct rte_fib *fib)
{
return (fib == NULL) ? NULL : fib->rib;
}
+
+int
+rte_fib_set_lookup_fn(struct rte_fib *fib,
+ enum rte_fib_dir24_8_lookup_type type)
+{
+ rte_fib_lookup_fn_t fn;
+
+ switch (fib->type) {
+ case RTE_FIB_DIR24_8:
+ fn = dir24_8_get_lookup_fn(fib->dp, type);
+ if (fn == NULL)
+ return -EINVAL;
+ fib->lookup = fn;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
@@ -58,6 +58,21 @@ enum rte_fib_dir24_8_nh_sz {
RTE_FIB_DIR24_8_8B
};
+/** Type of lookup function implementation */
+enum rte_fib_dir24_8_lookup_type {
+ RTE_FIB_DIR24_8_SCALAR_MACRO,
+ /**< Macro based lookup function */
+ RTE_FIB_DIR24_8_SCALAR_INLINE,
+ /**<
+ * Lookup implementation using inlined functions
+ * for different next hop sizes
+ */
+ RTE_FIB_DIR24_8_SCALAR_UNI
+ /**<
+ * Unified lookup function for all next hop sizes
+ */
+};
+
/** FIB configuration structure */
struct rte_fib_conf {
enum rte_fib_type type; /**< Type of FIB struct */
@@ -196,6 +211,23 @@ __rte_experimental
struct rte_rib *
rte_fib_get_rib(struct rte_fib *fib);
+/**
+ * Set lookup function based on type
+ *
+ * @param fib
+ * FIB object handle
+ * @param type
+ * type of lookup function
+ *
+ * @return
+ * -EINVAL on failure
+ * 0 on success
+ */
+__rte_experimental
+int
+rte_fib_set_lookup_fn(struct rte_fib *fib,
+ enum rte_fib_dir24_8_lookup_type type);
+
#ifdef __cplusplus
}
#endif
@@ -9,6 +9,7 @@ EXPERIMENTAL {
rte_fib_lookup_bulk;
rte_fib_get_dp;
rte_fib_get_rib;
+ rte_fib_set_lookup_fn;
rte_fib6_add;
rte_fib6_create;