Message ID | 1404808110-16314-1-git-send-email-simon.kuenzer@neclab.eu (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <simon.kuenzer@neclab.eu> Received: from mailer1.neclab.eu (mailer1.neclab.eu [195.37.70.40]) by dpdk.org (Postfix) with ESMTP id 0D34B5960 for <dev@dpdk.org>; Tue, 8 Jul 2014 11:14:00 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by mailer1.neclab.eu (Postfix) with ESMTP id F1E86FFFCF for <dev@dpdk.org>; Tue, 8 Jul 2014 11:14:21 +0200 (CEST) X-Virus-Scanned: Amavisd on Debian GNU/Linux (netlab.nec.de) Received: from mailer1.neclab.eu ([127.0.0.1]) by localhost (atlas-a.office.hd [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0wrYpNUAYorH for <dev@dpdk.org>; Tue, 8 Jul 2014 11:14:21 +0200 (CEST) X-ENC: Last-Hop-TLS-encrypted X-ENC: Last-Hop-TLS-encrypted Received: from METHONE.office.hd (methone.office.hd [192.168.24.54]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mailer1.neclab.eu (Postfix) with ESMTPS id D968BFFC42 for <dev@dpdk.org>; Tue, 8 Jul 2014 11:14:16 +0200 (CEST) Received: from localhost.localdomain (10.1.2.203) by skoll.office.hd (192.168.125.11) with Microsoft SMTP Server (TLS) id 14.1.323.3; Tue, 8 Jul 2014 11:14:16 +0200 From: Simon Kuenzer <simon.kuenzer@neclab.eu> To: <dev@dpdk.org> Date: Tue, 8 Jul 2014 10:28:30 +0200 Message-ID: <1404808110-16314-1-git-send-email-simon.kuenzer@neclab.eu> X-Mailer: git-send-email 1.7.10.4 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.1.2.203] Subject: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Tue, 08 Jul 2014 09:14:00 -0000 |
Commit Message
Simon Kuenzer
July 8, 2014, 8:28 a.m. UTC
This commit enables users to specify the lcore id that
is used as master lcore.
Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu>
---
lib/librte_eal/linuxapp/eal/eal.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
Comments
Here are some comments about the use case of this patch: This patch is especially useful in cases where DPDK applications scale their CPU resources at runtime via starting and stopping slave lcores. Since the coremask defines the maximum scale-out for such a application, the master lcore becomes to the minimum scale-in. Imagine, running multiple primary processed of such DPDK applications, users might want to overlap the coremasks for scaling. However, it would still make sense to run the master lcores on different CPU cores. In DPDK vSwitch we might end up in such a scenario with a future release: https://lists.01.org/pipermail/dpdk-ovs/2014-March/000770.html https://lists.01.org/pipermail/dpdk-ovs/2014-March/000773.html Thanks, Simon On 08.07.2014 10:28, Simon Kuenzer wrote: > This commit enables users to specify the lcore id that > is used as master lcore. > > Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu> > --- > lib/librte_eal/linuxapp/eal/eal.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c > index 573fd06..4ad5b9b 100644 > --- a/lib/librte_eal/linuxapp/eal/eal.c > +++ b/lib/librte_eal/linuxapp/eal/eal.c > @@ -101,6 +101,7 @@ > #define OPT_XEN_DOM0 "xen-dom0" > #define OPT_CREATE_UIO_DEV "create-uio-dev" > #define OPT_VFIO_INTR "vfio-intr" > +#define OPT_MASTER_LCORE "master-lcore" > > #define RTE_EAL_BLACKLIST_SIZE 0x100 > > @@ -336,6 +337,7 @@ eal_usage(const char *prgname) > "[--proc-type primary|secondary|auto] \n\n" > "EAL options:\n" > " -c COREMASK : A hexadecimal bitmask of cores to run on\n" > + " --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n" > " -n NUM : Number of memory channels\n" > " -v : Display version information on startup\n" > " -d LIB.so : add driver (can be used multiple times)\n" > @@ -468,6 +470,21 @@ eal_parse_coremask(const char *coremask) > return 0; > } > > +/* Changes the lcore id of the master thread */ > +static int > +eal_parse_master_lcore(const char *arg) > +{ > + struct rte_config *cfg = rte_eal_get_configuration(); > + int master_lcore = atoi(arg); > + > + if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE)) > + return -1; > + if (cfg->lcore_role[master_lcore] != ROLE_RTE) > + return -1; > + cfg->master_lcore = master_lcore; > + return 0; > +} > + > static int > eal_parse_syslog(const char *facility) > { > @@ -653,6 +670,7 @@ eal_parse_args(int argc, char **argv) > {OPT_HUGE_DIR, 1, 0, 0}, > {OPT_NO_SHCONF, 0, 0, 0}, > {OPT_PROC_TYPE, 1, 0, 0}, > + {OPT_MASTER_LCORE, 1, 0, 0}, > {OPT_FILE_PREFIX, 1, 0, 0}, > {OPT_SOCKET_MEM, 1, 0, 0}, > {OPT_PCI_WHITELIST, 1, 0, 0}, > @@ -802,6 +820,21 @@ eal_parse_args(int argc, char **argv) > else if (!strcmp(lgopts[option_index].name, OPT_PROC_TYPE)) { > internal_config.process_type = eal_parse_proc_type(optarg); > } > + else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) { > + if (!coremask_ok) { > + RTE_LOG(ERR, EAL, "please specify the master " > + "lcore id after specifying " > + "the coremask\n"); > + eal_usage(prgname); > + return -1; > + } > + if (eal_parse_master_lcore(optarg) < 0) { > + RTE_LOG(ERR, EAL, "invalid parameter for --" > + OPT_MASTER_LCORE "\n"); > + eal_usage(prgname); > + return -1; > + } > + } > else if (!strcmp(lgopts[option_index].name, OPT_FILE_PREFIX)) { > internal_config.hugefile_prefix = optarg; > } >
Comments? On 08.07.2014 11:42, Simon Kuenzer wrote: > Here are some comments about the use case of this patch: > > This patch is especially useful in cases where DPDK applications scale > their CPU resources at runtime via starting and stopping slave lcores. > Since the coremask defines the maximum scale-out for such a application, > the master lcore becomes to the minimum scale-in. > Imagine, running multiple primary processed of such DPDK applications, > users might want to overlap the coremasks for scaling. However, it would > still make sense to run the master lcores on different CPU cores. > > In DPDK vSwitch we might end up in such a scenario with a future release: > https://lists.01.org/pipermail/dpdk-ovs/2014-March/000770.html > https://lists.01.org/pipermail/dpdk-ovs/2014-March/000773.html > > Thanks, > > Simon > > On 08.07.2014 10:28, Simon Kuenzer wrote: >> This commit enables users to specify the lcore id that >> is used as master lcore. >> >> Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu> >> --- >> lib/librte_eal/linuxapp/eal/eal.c | 33 >> +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/lib/librte_eal/linuxapp/eal/eal.c >> b/lib/librte_eal/linuxapp/eal/eal.c >> index 573fd06..4ad5b9b 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal.c >> +++ b/lib/librte_eal/linuxapp/eal/eal.c >> @@ -101,6 +101,7 @@ >> #define OPT_XEN_DOM0 "xen-dom0" >> #define OPT_CREATE_UIO_DEV "create-uio-dev" >> #define OPT_VFIO_INTR "vfio-intr" >> +#define OPT_MASTER_LCORE "master-lcore" >> >> #define RTE_EAL_BLACKLIST_SIZE 0x100 >> >> @@ -336,6 +337,7 @@ eal_usage(const char *prgname) >> "[--proc-type primary|secondary|auto] \n\n" >> "EAL options:\n" >> " -c COREMASK : A hexadecimal bitmask of cores to run >> on\n" >> + " --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n" >> " -n NUM : Number of memory channels\n" >> " -v : Display version information on startup\n" >> " -d LIB.so : add driver (can be used multiple times)\n" >> @@ -468,6 +470,21 @@ eal_parse_coremask(const char *coremask) >> return 0; >> } >> >> +/* Changes the lcore id of the master thread */ >> +static int >> +eal_parse_master_lcore(const char *arg) >> +{ >> + struct rte_config *cfg = rte_eal_get_configuration(); >> + int master_lcore = atoi(arg); >> + >> + if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE)) >> + return -1; >> + if (cfg->lcore_role[master_lcore] != ROLE_RTE) >> + return -1; >> + cfg->master_lcore = master_lcore; >> + return 0; >> +} >> + >> static int >> eal_parse_syslog(const char *facility) >> { >> @@ -653,6 +670,7 @@ eal_parse_args(int argc, char **argv) >> {OPT_HUGE_DIR, 1, 0, 0}, >> {OPT_NO_SHCONF, 0, 0, 0}, >> {OPT_PROC_TYPE, 1, 0, 0}, >> + {OPT_MASTER_LCORE, 1, 0, 0}, >> {OPT_FILE_PREFIX, 1, 0, 0}, >> {OPT_SOCKET_MEM, 1, 0, 0}, >> {OPT_PCI_WHITELIST, 1, 0, 0}, >> @@ -802,6 +820,21 @@ eal_parse_args(int argc, char **argv) >> else if (!strcmp(lgopts[option_index].name, >> OPT_PROC_TYPE)) { >> internal_config.process_type = >> eal_parse_proc_type(optarg); >> } >> + else if (!strcmp(lgopts[option_index].name, >> OPT_MASTER_LCORE)) { >> + if (!coremask_ok) { >> + RTE_LOG(ERR, EAL, "please specify the master " >> + "lcore id after specifying " >> + "the coremask\n"); >> + eal_usage(prgname); >> + return -1; >> + } >> + if (eal_parse_master_lcore(optarg) < 0) { >> + RTE_LOG(ERR, EAL, "invalid parameter for --" >> + OPT_MASTER_LCORE "\n"); >> + eal_usage(prgname); >> + return -1; >> + } >> + } >> else if (!strcmp(lgopts[option_index].name, >> OPT_FILE_PREFIX)) { >> internal_config.hugefile_prefix = optarg; >> } >> >
Hi all, does anyone have interest in this functionality? I think this is important and useful. Since we should care about core assignment to get high performance and the master lcore thread is special in DPDK, we will want to assign the master to the target core. For example, with hyperthreading I'd like to make a pair of packet processing threads into one physical core and separate the master thread which does some management. thanks, Hiroshi > Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id > > Comments? > > On 08.07.2014 11:42, Simon Kuenzer wrote: > > Here are some comments about the use case of this patch: > > > > This patch is especially useful in cases where DPDK applications scale > > their CPU resources at runtime via starting and stopping slave lcores. > > Since the coremask defines the maximum scale-out for such a application, > > the master lcore becomes to the minimum scale-in. > > Imagine, running multiple primary processed of such DPDK applications, > > users might want to overlap the coremasks for scaling. However, it would > > still make sense to run the master lcores on different CPU cores. > > > > In DPDK vSwitch we might end up in such a scenario with a future release: > > https://lists.01.org/pipermail/dpdk-ovs/2014-March/000770.html > > https://lists.01.org/pipermail/dpdk-ovs/2014-March/000773.html > > > > Thanks, > > > > Simon > > > > On 08.07.2014 10:28, Simon Kuenzer wrote: > >> This commit enables users to specify the lcore id that > >> is used as master lcore. > >> > >> Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu> > >> --- > >> lib/librte_eal/linuxapp/eal/eal.c | 33 > >> +++++++++++++++++++++++++++++++++ > >> 1 file changed, 33 insertions(+) > >> > >> diff --git a/lib/librte_eal/linuxapp/eal/eal.c > >> b/lib/librte_eal/linuxapp/eal/eal.c > >> index 573fd06..4ad5b9b 100644 > >> --- a/lib/librte_eal/linuxapp/eal/eal.c > >> +++ b/lib/librte_eal/linuxapp/eal/eal.c > >> @@ -101,6 +101,7 @@ > >> #define OPT_XEN_DOM0 "xen-dom0" > >> #define OPT_CREATE_UIO_DEV "create-uio-dev" > >> #define OPT_VFIO_INTR "vfio-intr" > >> +#define OPT_MASTER_LCORE "master-lcore" > >> > >> #define RTE_EAL_BLACKLIST_SIZE 0x100 > >> > >> @@ -336,6 +337,7 @@ eal_usage(const char *prgname) > >> "[--proc-type primary|secondary|auto] \n\n" > >> "EAL options:\n" > >> " -c COREMASK : A hexadecimal bitmask of cores to run > >> on\n" > >> + " --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n" > >> " -n NUM : Number of memory channels\n" > >> " -v : Display version information on startup\n" > >> " -d LIB.so : add driver (can be used multiple times)\n" > >> @@ -468,6 +470,21 @@ eal_parse_coremask(const char *coremask) > >> return 0; > >> } > >> > >> +/* Changes the lcore id of the master thread */ > >> +static int > >> +eal_parse_master_lcore(const char *arg) > >> +{ > >> + struct rte_config *cfg = rte_eal_get_configuration(); > >> + int master_lcore = atoi(arg); > >> + > >> + if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE)) > >> + return -1; > >> + if (cfg->lcore_role[master_lcore] != ROLE_RTE) > >> + return -1; > >> + cfg->master_lcore = master_lcore; > >> + return 0; > >> +} > >> + > >> static int > >> eal_parse_syslog(const char *facility) > >> { > >> @@ -653,6 +670,7 @@ eal_parse_args(int argc, char **argv) > >> {OPT_HUGE_DIR, 1, 0, 0}, > >> {OPT_NO_SHCONF, 0, 0, 0}, > >> {OPT_PROC_TYPE, 1, 0, 0}, > >> + {OPT_MASTER_LCORE, 1, 0, 0}, > >> {OPT_FILE_PREFIX, 1, 0, 0}, > >> {OPT_SOCKET_MEM, 1, 0, 0}, > >> {OPT_PCI_WHITELIST, 1, 0, 0}, > >> @@ -802,6 +820,21 @@ eal_parse_args(int argc, char **argv) > >> else if (!strcmp(lgopts[option_index].name, > >> OPT_PROC_TYPE)) { > >> internal_config.process_type = > >> eal_parse_proc_type(optarg); > >> } > >> + else if (!strcmp(lgopts[option_index].name, > >> OPT_MASTER_LCORE)) { > >> + if (!coremask_ok) { > >> + RTE_LOG(ERR, EAL, "please specify the master " > >> + "lcore id after specifying " > >> + "the coremask\n"); > >> + eal_usage(prgname); > >> + return -1; > >> + } > >> + if (eal_parse_master_lcore(optarg) < 0) { > >> + RTE_LOG(ERR, EAL, "invalid parameter for --" > >> + OPT_MASTER_LCORE "\n"); > >> + eal_usage(prgname); > >> + return -1; > >> + } > >> + } > >> else if (!strcmp(lgopts[option_index].name, > >> OPT_FILE_PREFIX)) { > >> internal_config.hugefile_prefix = optarg; > >> } > >> > >
Hi Hiroshi, 2014-07-22 23:40, Hiroshi Shimamoto: > does anyone have interest in this functionality? > > I think this is important and useful. > Since we should care about core assignment to get high performance > and the master lcore thread is special in DPDK, we will want to > assign the master to the target core. > For example, with hyperthreading I'd like to make a pair of packet > processing threads into one physical core and separate the master > thread which does some management. Thank you for showing your interest. Does it mean you carefully reviewed this patch? In this case, I'd appreciate a note "Reviewed-by:". Thanks
> Hi all, > > does anyone have interest in this functionality? Yes, I think this is useful for DPDK vSwitch. > > I think this is important and useful. > Since we should care about core assignment to get high performance and the > master lcore thread is special in DPDK, we will want to assign the master to > the target core. > For example, with hyperthreading I'd like to make a pair of packet processing > threads into one physical core and separate the master thread which does > some management. > > thanks, > Hiroshi >
Hi, > Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id > > Hi Hiroshi, > > 2014-07-22 23:40, Hiroshi Shimamoto: > > does anyone have interest in this functionality? > > > > I think this is important and useful. > > Since we should care about core assignment to get high performance > > and the master lcore thread is special in DPDK, we will want to > > assign the master to the target core. > > For example, with hyperthreading I'd like to make a pair of packet > > processing threads into one physical core and separate the master > > thread which does some management. > > Thank you for showing your interest. > Does it mean you carefully reviewed this patch? In this case, I'd appreciate > a note "Reviewed-by:". Not yet deeply, wait a bit, we're testing this patch in our application. Will report if it works fine. By the way, we should add the same code into the BSD code, right? thanks, Hiroshi > > Thanks > -- > Thomas
2014-07-23 08:53, Hiroshi Shimamoto: > 2014-07-23 09:50, Thomas Monjalon: > > 2014-07-22 23:40, Hiroshi Shimamoto: > > > does anyone have interest in this functionality? > > > > > > I think this is important and useful. > > > Since we should care about core assignment to get high performance > > > and the master lcore thread is special in DPDK, we will want to > > > assign the master to the target core. > > > For example, with hyperthreading I'd like to make a pair of packet > > > processing threads into one physical core and separate the master > > > thread which does some management. > > > > Thank you for showing your interest. > > Does it mean you carefully reviewed this patch? In this case, I'd appreciate > > a note "Reviewed-by:". > > Not yet deeply, wait a bit, we're testing this patch in our application. > Will report if it works fine. > > By the way, we should add the same code into the BSD code, right? Right. I'd prefer to reduce the duplicated footprint and have more common code between BSD and Linux. But waiting this enhancement, we have to maintain the duplicated code for BSD.
On 23.07.2014 11:04, Thomas Monjalon wrote: > 2014-07-23 08:53, Hiroshi Shimamoto: >> 2014-07-23 09:50, Thomas Monjalon: >>> 2014-07-22 23:40, Hiroshi Shimamoto: >>>> does anyone have interest in this functionality? >>>> >>>> I think this is important and useful. >>>> Since we should care about core assignment to get high performance >>>> and the master lcore thread is special in DPDK, we will want to >>>> assign the master to the target core. >>>> For example, with hyperthreading I'd like to make a pair of packet >>>> processing threads into one physical core and separate the master >>>> thread which does some management. >>> >>> Thank you for showing your interest. >>> Does it mean you carefully reviewed this patch? In this case, I'd appreciate >>> a note "Reviewed-by:". >> >> Not yet deeply, wait a bit, we're testing this patch in our application. >> Will report if it works fine. >> >> By the way, we should add the same code into the BSD code, right? > > Right. > I'd prefer to reduce the duplicated footprint and have more common code > between BSD and Linux. But waiting this enhancement, we have to maintain > the duplicated code for BSD. > Hi all, I can provide the same patch also for BSD. However, I do not have a machine to test it. Interested? Thanks, Simon
Hi all, the only issue I could imagine is that current DPDK applications are utilizing the implicit assumption that the master lcore is always set to the first available lcore. I would consider this as a "bug" in the application because it sets up its worker threads not "properly". However, as far I could check it, the DPDK framework seems to cope with it correctly. It would be nice if somebody else could confirm my statement. Thanks, Simon On 23.07.2014 10:53, Hiroshi Shimamoto wrote: > Hi, > >> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id >> >> Hi Hiroshi, >> >> 2014-07-22 23:40, Hiroshi Shimamoto: >>> does anyone have interest in this functionality? >>> >>> I think this is important and useful. >>> Since we should care about core assignment to get high performance >>> and the master lcore thread is special in DPDK, we will want to >>> assign the master to the target core. >>> For example, with hyperthreading I'd like to make a pair of packet >>> processing threads into one physical core and separate the master >>> thread which does some management. >> >> Thank you for showing your interest. >> Does it mean you carefully reviewed this patch? In this case, I'd appreciate >> a note "Reviewed-by:". > > Not yet deeply, wait a bit, we're testing this patch in our application. > Will report if it works fine. > > By the way, we should add the same code into the BSD code, right? > > thanks, > Hiroshi > >> >> Thanks >> -- >> Thomas
Hi, > Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id > > 2014-07-23 08:53, Hiroshi Shimamoto: > > 2014-07-23 09:50, Thomas Monjalon: > > > 2014-07-22 23:40, Hiroshi Shimamoto: > > > > does anyone have interest in this functionality? > > > > > > > > I think this is important and useful. > > > > Since we should care about core assignment to get high performance > > > > and the master lcore thread is special in DPDK, we will want to > > > > assign the master to the target core. > > > > For example, with hyperthreading I'd like to make a pair of packet > > > > processing threads into one physical core and separate the master > > > > thread which does some management. > > > > > > Thank you for showing your interest. > > > Does it mean you carefully reviewed this patch? In this case, I'd appreciate > > > a note "Reviewed-by:". > > > > Not yet deeply, wait a bit, we're testing this patch in our application. > > Will report if it works fine. Sorry a delay, I had confirmed the functionality. I'm fine to add Reviewed-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> thanks, Hiroshi > > > > By the way, we should add the same code into the BSD code, right? > > Right. > I'd prefer to reduce the duplicated footprint and have more common code > between BSD and Linux. But waiting this enhancement, we have to maintain > the duplicated code for BSD. > > -- > Thomas
> On Jul 8, 2014, at 5:28 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote: > > + else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) { > + if (!coremask_ok) { > + RTE_LOG(ERR, EAL, "please specify the master " > + "lcore id after specifying " > + "the coremask\n"); > + eal_usage(prgname); > + return -1; > + } Hi Simon, I think that forcing a particular command line order is not that clean. It might be better to remove the cfg->master_lcore setting from eal_parse_coremask(), and defer the selection of the master lcore until all of the command-line options have been parsed. If —master-lcore was specified, save the value and use that, otherwise rte_get_next_lcore(-1, 0, 0) can return the first bit set in the coremask. -Aaron
Hi Simon, Thanks for the patch, this will be useful for us. I responded separately to your original post with one suggestion. Our application currently assumes that DPDK will assign the first bit set in the coremask to the master lcore. As far as I can tell, this is hard-coded as of 1.7.1. But we would like the ability for our application to specify any bit from the coremask to serve as the master lcore. I don’t see any compatibility issues with this. Existing applications should behave as before. Thomas, could this be accepted for the 1.8 release? Or will that only happen if the BSD side can be patched as well? -Aaron > On Jul 23, 2014, at 9:10 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote: > > Hi all, > > the only issue I could imagine is that current DPDK applications are > utilizing the implicit assumption that the master lcore is always set to > the first available lcore. I would consider this as a "bug" in the > application because it sets up its worker threads not "properly". > > However, as far I could check it, the DPDK framework seems to cope with > it correctly. > It would be nice if somebody else could confirm my statement. > > Thanks, > > Simon > > On 23.07.2014 10:53, Hiroshi Shimamoto wrote: >> Hi, >> >>> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id >>> >>> Hi Hiroshi, >>> >>> 2014-07-22 23:40, Hiroshi Shimamoto: >>>> does anyone have interest in this functionality? >>>> >>>> I think this is important and useful. >>>> Since we should care about core assignment to get high performance >>>> and the master lcore thread is special in DPDK, we will want to >>>> assign the master to the target core. >>>> For example, with hyperthreading I'd like to make a pair of packet >>>> processing threads into one physical core and separate the master >>>> thread which does some management. >>> >>> Thank you for showing your interest. >>> Does it mean you carefully reviewed this patch? In this case, I'd appreciate >>> a note "Reviewed-by:". >> >> Not yet deeply, wait a bit, we're testing this patch in our application. >> Will report if it works fine. >> >> By the way, we should add the same code into the BSD code, right? >> >> thanks, >> Hiroshi >> >>> >>> Thanks >>> -- >>> Thomas >
2014-11-03 13:02, Aaron Campbell: > Hi Simon, > > Thanks for the patch, this will be useful for us. I responded separately to your original post with one suggestion. > > Our application currently assumes that DPDK will assign the first bit set in the coremask to the master lcore. As far as I can tell, this is hard-coded as of 1.7.1. But we would like the ability for our application to specify any bit from the coremask to serve as the master lcore. > > I don’t see any compatibility issues with this. Existing applications should behave as before. > > Thomas, could this be accepted for the 1.8 release? Or will that only happen if the BSD side can be patched as well? No need for BSD side patch because option management is now common between BSD and Linux. I'm going to send an updated version of this patch. > > On Jul 23, 2014, at 9:10 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote: > > > > Hi all, > > > > the only issue I could imagine is that current DPDK applications are > > utilizing the implicit assumption that the master lcore is always set to > > the first available lcore. I would consider this as a "bug" in the > > application because it sets up its worker threads not "properly". > > > > However, as far I could check it, the DPDK framework seems to cope with > > it correctly. > > It would be nice if somebody else could confirm my statement. > > > > Thanks, > > > > Simon > > > > On 23.07.2014 10:53, Hiroshi Shimamoto wrote: > >> Hi, > >> > >>> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id > >>> > >>> Hi Hiroshi, > >>> > >>> 2014-07-22 23:40, Hiroshi Shimamoto: > >>>> does anyone have interest in this functionality? > >>>> > >>>> I think this is important and useful. > >>>> Since we should care about core assignment to get high performance > >>>> and the master lcore thread is special in DPDK, we will want to > >>>> assign the master to the target core. > >>>> For example, with hyperthreading I'd like to make a pair of packet > >>>> processing threads into one physical core and separate the master > >>>> thread which does some management. > >>> > >>> Thank you for showing your interest. > >>> Does it mean you carefully reviewed this patch? In this case, I'd appreciate > >>> a note "Reviewed-by:". > >> > >> Not yet deeply, wait a bit, we're testing this patch in our application. > >> Will report if it works fine. > >> > >> By the way, we should add the same code into the BSD code, right? > >> > >> thanks, > >> Hiroshi
2014-11-03 13:02, Aaron Campbell: > > On Jul 8, 2014, at 5:28 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote: > > > > + else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) { > > + if (!coremask_ok) { > > + RTE_LOG(ERR, EAL, "please specify the master " > > + "lcore id after specifying " > > + "the coremask\n"); > > + eal_usage(prgname); > > + return -1; > > + } > > > Hi Simon, > > I think that forcing a particular command line order is not that clean. > It might be better to remove the cfg->master_lcore setting from > eal_parse_coremask(), and defer the selection of the master lcore until > all of the command-line options have been parsed. If —master-lcore was > specified, save the value and use that, otherwise > rte_get_next_lcore(-1, 0, 0) can return the first bit set in the coremask. It's not sufficient: eal_parse_master_lcore() requires cfg->lcore_role to be set. There is a real dependency between these 2 options. I'm going to submit a v2. Feel free to improve it with another patch.
> On Nov 4, 2014, at 3:00 PM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote: > > 2014-11-03 13:02, Aaron Campbell: >>> On Jul 8, 2014, at 5:28 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote: >>> >>> + else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) { >>> + if (!coremask_ok) { >>> + RTE_LOG(ERR, EAL, "please specify the master " >>> + "lcore id after specifying " >>> + "the coremask\n"); >>> + eal_usage(prgname); >>> + return -1; >>> + } >> >> >> Hi Simon, >> >> I think that forcing a particular command line order is not that clean. >> It might be better to remove the cfg->master_lcore setting from >> eal_parse_coremask(), and defer the selection of the master lcore until >> all of the command-line options have been parsed. If —master-lcore was >> specified, save the value and use that, otherwise >> rte_get_next_lcore(-1, 0, 0) can return the first bit set in the coremask. > > It's not sufficient: eal_parse_master_lcore() requires cfg->lcore_role > to be set. There is a real dependency between these 2 options. > I'm going to submit a v2. Feel free to improve it with another patch. I was nit-picking; although it might be nice if the new option is given, to verify the specified lcore is in the coremask. I will ack v2 though and this can be improved some other time. -Aaron
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index 573fd06..4ad5b9b 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c @@ -101,6 +101,7 @@ #define OPT_XEN_DOM0 "xen-dom0" #define OPT_CREATE_UIO_DEV "create-uio-dev" #define OPT_VFIO_INTR "vfio-intr" +#define OPT_MASTER_LCORE "master-lcore" #define RTE_EAL_BLACKLIST_SIZE 0x100 @@ -336,6 +337,7 @@ eal_usage(const char *prgname) "[--proc-type primary|secondary|auto] \n\n" "EAL options:\n" " -c COREMASK : A hexadecimal bitmask of cores to run on\n" + " --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n" " -n NUM : Number of memory channels\n" " -v : Display version information on startup\n" " -d LIB.so : add driver (can be used multiple times)\n" @@ -468,6 +470,21 @@ eal_parse_coremask(const char *coremask) return 0; } +/* Changes the lcore id of the master thread */ +static int +eal_parse_master_lcore(const char *arg) +{ + struct rte_config *cfg = rte_eal_get_configuration(); + int master_lcore = atoi(arg); + + if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE)) + return -1; + if (cfg->lcore_role[master_lcore] != ROLE_RTE) + return -1; + cfg->master_lcore = master_lcore; + return 0; +} + static int eal_parse_syslog(const char *facility) { @@ -653,6 +670,7 @@ eal_parse_args(int argc, char **argv) {OPT_HUGE_DIR, 1, 0, 0}, {OPT_NO_SHCONF, 0, 0, 0}, {OPT_PROC_TYPE, 1, 0, 0}, + {OPT_MASTER_LCORE, 1, 0, 0}, {OPT_FILE_PREFIX, 1, 0, 0}, {OPT_SOCKET_MEM, 1, 0, 0}, {OPT_PCI_WHITELIST, 1, 0, 0}, @@ -802,6 +820,21 @@ eal_parse_args(int argc, char **argv) else if (!strcmp(lgopts[option_index].name, OPT_PROC_TYPE)) { internal_config.process_type = eal_parse_proc_type(optarg); } + else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) { + if (!coremask_ok) { + RTE_LOG(ERR, EAL, "please specify the master " + "lcore id after specifying " + "the coremask\n"); + eal_usage(prgname); + return -1; + } + if (eal_parse_master_lcore(optarg) < 0) { + RTE_LOG(ERR, EAL, "invalid parameter for --" + OPT_MASTER_LCORE "\n"); + eal_usage(prgname); + return -1; + } + } else if (!strcmp(lgopts[option_index].name, OPT_FILE_PREFIX)) { internal_config.hugefile_prefix = optarg; }