fib: add rib extension size parameter
Checks
Commit Message
This patch adds a new parameter to the fib configuration to specify
the size of the extension for internal RIB structure.
Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
app/test-fib/main.c | 2 ++
app/test/test_fib.c | 5 +++++
app/test/test_fib6.c | 5 +++++
app/test/test_fib6_perf.c | 1 +
app/test/test_fib_perf.c | 1 +
examples/l3fwd/l3fwd_fib.c | 2 ++
lib/fib/rte_fib.c | 2 +-
lib/fib/rte_fib.h | 2 ++
lib/fib/rte_fib6.c | 2 +-
lib/fib/rte_fib6.h | 2 ++
10 files changed, 22 insertions(+), 2 deletions(-)
Comments
> This patch adds a new parameter to the fib configuration to specify
> the size of the extension for internal RIB structure.
>
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Tested-by: Conor Walsh <conor.walsh@intel.com>
06/09/2021 17:55, Vladimir Medvedkin:
> This patch adds a new parameter to the fib configuration to specify
> the size of the extension for internal RIB structure.
It looks to be an announced API change.
What happens if the new field is not initialized in the app?
At least it would deserve a note in the release notes in API changes I think.
> --- a/examples/l3fwd/l3fwd_fib.c
> +++ b/examples/l3fwd/l3fwd_fib.c
> @@ -426,6 +426,7 @@ setup_fib(const int socketid)
> /* Create the fib IPv4 table. */
> config_ipv4.type = RTE_FIB_DIR24_8;
> config_ipv4.max_routes = (1 << 16);
> + config_ipv4.rib_ext_sz = 0;
> config_ipv4.default_nh = FIB_DEFAULT_HOP;
> config_ipv4.dir24_8.nh_sz = RTE_FIB_DIR24_8_4B;
> config_ipv4.dir24_8.num_tbl8 = (1 << 15);
> @@ -475,6 +476,7 @@ setup_fib(const int socketid)
>
> config.type = RTE_FIB6_TRIE;
> config.max_routes = (1 << 16) - 1;
> + config.rib_ext_sz = 0;
> config.default_nh = FIB_DEFAULT_HOP;
> config.trie.nh_sz = RTE_FIB6_TRIE_4B;
> config.trie.num_tbl8 = (1 << 15);
> diff --git a/lib/fib/rte_fib.c b/lib/fib/rte_fib.c
> index b354d4b..6ca180d 100644
> --- a/lib/fib/rte_fib.c
> +++ b/lib/fib/rte_fib.c
> @@ -164,7 +164,7 @@ rte_fib_create(const char *name, int socket_id, struct rte_fib_conf *conf)
> return NULL;
> }
>
> - rib_conf.ext_sz = 0;
> + rib_conf.ext_sz = conf->rib_ext_sz;
> rib_conf.max_nodes = conf->max_routes * 2;
>
> rib = rte_rib_create(name, socket_id, &rib_conf);
> diff --git a/lib/fib/rte_fib.h b/lib/fib/rte_fib.h
> index acad209..570b4b6 100644
> --- a/lib/fib/rte_fib.h
> +++ b/lib/fib/rte_fib.h
> @@ -84,6 +84,8 @@ struct rte_fib_conf {
> /** Default value returned on lookup if there is no route */
> uint64_t default_nh;
> int max_routes;
> + /** Size of the node extension in the internal RIB struct */
> + unsigned int rib_ext_sz;
> union {
> struct {
> enum rte_fib_dir24_8_nh_sz nh_sz;
Hi Thomas,
On 25/10/2021 19:23, Thomas Monjalon wrote:
> 06/09/2021 17:55, Vladimir Medvedkin:
>> This patch adds a new parameter to the fib configuration to specify
>> the size of the extension for internal RIB structure.
>
> It looks to be an announced API change.
> What happens if the new field is not initialized in the app?
> At least it would deserve a note in the release notes in API changes I think.
>
This patch was sent for an experimental library, so no update was required.
In the patch about the promotion of the FIB API to stable:
https://patches.dpdk.org/project/dpdk/patch/1630944075-363524-1-git-send-email-vladimir.medvedkin@intel.com/
I put a note about this patch and mentioned that it should be applied
before promotion API to stable.
Can we accept it now or we have to wait for next LTS release?
>> --- a/examples/l3fwd/l3fwd_fib.c
>> +++ b/examples/l3fwd/l3fwd_fib.c
>> @@ -426,6 +426,7 @@ setup_fib(const int socketid)
>> /* Create the fib IPv4 table. */
>> config_ipv4.type = RTE_FIB_DIR24_8;
>> config_ipv4.max_routes = (1 << 16);
>> + config_ipv4.rib_ext_sz = 0;
>> config_ipv4.default_nh = FIB_DEFAULT_HOP;
>> config_ipv4.dir24_8.nh_sz = RTE_FIB_DIR24_8_4B;
>> config_ipv4.dir24_8.num_tbl8 = (1 << 15);
>> @@ -475,6 +476,7 @@ setup_fib(const int socketid)
>>
>> config.type = RTE_FIB6_TRIE;
>> config.max_routes = (1 << 16) - 1;
>> + config.rib_ext_sz = 0;
>> config.default_nh = FIB_DEFAULT_HOP;
>> config.trie.nh_sz = RTE_FIB6_TRIE_4B;
>> config.trie.num_tbl8 = (1 << 15);
>> diff --git a/lib/fib/rte_fib.c b/lib/fib/rte_fib.c
>> index b354d4b..6ca180d 100644
>> --- a/lib/fib/rte_fib.c
>> +++ b/lib/fib/rte_fib.c
>> @@ -164,7 +164,7 @@ rte_fib_create(const char *name, int socket_id, struct rte_fib_conf *conf)
>> return NULL;
>> }
>>
>> - rib_conf.ext_sz = 0;
>> + rib_conf.ext_sz = conf->rib_ext_sz;
>> rib_conf.max_nodes = conf->max_routes * 2;
>>
>> rib = rte_rib_create(name, socket_id, &rib_conf);
>> diff --git a/lib/fib/rte_fib.h b/lib/fib/rte_fib.h
>> index acad209..570b4b6 100644
>> --- a/lib/fib/rte_fib.h
>> +++ b/lib/fib/rte_fib.h
>> @@ -84,6 +84,8 @@ struct rte_fib_conf {
>> /** Default value returned on lookup if there is no route */
>> uint64_t default_nh;
>> int max_routes;
>> + /** Size of the node extension in the internal RIB struct */
>> + unsigned int rib_ext_sz;
>> union {
>> struct {
>> enum rte_fib_dir24_8_nh_sz nh_sz;
>
>
>
26/10/2021 22:30, Medvedkin, Vladimir:
> Hi Thomas,
>
> On 25/10/2021 19:23, Thomas Monjalon wrote:
> > 06/09/2021 17:55, Vladimir Medvedkin:
> >> This patch adds a new parameter to the fib configuration to specify
> >> the size of the extension for internal RIB structure.
> >
> > It looks to be an announced API change.
> > What happens if the new field is not initialized in the app?
> > At least it would deserve a note in the release notes in API changes I think.
> >
>
> This patch was sent for an experimental library, so no update was required.
> In the patch about the promotion of the FIB API to stable:
>
> https://patches.dpdk.org/project/dpdk/patch/1630944075-363524-1-git-send-email-vladimir.medvedkin@intel.com/
>
> I put a note about this patch and mentioned that it should be applied
> before promotion API to stable.
Indeed, we did not wait.
> Can we accept it now or we have to wait for next LTS release?
I think it is OK in this release.
You didn't reply about the release notes. Should we notify an API change?
Note: even experimental functions should be notified in release notes
if it requires a code change in the app.
Hi Thomas,
On 27/10/2021 10:28, Thomas Monjalon wrote:
> 26/10/2021 22:30, Medvedkin, Vladimir:
>> Hi Thomas,
>>
>> On 25/10/2021 19:23, Thomas Monjalon wrote:
>>> 06/09/2021 17:55, Vladimir Medvedkin:
>>>> This patch adds a new parameter to the fib configuration to specify
>>>> the size of the extension for internal RIB structure.
>>>
>>> It looks to be an announced API change.
>>> What happens if the new field is not initialized in the app?
>>> At least it would deserve a note in the release notes in API changes I think.
>>>
>>
>> This patch was sent for an experimental library, so no update was required.
>> In the patch about the promotion of the FIB API to stable:
>>
>> https://patches.dpdk.org/project/dpdk/patch/1630944075-363524-1-git-send-email-vladimir.medvedkin@intel.com/
>>
>> I put a note about this patch and mentioned that it should be applied
>> before promotion API to stable.
>
> Indeed, we did not wait.
>
>> Can we accept it now or we have to wait for next LTS release?
>
> I think it is OK in this release.
>
That's great!
> You didn't reply about the release notes. Should we notify an API change?
> Note: even experimental functions should be notified in release notes
> if it requires a code change in the app.
>
Sure, I'll send v2 with release notes updates.
>
@@ -857,6 +857,7 @@ run_v4(void)
conf.type = get_fib_type();
conf.default_nh = def_nh;
conf.max_routes = config.nb_routes * 2;
+ conf.rib_ext_sz = 0;
if (conf.type == RTE_FIB_DIR24_8) {
conf.dir24_8.nh_sz = __builtin_ctz(config.ent_sz);
conf.dir24_8.num_tbl8 = RTE_MIN(config.tbl8,
@@ -1057,6 +1058,7 @@ run_v6(void)
conf.type = get_fib_type();
conf.default_nh = def_nh;
conf.max_routes = config.nb_routes * 2;
+ conf.rib_ext_sz = 0;
if (conf.type == RTE_FIB6_TRIE) {
conf.trie.nh_sz = __builtin_ctz(config.ent_sz);
conf.trie.num_tbl8 = RTE_MIN(config.tbl8,
@@ -36,6 +36,7 @@ test_create_invalid(void)
struct rte_fib_conf config;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB_DUMMY;
@@ -94,6 +95,7 @@ test_multiple_create(void)
struct rte_fib_conf config;
int32_t i;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB_DUMMY;
@@ -120,6 +122,7 @@ test_free_null(void)
struct rte_fib_conf config;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB_DUMMY;
@@ -146,6 +149,7 @@ test_add_del_invalid(void)
uint8_t depth = 24;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB_DUMMY;
@@ -320,6 +324,7 @@ test_lookup(void)
int ret;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = def_nh;
config.type = RTE_FIB_DUMMY;
@@ -38,6 +38,7 @@ test_create_invalid(void)
struct rte_fib6_conf config;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB6_DUMMY;
@@ -96,6 +97,7 @@ test_multiple_create(void)
struct rte_fib6_conf config;
int32_t i;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB6_DUMMY;
@@ -122,6 +124,7 @@ test_free_null(void)
struct rte_fib6_conf config;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB6_DUMMY;
@@ -149,6 +152,7 @@ test_add_del_invalid(void)
uint8_t depth = 24;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = 0;
config.type = RTE_FIB6_DUMMY;
@@ -338,6 +342,7 @@ test_lookup(void)
int ret;
config.max_routes = MAX_ROUTES;
+ config.rib_ext_sz = 0;
config.default_nh = def_nh;
config.type = RTE_FIB6_DUMMY;
@@ -78,6 +78,7 @@ test_fib6_perf(void)
conf.type = RTE_FIB6_TRIE;
conf.default_nh = 0;
conf.max_routes = 1000000;
+ conf.rib_ext_sz = 0;
conf.trie.nh_sz = RTE_FIB6_TRIE_4B;
conf.trie.num_tbl8 = RTE_MIN(get_max_nh(conf.trie.nh_sz), 1000000U);
@@ -323,6 +323,7 @@ test_fib_perf(void)
struct rte_fib_conf config;
config.max_routes = 2000000;
+ config.rib_ext_sz = 0;
config.type = RTE_FIB_DIR24_8;
config.default_nh = 0;
config.dir24_8.nh_sz = RTE_FIB_DIR24_8_4B;
@@ -426,6 +426,7 @@ setup_fib(const int socketid)
/* Create the fib IPv4 table. */
config_ipv4.type = RTE_FIB_DIR24_8;
config_ipv4.max_routes = (1 << 16);
+ config_ipv4.rib_ext_sz = 0;
config_ipv4.default_nh = FIB_DEFAULT_HOP;
config_ipv4.dir24_8.nh_sz = RTE_FIB_DIR24_8_4B;
config_ipv4.dir24_8.num_tbl8 = (1 << 15);
@@ -475,6 +476,7 @@ setup_fib(const int socketid)
config.type = RTE_FIB6_TRIE;
config.max_routes = (1 << 16) - 1;
+ config.rib_ext_sz = 0;
config.default_nh = FIB_DEFAULT_HOP;
config.trie.nh_sz = RTE_FIB6_TRIE_4B;
config.trie.num_tbl8 = (1 << 15);
@@ -164,7 +164,7 @@ rte_fib_create(const char *name, int socket_id, struct rte_fib_conf *conf)
return NULL;
}
- rib_conf.ext_sz = 0;
+ rib_conf.ext_sz = conf->rib_ext_sz;
rib_conf.max_nodes = conf->max_routes * 2;
rib = rte_rib_create(name, socket_id, &rib_conf);
@@ -84,6 +84,8 @@ struct rte_fib_conf {
/** Default value returned on lookup if there is no route */
uint64_t default_nh;
int max_routes;
+ /** Size of the node extension in the internal RIB struct */
+ unsigned int rib_ext_sz;
union {
struct {
enum rte_fib_dir24_8_nh_sz nh_sz;
@@ -165,7 +165,7 @@ rte_fib6_create(const char *name, int socket_id, struct rte_fib6_conf *conf)
return NULL;
}
- rib_conf.ext_sz = 0;
+ rib_conf.ext_sz = conf->rib_ext_sz;
rib_conf.max_nodes = conf->max_routes * 2;
rib = rte_rib6_create(name, socket_id, &rib_conf);
@@ -75,6 +75,8 @@ struct rte_fib6_conf {
/** Default value returned on lookup if there is no route */
uint64_t default_nh;
int max_routes;
+ /** Size of the node extension in the internal RIB struct */
+ unsigned int rib_ext_sz;
union {
struct {
enum rte_fib_trie_nh_sz nh_sz;