* [dpdk-stable] [PATCH v2] app/testpmd: revert setting default RSS [not found] <1541754129-26009-1-git-send-email-ophirmu@mellanox.com> @ 2018-11-09 9:36 ` Ophir Munk 2018-11-09 9:38 ` Ophir Munk 1 sibling, 0 replies; 6+ messages in thread From: Ophir Munk @ 2018-11-09 9:36 UTC (permalink / raw) To: Ophir Munk; +Cc: stable This reverts the patch that enables default RSS action by setting key=NULL and key_len=0. In current testpmd implementation a key pointer must exist if key_len!=0. For example, the following flow rule will cause a segmentation fault: flow create 0 <pattern> actions rss queues 0 1 end key_len 40 / end Fixes: a4391f8bae ("app/testpmd: set default RSS key as null") Cc: stable@dpdk.org Signed-off-by: Ophir Munk <ophirmu@mellanox.com> --- v1: Initial version v2: Update commit message (add Cc: stable@dpdk.org) app/test-pmd/cmdline_flow.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index 91e2e35..23ea7cc 100644 --- a/app/test-pmd/cmdline_flow.c +++ b/app/test-pmd/cmdline_flow.c @@ -3248,15 +3248,26 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *, .func = RTE_ETH_HASH_FUNCTION_DEFAULT, .level = 0, .types = rss_hf, - .key_len = 0, + .key_len = sizeof(action_rss_data->key), .queue_num = RTE_MIN(nb_rxq, ACTION_RSS_QUEUE_NUM), - .key = NULL, + .key = action_rss_data->key, .queue = action_rss_data->queue, }, + .key = "testpmd's default RSS hash key, " + "override it for better balancing", .queue = { 0 }, }; for (i = 0; i < action_rss_data->conf.queue_num; ++i) action_rss_data->queue[i] = i; + if (!port_id_is_invalid(ctx->port, DISABLED_WARN) && + ctx->port != (portid_t)RTE_PORT_ALL) { + struct rte_eth_dev_info info; + + rte_eth_dev_info_get(ctx->port, &info); + action_rss_data->conf.key_len = + RTE_MIN(sizeof(action_rss_data->key), + info.hash_key_size); + } action->conf = &action_rss_data->conf; return ret; } -- 1.8.3.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-stable] [PATCH v2] app/testpmd: revert setting default RSS [not found] <1541754129-26009-1-git-send-email-ophirmu@mellanox.com> 2018-11-09 9:36 ` [dpdk-stable] [PATCH v2] app/testpmd: revert setting default RSS Ophir Munk @ 2018-11-09 9:38 ` Ophir Munk [not found] ` <CGME20181109104253eucas1p1b60fccca0a181cc6bb974a3aea697748@eucas1p1.samsung.com> 2018-11-12 14:49 ` [dpdk-stable] [PATCH v2] " Ferruh Yigit 1 sibling, 2 replies; 6+ messages in thread From: Ophir Munk @ 2018-11-09 9:38 UTC (permalink / raw) To: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, dev, Adrien Mazarguil Cc: Asaf Penso, Shahaf Shuler, Thomas Monjalon, Olga Shern, Ophir Munk, stable This reverts the patch that enables default RSS action by setting key=NULL and key_len=0. In current testpmd implementation a key pointer must exist if key_len!=0. For example, the following flow rule will cause a segmentation fault: flow create 0 <pattern> actions rss queues 0 1 end key_len 40 / end Fixes: a4391f8bae ("app/testpmd: set default RSS key as null") Cc: stable@dpdk.org Signed-off-by: Ophir Munk <ophirmu@mellanox.com> --- v1: Initial version v2: Update commit message (add Cc: stable@dpdk.org) app/test-pmd/cmdline_flow.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index 91e2e35..23ea7cc 100644 --- a/app/test-pmd/cmdline_flow.c +++ b/app/test-pmd/cmdline_flow.c @@ -3248,15 +3248,26 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *, .func = RTE_ETH_HASH_FUNCTION_DEFAULT, .level = 0, .types = rss_hf, - .key_len = 0, + .key_len = sizeof(action_rss_data->key), .queue_num = RTE_MIN(nb_rxq, ACTION_RSS_QUEUE_NUM), - .key = NULL, + .key = action_rss_data->key, .queue = action_rss_data->queue, }, + .key = "testpmd's default RSS hash key, " + "override it for better balancing", .queue = { 0 }, }; for (i = 0; i < action_rss_data->conf.queue_num; ++i) action_rss_data->queue[i] = i; + if (!port_id_is_invalid(ctx->port, DISABLED_WARN) && + ctx->port != (portid_t)RTE_PORT_ALL) { + struct rte_eth_dev_info info; + + rte_eth_dev_info_get(ctx->port, &info); + action_rss_data->conf.key_len = + RTE_MIN(sizeof(action_rss_data->key), + info.hash_key_size); + } action->conf = &action_rss_data->conf; return ret; } -- 1.8.3.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CGME20181109104253eucas1p1b60fccca0a181cc6bb974a3aea697748@eucas1p1.samsung.com>]
* Re: [dpdk-stable] [v2] app/testpmd: revert setting default RSS [not found] ` <CGME20181109104253eucas1p1b60fccca0a181cc6bb974a3aea697748@eucas1p1.samsung.com> @ 2018-11-09 10:42 ` Ilya Maximets 2018-11-11 9:56 ` Ophir Munk 0 siblings, 1 reply; 6+ messages in thread From: Ilya Maximets @ 2018-11-09 10:42 UTC (permalink / raw) To: Ophir Munk, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, dev, Adrien Mazarguil Cc: Asaf Penso, Shahaf Shuler, Thomas Monjalon, Olga Shern, stable, Ferruh Yigit On 09.11.2018 12:38, Ophir Munk wrote: > This reverts the patch that enables default RSS action by setting > key=NULL and key_len=0. > In current testpmd implementation a key pointer must exist if > key_len!=0. For example, the following flow rule will cause a > segmentation fault: > flow create 0 <pattern> actions rss queues 0 1 end key_len 40 / end Maybe it's better to check that 'key_len' and 'key' passed both or none? BTW, is there any profit from the 'key_len' argument for testpmd? Can we just always use the size of the passed 'key' and drop the configurable from the user interface? Best regards, Ilya Maximets. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [v2] app/testpmd: revert setting default RSS 2018-11-09 10:42 ` [dpdk-stable] [v2] " Ilya Maximets @ 2018-11-11 9:56 ` Ophir Munk 2018-11-11 11:41 ` Shahaf Shuler 0 siblings, 1 reply; 6+ messages in thread From: Ophir Munk @ 2018-11-11 9:56 UTC (permalink / raw) To: Ilya Maximets, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, dev, Adrien Mazarguil Cc: Asaf Penso, Shahaf Shuler, Thomas Monjalon, Olga Shern, stable, Ferruh Yigit > -----Original Message----- > From: Ilya Maximets [mailto:i.maximets@samsung.com] > Sent: Friday, November 09, 2018 12:43 PM > To: Ophir Munk <ophirmu@mellanox.com>; Wenzhuo Lu > <wenzhuo.lu@intel.com>; Jingjing Wu <jingjing.wu@intel.com>; Bernard > Iremonger <bernard.iremonger@intel.com>; dev@dpdk.org; Adrien > Mazarguil <adrien.mazarguil@6wind.com> > Cc: Asaf Penso <asafp@mellanox.com>; Shahaf Shuler > <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; > Olga Shern <olgas@mellanox.com>; stable@dpdk.org; Ferruh Yigit > <ferruh.yigit@intel.com> > Subject: Re: [v2] app/testpmd: revert setting default RSS > > On 09.11.2018 12:38, Ophir Munk wrote: > > This reverts the patch that enables default RSS action by setting > > key=NULL and key_len=0. > > In current testpmd implementation a key pointer must exist if > > key_len!=0. For example, the following flow rule will cause a > > segmentation fault: > > flow create 0 <pattern> actions rss queues 0 1 end key_len 40 / end > > Maybe it's better to check that 'key_len' and 'key' passed both or none? I agree. However I don't see this option easily added to current testpmd flow implementation. Adrien - how would you recommend adding this check? Please note that currently if no key and no key_len are specified - testpmd still assign a dummy string. > > BTW, is there any profit from the 'key_len' argument for testpmd? > Can we just always use the size of the passed 'key' and drop the configurable > from the user interface? > If you just specify key without key_len then the key length is calculated implicitly from the key itself. So this is an already implemented feature. You can still use key_len (with different values) maybe for special case handling / debugging in the PMD. > Best regards, Ilya Maximets. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [v2] app/testpmd: revert setting default RSS 2018-11-11 9:56 ` Ophir Munk @ 2018-11-11 11:41 ` Shahaf Shuler 0 siblings, 0 replies; 6+ messages in thread From: Shahaf Shuler @ 2018-11-11 11:41 UTC (permalink / raw) To: Ophir Munk, Ilya Maximets, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, dev, Adrien Mazarguil Cc: Asaf Penso, Thomas Monjalon, Olga Shern, stable, Ferruh Yigit Sunday, November 11, 2018 11:56 AM, Ophir Munk: > Subject: RE: [v2] app/testpmd: revert setting default RSS > > On 09.11.2018 12:38, Ophir Munk wrote: > > > This reverts the patch that enables default RSS action by setting > > > key=NULL and key_len=0. > > > In current testpmd implementation a key pointer must exist if > > > key_len!=0. For example, the following flow rule will cause a > > > segmentation fault: > > > flow create 0 <pattern> actions rss queues 0 1 end key_len 40 / end > > > > Maybe it's better to check that 'key_len' and 'key' passed both or none? > > I agree. However I don't see this option easily added to current testpmd flow > implementation. > Adrien - how would you recommend adding this check? > Please note that currently if no key and no key_len are specified - testpmd > still assign a dummy string. AFAIU, this patch is to restore the previous behavior of testpmd. it might not be perfect, yet worked. The current mode is that testpmd is broken. So I suggest to take this patch as is, and have the optimization later/in other release. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH v2] app/testpmd: revert setting default RSS 2018-11-09 9:38 ` Ophir Munk [not found] ` <CGME20181109104253eucas1p1b60fccca0a181cc6bb974a3aea697748@eucas1p1.samsung.com> @ 2018-11-12 14:49 ` Ferruh Yigit 1 sibling, 0 replies; 6+ messages in thread From: Ferruh Yigit @ 2018-11-12 14:49 UTC (permalink / raw) To: Ophir Munk, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, dev, Adrien Mazarguil Cc: Asaf Penso, Shahaf Shuler, Thomas Monjalon, Olga Shern, stable On 11/9/2018 9:38 AM, Ophir Munk wrote: > This reverts the patch that enables default RSS action by setting > key=NULL and key_len=0. > In current testpmd implementation a key pointer must exist if > key_len!=0. For example, the following flow rule will cause a > segmentation fault: > flow create 0 <pattern> actions rss queues 0 1 end key_len 40 / end > > Fixes: a4391f8bae ("app/testpmd: set default RSS key as null") > Cc: stable@dpdk.org > > Signed-off-by: Ophir Munk <ophirmu@mellanox.com> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-12 14:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1541754129-26009-1-git-send-email-ophirmu@mellanox.com>
2018-11-09 9:36 ` [dpdk-stable] [PATCH v2] app/testpmd: revert setting default RSS Ophir Munk
2018-11-09 9:38 ` Ophir Munk
[not found] ` <CGME20181109104253eucas1p1b60fccca0a181cc6bb974a3aea697748@eucas1p1.samsung.com>
2018-11-09 10:42 ` [dpdk-stable] [v2] " Ilya Maximets
2018-11-11 9:56 ` Ophir Munk
2018-11-11 11:41 ` Shahaf Shuler
2018-11-12 14:49 ` [dpdk-stable] [PATCH v2] " Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).