why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Xiaoping Yan (NSB)]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

Hi experts,

I'm trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     ...
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 9650 bytes --]

RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Xiaoping Yan (NSB)]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

Hi Anatoly

I see this multiple queue support is added by you.
Could you kindly help me to understand why rte_power_ethdev_pmgmt_queue_enable need other queues to be in stopped state?

commit 5dff9a72b0efeab02a2b71e52c4871805b7e64cb
Author: Anatoly Burakov anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>
Date:   Fri Jul 9 16:08:15 2021 +0000

power: support callbacks for multiple Rx queues


Thank you.

Br, Xiaoping

From: Xiaoping Yan (NSB)
Sent: 2023年2月16日 21:00
To: users@dpdk.org
Subject: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi experts,

I’m trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     …
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 12552 bytes --]

RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Burakov, Anatoly]

[-- Attachment #1: Type: text/plain, Size: 2231 bytes --]

Hi,

It is mainly because we’re install callbacks, which is not thread-safe unless the PMD is stopped. Our PMD’s internal config structures are not thread-safe. You should only start these ports after you configure everything.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com>
Sent: Monday, February 20, 2023 6:12 AM
To: users@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi Anatoly

I see this multiple queue support is added by you.
Could you kindly help me to understand why rte_power_ethdev_pmgmt_queue_enable need other queues to be in stopped state?

commit 5dff9a72b0efeab02a2b71e52c4871805b7e64cb
Author: Anatoly Burakov anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>
Date:   Fri Jul 9 16:08:15 2021 +0000

power: support callbacks for multiple Rx queues


Thank you.

Br, Xiaoping

From: Xiaoping Yan (NSB)
Sent: 2023年2月16日 21:00
To: users@dpdk.org<mailto:users@dpdk.org>
Subject: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi experts,

I’m trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     …
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 14434 bytes --]

RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Xiaoping Yan (NSB)]

[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]

Hi,

Thank you for the information.
I see from rte_eth_add_rx_callback, the callback is added to per queue data: rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
So it should not affect polling on other queues?


Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com>
Sent: 2023年2月20日 18:28
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com>; users@dpdk.org
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

It is mainly because we’re install callbacks, which is not thread-safe unless the PMD is stopped. Our PMD’s internal config structures are not thread-safe. You should only start these ports after you configure everything.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 6:12 AM
To: users@dpdk.org<mailto:users@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi Anatoly

I see this multiple queue support is added by you.
Could you kindly help me to understand why rte_power_ethdev_pmgmt_queue_enable need other queues to be in stopped state?

commit 5dff9a72b0efeab02a2b71e52c4871805b7e64cb
Author: Anatoly Burakov anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>
Date:   Fri Jul 9 16:08:15 2021 +0000

power: support callbacks for multiple Rx queues


Thank you.

Br, Xiaoping

From: Xiaoping Yan (NSB)
Sent: 2023年2月16日 21:00
To: users@dpdk.org<mailto:users@dpdk.org>
Subject: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi experts,

I’m trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     …
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 16914 bytes --]

RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Burakov, Anatoly]

[-- Attachment #1: Type: text/plain, Size: 3678 bytes --]

Well, technically, no, you’re right, it wouldn’t - not unless you start polling those queues from some other thread. We can’t prevent that from happening, so we figured the best way would be to just disallow queue starts until we’re done configuring everything. So, yes, we could relax that restriction, it’s just a matter of specifying what’s allowed and what’s not vs. just doing a blanket “no” and keeping things simple.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com>
Sent: Monday, February 20, 2023 1:41 PM
To: Burakov, Anatoly <anatoly.burakov@intel.com>; users@dpdk.org
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

Thank you for the information.
I see from rte_eth_add_rx_callback, the callback is added to per queue data: rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
So it should not affect polling on other queues?


Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Sent: 2023年2月20日 18:28
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

It is mainly because we’re install callbacks, which is not thread-safe unless the PMD is stopped. Our PMD’s internal config structures are not thread-safe. You should only start these ports after you configure everything.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 6:12 AM
To: users@dpdk.org<mailto:users@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi Anatoly

I see this multiple queue support is added by you.
Could you kindly help me to understand why rte_power_ethdev_pmgmt_queue_enable need other queues to be in stopped state?

commit 5dff9a72b0efeab02a2b71e52c4871805b7e64cb
Author: Anatoly Burakov anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>
Date:   Fri Jul 9 16:08:15 2021 +0000

power: support callbacks for multiple Rx queues


Thank you.

Br, Xiaoping

From: Xiaoping Yan (NSB)
Sent: 2023年2月16日 21:00
To: users@dpdk.org<mailto:users@dpdk.org>
Subject: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi experts,

I’m trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     …
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 18864 bytes --]

RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Xiaoping Yan (NSB)]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 4525 bytes --]

Hi,

>> not unless you start polling those queues from some other thread
I don¡¯t understand this point.
As the rx callback is per queue data, it seems to me only thing is we should not polling this queue  while we are modify its callback. So only this queue should be in stopped state (maybe even this is not required? Because I see atomic operation is used (__atomic_store_n in rte_eth_add_rx_callback and __atomic_load_n in rte_eth_rx_burst)).
Anyway, polling other queues on some other thread should not affect, right? Or can you help to explain a bit more on this?

Thank you very much.

Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com>
Sent: 2023Äê2ÔÂ20ÈÕ 21:59
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com>; users@dpdk.org
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Well, technically, no, you¡¯re right, it wouldn¡¯t ¨C not unless you start polling those queues from some other thread. We can¡¯t prevent that from happening, so we figured the best way would be to just disallow queue starts until we¡¯re done configuring everything. So, yes, we could relax that restriction, it¡¯s just a matter of specifying what¡¯s allowed and what¡¯s not vs. just doing a blanket ¡°no¡± and keeping things simple.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 1:41 PM
To: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

Thank you for the information.
I see from rte_eth_add_rx_callback, the callback is added to per queue data: rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
So it should not affect polling on other queues?


Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Sent: 2023Äê2ÔÂ20ÈÕ 18:28
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

It is mainly because we¡¯re install callbacks, which is not thread-safe unless the PMD is stopped. Our PMD¡¯s internal config structures are not thread-safe. You should only start these ports after you configure everything.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 6:12 AM
To: users@dpdk.org<mailto:users@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi Anatoly

I see this multiple queue support is added by you.
Could you kindly help me to understand why rte_power_ethdev_pmgmt_queue_enable need other queues to be in stopped state?

commit 5dff9a72b0efeab02a2b71e52c4871805b7e64cb
Author: Anatoly Burakov anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>
Date:   Fri Jul 9 16:08:15 2021 +0000

power: support callbacks for multiple Rx queues


Thank you.

Br, Xiaoping

From: Xiaoping Yan (NSB)
Sent: 2023Äê2ÔÂ16ÈÕ 21:00
To: users@dpdk.org<mailto:users@dpdk.org>
Subject: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi experts,

I¡¯m trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     ¡­
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 22568 bytes --]

RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Burakov, Anatoly]

[-- Attachment #1: Type: text/plain, Size: 6265 bytes --]

Ø  I don’t understand this point.

Technically, no one stops you from polling the same queue from any thread, not just the one you’re configuring with. It sounds like nothing anyone would do, but we prefer to be on the safe side 😊


Ø  Anyway, polling other queues on some other thread should not affect, right? Or can you help to explain a bit more on this?

It would not affect the callback, but it would affect internal structures of the PMD power management, if it’s one of those queues you are setting up for that. PMD callbacks appear to be thread-safe (so I was wrong about that), but the PMD power management internal structures aren’t, as the queue configuration will be shared among those queues that are participating in the scheme. We do modify some shared data when we’re triggering callbacks, so we do not want any queues to be polling while we configure things. Again, this would only matter if you tried to poll inbetween configuration, or polled from a different thread, so it doesn’t sound like anything anyone would do… But the API disallows that “just in case”.

So, I think you’re right in that it’s perfectly safe to start ports as long as you’re not polling them, but it’s simpler to tell users to not start the ports than it is to explain what you can or can’t do without things blowing up.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com>
Sent: Monday, February 20, 2023 2:10 PM
To: Burakov, Anatoly <anatoly.burakov@intel.com>; users@dpdk.org
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

>> not unless you start polling those queues from some other thread
I don’t understand this point.
As the rx callback is per queue data, it seems to me only thing is we should not polling this queue  while we are modify its callback. So only this queue should be in stopped state (maybe even this is not required? Because I see atomic operation is used (__atomic_store_n in rte_eth_add_rx_callback and __atomic_load_n in rte_eth_rx_burst)).
Anyway, polling other queues on some other thread should not affect, right? Or can you help to explain a bit more on this?

Thank you very much.

Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Sent: 2023年2月20日 21:59
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Well, technically, no, you’re right, it wouldn’t – not unless you start polling those queues from some other thread. We can’t prevent that from happening, so we figured the best way would be to just disallow queue starts until we’re done configuring everything. So, yes, we could relax that restriction, it’s just a matter of specifying what’s allowed and what’s not vs. just doing a blanket “no” and keeping things simple.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 1:41 PM
To: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

Thank you for the information.
I see from rte_eth_add_rx_callback, the callback is added to per queue data: rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
So it should not affect polling on other queues?


Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Sent: 2023年2月20日 18:28
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

It is mainly because we’re install callbacks, which is not thread-safe unless the PMD is stopped. Our PMD’s internal config structures are not thread-safe. You should only start these ports after you configure everything.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 6:12 AM
To: users@dpdk.org<mailto:users@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi Anatoly

I see this multiple queue support is added by you.
Could you kindly help me to understand why rte_power_ethdev_pmgmt_queue_enable need other queues to be in stopped state?

commit 5dff9a72b0efeab02a2b71e52c4871805b7e64cb
Author: Anatoly Burakov anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>
Date:   Fri Jul 9 16:08:15 2021 +0000

power: support callbacks for multiple Rx queues


Thank you.

Br, Xiaoping

From: Xiaoping Yan (NSB)
Sent: 2023年2月16日 21:00
To: users@dpdk.org<mailto:users@dpdk.org>
Subject: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi experts,

I’m trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     …
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 29119 bytes --]

RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

[Xiaoping Yan (NSB)]

[-- Attachment #1: Type: text/plain, Size: 6676 bytes --]

Hi,

Ok, I get your point now.
Thank you.

Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com>
Sent: 2023年2月20日 22:19
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com>; users@dpdk.org
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped


Ø  I don’t understand this point.

Technically, no one stops you from polling the same queue from any thread, not just the one you’re configuring with. It sounds like nothing anyone would do, but we prefer to be on the safe side 😊


Ø  Anyway, polling other queues on some other thread should not affect, right? Or can you help to explain a bit more on this?

It would not affect the callback, but it would affect internal structures of the PMD power management, if it’s one of those queues you are setting up for that. PMD callbacks appear to be thread-safe (so I was wrong about that), but the PMD power management internal structures aren’t, as the queue configuration will be shared among those queues that are participating in the scheme. We do modify some shared data when we’re triggering callbacks, so we do not want any queues to be polling while we configure things. Again, this would only matter if you tried to poll inbetween configuration, or polled from a different thread, so it doesn’t sound like anything anyone would do… But the API disallows that “just in case”.

So, I think you’re right in that it’s perfectly safe to start ports as long as you’re not polling them, but it’s simpler to tell users to not start the ports than it is to explain what you can or can’t do without things blowing up.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 2:10 PM
To: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

>> not unless you start polling those queues from some other thread
I don’t understand this point.
As the rx callback is per queue data, it seems to me only thing is we should not polling this queue  while we are modify its callback. So only this queue should be in stopped state (maybe even this is not required? Because I see atomic operation is used (__atomic_store_n in rte_eth_add_rx_callback and __atomic_load_n in rte_eth_rx_burst)).
Anyway, polling other queues on some other thread should not affect, right? Or can you help to explain a bit more on this?

Thank you very much.

Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Sent: 2023年2月20日 21:59
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Well, technically, no, you’re right, it wouldn’t – not unless you start polling those queues from some other thread. We can’t prevent that from happening, so we figured the best way would be to just disallow queue starts until we’re done configuring everything. So, yes, we could relax that restriction, it’s just a matter of specifying what’s allowed and what’s not vs. just doing a blanket “no” and keeping things simple.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 1:41 PM
To: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

Thank you for the information.
I see from rte_eth_add_rx_callback, the callback is added to per queue data: rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
So it should not affect polling on other queues?


Br, Xiaoping

From: Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Sent: 2023年2月20日 18:28
To: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>; users@dpdk.org<mailto:users@dpdk.org>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi,

It is mainly because we’re install callbacks, which is not thread-safe unless the PMD is stopped. Our PMD’s internal config structures are not thread-safe. You should only start these ports after you configure everything.

From: Xiaoping Yan (NSB) <xiaoping.yan@nokia-sbell.com<mailto:xiaoping.yan@nokia-sbell.com>>
Sent: Monday, February 20, 2023 6:12 AM
To: users@dpdk.org<mailto:users@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Subject: RE: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi Anatoly

I see this multiple queue support is added by you.
Could you kindly help me to understand why rte_power_ethdev_pmgmt_queue_enable need other queues to be in stopped state?

commit 5dff9a72b0efeab02a2b71e52c4871805b7e64cb
Author: Anatoly Burakov anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>
Date:   Fri Jul 9 16:08:15 2021 +0000

power: support callbacks for multiple Rx queues


Thank you.

Br, Xiaoping

From: Xiaoping Yan (NSB)
Sent: 2023年2月16日 21:00
To: users@dpdk.org<mailto:users@dpdk.org>
Subject: why rte_power_ethdev_pmgmt_queue_enable need to check if other queues are stopped

Hi experts,

I’m trying to use dpdk power pmd management APIs in my dpdk application.
My application uses several ports, each have one rx queue, and it goes like this

1.     Init first port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the first port

2.     Init second port, setup rx queue, call rte_power_ethdev_pmgmt_queue_enable, and start the second port

3.     …
Now for the first port & queue, rte_power_ethdev_pmgmt_queue_enable return success, but for the second port & queue, it returns -16
From rte_power_ethdev_pmgmt_queue_enable code, I think it fails when checking if other queues are stopped as well.
    /* check if other queues are stopped as well */
    ret = cfg_queues_stopped(lcore_cfg);
    if (ret != 1) {
        /* error means invalid queue, 0 means queue wasn't stopped */
        ret = ret < 0 ? -EINVAL : -EBUSY;
        goto end;
    }
This seems quite strange for me, why other queues have to be in stopped state?
Can anyone help to explain?

Thank you.


Br, Xiaoping


[-- Attachment #2: Type: text/html, Size: 31372 bytes --]