Re: [ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-09-06 Thread Ilya Maximets
On 13.08.2019 19:37, Ilya Maximets wrote:
> This is highly useful to see on which core PMD is running by
> only looking at the thread name. Thread Id still allows to
> distinguish different threads running on the same core over the time:
> 
>|dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
>|dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
>|dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>
> 
> In gdb, top or any other utility it's useful to quickly catch up
> needed thread without parsing logs, memory or matching threads by port
> names they're handling.
> 
> Signed-off-by: Ilya Maximets 
> ---

Thanks, Eelco and David! Applied to master.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-08-20 Thread David Marchand
On Tue, Aug 20, 2019 at 12:06 PM Ilya Maximets  wrote:
>
> On 20.08.2019 13:00, David Marchand wrote:
> > On Wed, Aug 14, 2019 at 9:45 AM Ilya Maximets  
> > wrote:
> >>
> >> On 13.08.2019 19:46, Eelco Chaudron wrote:
> >>> This is a really good idea :) One remark should we make it %03d?
> >>
> >> There is a hard limit for the thread name. It's 15 meaningful chars 
> >> excluding the
> >> terminating null byte. 'pmd-c02/id:' is 11 bytes wide keeping 4 bytes for 
> >> the
> >> thread id. 'pmd-c002/id:' is 12 bytes wide with only 3 bytes remaining for 
> >> id.
> >> Thread ids could easily become big ( > 1000) for a long running process, 
> >> that is
> >> why %02d was chosen, to save some space.
> >
> > Do we really need the /id: part?
> > A c%03d/pmd prefix would keep the existing pmd%d pattern and save 3 
> > characters.
>
> This will break lib/ovs-thread.c:thread_is_pmd() function.

Ok, not really a problem to change from my pov.

I don't really like the /id: in the thread name, as usually userspace
thread names contains simple alphanumeric characters.
But I can see no problem with this so:
Reviewed-by: David Marchand 


--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-08-20 Thread Ilya Maximets
On 20.08.2019 13:00, David Marchand wrote:
> On Wed, Aug 14, 2019 at 9:45 AM Ilya Maximets  wrote:
>>
>> On 13.08.2019 19:46, Eelco Chaudron wrote:
>>>
>>>
>>> On 13 Aug 2019, at 18:37, Ilya Maximets wrote:
>>>
 This is highly useful to see on which core PMD is running by
 only looking at the thread name. Thread Id still allows to
 distinguish different threads running on the same core over the time:

|dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
|dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
|dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>

 In gdb, top or any other utility it's useful to quickly catch up
 needed thread without parsing logs, memory or matching threads by port
 names they're handling.

 Signed-off-by: Ilya Maximets 
 ---
  lib/dpif-netdev.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
 index d0a1c58ad..34ba03836 100644
 --- a/lib/dpif-netdev.c
 +++ b/lib/dpif-netdev.c
 @@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
  FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
  pmd = dp_netdev_get_pmd(dp, core->core_id);
  if (!pmd) {
 +struct ds name = DS_EMPTY_INITIALIZER;
 +
  pmd = xzalloc(sizeof *pmd);
  dp_netdev_configure_pmd(pmd, dp, core->core_id, 
 core->numa_id);
 -pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
 +
 +ds_put_format(&name, "pmd-c%02d/id:", core->core_id);
>>>
>>> This is a really good idea :) One remark should we make it %03d?
>>
>> There is a hard limit for the thread name. It's 15 meaningful chars 
>> excluding the
>> terminating null byte. 'pmd-c02/id:' is 11 bytes wide keeping 4 bytes for the
>> thread id. 'pmd-c002/id:' is 12 bytes wide with only 3 bytes remaining for 
>> id.
>> Thread ids could easily become big ( > 1000) for a long running process, 
>> that is
>> why %02d was chosen, to save some space.
> 
> Do we really need the /id: part?
> A c%03d/pmd prefix would keep the existing pmd%d pattern and save 3 
> characters.

This will break lib/ovs-thread.c:thread_is_pmd() function.

> 
> Otherwise, this is a good idea yes.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-08-20 Thread David Marchand
On Wed, Aug 14, 2019 at 9:45 AM Ilya Maximets  wrote:
>
> On 13.08.2019 19:46, Eelco Chaudron wrote:
> >
> >
> > On 13 Aug 2019, at 18:37, Ilya Maximets wrote:
> >
> >> This is highly useful to see on which core PMD is running by
> >> only looking at the thread name. Thread Id still allows to
> >> distinguish different threads running on the same core over the time:
> >>
> >>|dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
> >>|dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
> >>|dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>
> >>
> >> In gdb, top or any other utility it's useful to quickly catch up
> >> needed thread without parsing logs, memory or matching threads by port
> >> names they're handling.
> >>
> >> Signed-off-by: Ilya Maximets 
> >> ---
> >>  lib/dpif-netdev.c | 9 -
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> >> index d0a1c58ad..34ba03836 100644
> >> --- a/lib/dpif-netdev.c
> >> +++ b/lib/dpif-netdev.c
> >> @@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
> >>  FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
> >>  pmd = dp_netdev_get_pmd(dp, core->core_id);
> >>  if (!pmd) {
> >> +struct ds name = DS_EMPTY_INITIALIZER;
> >> +
> >>  pmd = xzalloc(sizeof *pmd);
> >>  dp_netdev_configure_pmd(pmd, dp, core->core_id, 
> >> core->numa_id);
> >> -pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
> >> +
> >> +ds_put_format(&name, "pmd-c%02d/id:", core->core_id);
> >
> > This is a really good idea :) One remark should we make it %03d?
>
> There is a hard limit for the thread name. It's 15 meaningful chars excluding 
> the
> terminating null byte. 'pmd-c02/id:' is 11 bytes wide keeping 4 bytes for the
> thread id. 'pmd-c002/id:' is 12 bytes wide with only 3 bytes remaining for id.
> Thread ids could easily become big ( > 1000) for a long running process, that 
> is
> why %02d was chosen, to save some space.

Do we really need the /id: part?
A c%03d/pmd prefix would keep the existing pmd%d pattern and save 3 characters.

Otherwise, this is a good idea yes.


-- 
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-08-14 Thread Eelco Chaudron



On 14 Aug 2019, at 9:45, Ilya Maximets wrote:


On 13.08.2019 19:46, Eelco Chaudron wrote:



On 13 Aug 2019, at 18:37, Ilya Maximets wrote:


This is highly useful to see on which core PMD is running by
only looking at the thread name. Thread Id still allows to
distinguish different threads running on the same core over the 
time:


   |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
   |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
   |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>

In gdb, top or any other utility it's useful to quickly catch up
needed thread without parsing logs, memory or matching threads by 
port

names they're handling.

Signed-off-by: Ilya Maximets 
---
 lib/dpif-netdev.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d0a1c58ad..34ba03836 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
 FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
 pmd = dp_netdev_get_pmd(dp, core->core_id);
 if (!pmd) {
+    struct ds name = DS_EMPTY_INITIALIZER;
+
 pmd = xzalloc(sizeof *pmd);
 dp_netdev_configure_pmd(pmd, dp, 
core->core_id, core->numa_id);
-    pmd->thread = ovs_thread_create("pmd", 
pmd_thread_main, pmd);

+
+    ds_put_format(&name, "pmd-c%02d/id:", 
core->core_id);


This is a really good idea :) One remark should we make it %03d?


There is a hard limit for the thread name. It's 15 meaningful chars 
excluding the
terminating null byte. 'pmd-c02/id:' is 11 bytes wide keeping 4 bytes 
for the
thread id. 'pmd-c002/id:' is 12 bytes wide with only 3 bytes remaining 
for id.
Thread ids could easily become big ( > 1000) for a long running 
process, that is

why %02d was chosen, to save some space.

BTW, even on a systems with 100+ CPUs I'm usually placing OVS threads 
on a lower
cores. Anyway, this is only the matter of a bit more visual beauty in 
the logs.


What do you think?


Makes sense

Acked-by: Eelco Chaudron 





+    pmd->thread = 
ovs_thread_create(ds_cstr(&name),
+    
pmd_thread_main, pmd);

+    ds_destroy(&name);
+
 VLOG_INFO("PMD thread on numa_id: %d, core 
id: %2d created.",
   pmd->numa_id, 
pmd->core_id);

 changed = true;
-- 
2.17.1




___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-08-14 Thread Ilya Maximets
On 13.08.2019 19:46, Eelco Chaudron wrote:
> 
> 
> On 13 Aug 2019, at 18:37, Ilya Maximets wrote:
> 
>> This is highly useful to see on which core PMD is running by
>> only looking at the thread name. Thread Id still allows to
>> distinguish different threads running on the same core over the time:
>>
>>    |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
>>    |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
>>    |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>
>>
>> In gdb, top or any other utility it's useful to quickly catch up
>> needed thread without parsing logs, memory or matching threads by port
>> names they're handling.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  lib/dpif-netdev.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index d0a1c58ad..34ba03836 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>  FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
>>  pmd = dp_netdev_get_pmd(dp, core->core_id);
>>  if (!pmd) {
>> +    struct ds name = DS_EMPTY_INITIALIZER;
>> +
>>  pmd = xzalloc(sizeof *pmd);
>>  dp_netdev_configure_pmd(pmd, dp, core->core_id, core->numa_id);
>> -    pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
>> +
>> +    ds_put_format(&name, "pmd-c%02d/id:", core->core_id);
> 
> This is a really good idea :) One remark should we make it %03d?

There is a hard limit for the thread name. It's 15 meaningful chars excluding 
the
terminating null byte. 'pmd-c02/id:' is 11 bytes wide keeping 4 bytes for the
thread id. 'pmd-c002/id:' is 12 bytes wide with only 3 bytes remaining for id.
Thread ids could easily become big ( > 1000) for a long running process, that is
why %02d was chosen, to save some space.

BTW, even on a systems with 100+ CPUs I'm usually placing OVS threads on a lower
cores. Anyway, this is only the matter of a bit more visual beauty in the logs.

What do you think?

> 
>> +    pmd->thread = ovs_thread_create(ds_cstr(&name),
>> +    pmd_thread_main, pmd);
>> +    ds_destroy(&name);
>> +
>>  VLOG_INFO("PMD thread on numa_id: %d, core id: %2d created.",
>>    pmd->numa_id, pmd->core_id);
>>  changed = true;
>> -- 
>> 2.17.1
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-08-13 Thread Eelco Chaudron




On 13 Aug 2019, at 18:37, Ilya Maximets wrote:


This is highly useful to see on which core PMD is running by
only looking at the thread name. Thread Id still allows to
distinguish different threads running on the same core over the time:

   |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
   |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
   |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>

In gdb, top or any other utility it's useful to quickly catch up
needed thread without parsing logs, memory or matching threads by port
names they're handling.

Signed-off-by: Ilya Maximets 
---
 lib/dpif-netdev.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d0a1c58ad..34ba03836 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
 FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
 pmd = dp_netdev_get_pmd(dp, core->core_id);
 if (!pmd) {
+struct ds name = DS_EMPTY_INITIALIZER;
+
 pmd = xzalloc(sizeof *pmd);
 dp_netdev_configure_pmd(pmd, dp, core->core_id, 
core->numa_id);
-pmd->thread = ovs_thread_create("pmd", pmd_thread_main, 
pmd);

+
+ds_put_format(&name, "pmd-c%02d/id:", core->core_id);


This is a really good idea :) One remark should we make it %03d?


+pmd->thread = ovs_thread_create(ds_cstr(&name),
+pmd_thread_main, pmd);
+ds_destroy(&name);
+
 VLOG_INFO("PMD thread on numa_id: %d, core id: %2d 
created.",

   pmd->numa_id, pmd->core_id);
 changed = true;
--
2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] dpif-netdev: Add core id in the PMD thread name.

2019-08-13 Thread Ilya Maximets
This is highly useful to see on which core PMD is running by
only looking at the thread name. Thread Id still allows to
distinguish different threads running on the same core over the time:

   |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
   |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
   |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>

In gdb, top or any other utility it's useful to quickly catch up
needed thread without parsing logs, memory or matching threads by port
names they're handling.

Signed-off-by: Ilya Maximets 
---
 lib/dpif-netdev.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d0a1c58ad..34ba03836 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
 FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
 pmd = dp_netdev_get_pmd(dp, core->core_id);
 if (!pmd) {
+struct ds name = DS_EMPTY_INITIALIZER;
+
 pmd = xzalloc(sizeof *pmd);
 dp_netdev_configure_pmd(pmd, dp, core->core_id, core->numa_id);
-pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
+
+ds_put_format(&name, "pmd-c%02d/id:", core->core_id);
+pmd->thread = ovs_thread_create(ds_cstr(&name),
+pmd_thread_main, pmd);
+ds_destroy(&name);
+
 VLOG_INFO("PMD thread on numa_id: %d, core id: %2d created.",
   pmd->numa_id, pmd->core_id);
 changed = true;
-- 
2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev