* [dpdk-dev] [PATCH] net/sfc: do not panic if alarms are not supported
@ 2017-01-19 10:44 Andrew Rybchenko
2017-01-19 11:12 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Rybchenko @ 2017-01-19 10:44 UTC (permalink / raw)
To: dev
Alarms are not supported on the FreeBSD.
Application must poll link status periodically itself using
rte_eth_link_get_nowait() to avoid managment event queue overflow.
Fixes: 2de39f4e1310 ("net/sfc: periodic management EVQ polling using alarm")
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andrew Lee <alee@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
drivers/net/sfc/sfc_ev.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/sfc/sfc_ev.c b/drivers/net/sfc/sfc_ev.c
index c788986..fe6de6f 100644
--- a/drivers/net/sfc/sfc_ev.c
+++ b/drivers/net/sfc/sfc_ev.c
@@ -499,10 +499,14 @@
rc = rte_eal_alarm_set(SFC_MGMT_EV_QPOLL_PERIOD_US,
sfc_ev_mgmt_periodic_qpoll, sa);
- if (rc != 0)
- sfc_panic(sa,
- "cannot rearm management EVQ polling alarm (rc=%d)",
- rc);
+ if (rc == -ENOTSUP) {
+ sfc_warn(sa, "alarms are not supported");
+ sfc_warn(sa, "management EVQ must be polled indirectly using no-wait link status update");
+ } else if (rc != 0) {
+ sfc_err(sa,
+ "cannot rearm management EVQ polling alarm (rc=%d)",
+ rc);
+ }
}
static void
--
1.8.2.3
^ permalink raw reply [flat|nested] 5+ messages in thread* [dpdk-dev] [PATCH v2] net/sfc: do not panic if alarms are not supported
2017-01-19 10:44 [dpdk-dev] [PATCH] net/sfc: do not panic if alarms are not supported Andrew Rybchenko
@ 2017-01-19 11:12 ` Andrew Rybchenko
2017-01-20 12:36 ` Ferruh Yigit
2017-01-23 9:53 ` Ferruh Yigit
0 siblings, 2 replies; 5+ messages in thread
From: Andrew Rybchenko @ 2017-01-19 11:12 UTC (permalink / raw)
To: dev
Alarms are not supported on the FreeBSD.
Application must poll link status periodically itself using
rte_eth_link_get_nowait() to avoid management event queue overflow.
Fixes: 2de39f4e1310 ("net/sfc: periodic management EVQ polling using alarm")
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andrew Lee <alee@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
v2:
- fix spelling
drivers/net/sfc/sfc_ev.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/sfc/sfc_ev.c b/drivers/net/sfc/sfc_ev.c
index c788986..fe6de6f 100644
--- a/drivers/net/sfc/sfc_ev.c
+++ b/drivers/net/sfc/sfc_ev.c
@@ -499,10 +499,14 @@
rc = rte_eal_alarm_set(SFC_MGMT_EV_QPOLL_PERIOD_US,
sfc_ev_mgmt_periodic_qpoll, sa);
- if (rc != 0)
- sfc_panic(sa,
- "cannot rearm management EVQ polling alarm (rc=%d)",
- rc);
+ if (rc == -ENOTSUP) {
+ sfc_warn(sa, "alarms are not supported");
+ sfc_warn(sa, "management EVQ must be polled indirectly using no-wait link status update");
+ } else if (rc != 0) {
+ sfc_err(sa,
+ "cannot rearm management EVQ polling alarm (rc=%d)",
+ rc);
+ }
}
static void
--
1.8.2.3
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH v2] net/sfc: do not panic if alarms are not supported
2017-01-19 11:12 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
@ 2017-01-20 12:36 ` Ferruh Yigit
2017-01-20 12:55 ` Andrew Rybchenko
2017-01-23 9:53 ` Ferruh Yigit
1 sibling, 1 reply; 5+ messages in thread
From: Ferruh Yigit @ 2017-01-20 12:36 UTC (permalink / raw)
To: Andrew Rybchenko, dev
On 1/19/2017 11:12 AM, Andrew Rybchenko wrote:
> Alarms are not supported on the FreeBSD.
> Application must poll link status periodically itself using
> rte_eth_link_get_nowait() to avoid management event queue overflow.
>
> Fixes: 2de39f4e1310 ("net/sfc: periodic management EVQ polling using alarm")
>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Andrew Lee <alee@solarflare.com>
> Reviewed-by: Andy Moreton <amoreton@solarflare.com>
> ---
> v2:
> - fix spelling
>
> drivers/net/sfc/sfc_ev.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/sfc/sfc_ev.c b/drivers/net/sfc/sfc_ev.c
> index c788986..fe6de6f 100644
> --- a/drivers/net/sfc/sfc_ev.c
> +++ b/drivers/net/sfc/sfc_ev.c
> @@ -499,10 +499,14 @@
>
> rc = rte_eal_alarm_set(SFC_MGMT_EV_QPOLL_PERIOD_US,
> sfc_ev_mgmt_periodic_qpoll, sa);
> - if (rc != 0)
> - sfc_panic(sa,
> - "cannot rearm management EVQ polling alarm (rc=%d)",
> - rc);
> + if (rc == -ENOTSUP) {
> + sfc_warn(sa, "alarms are not supported");
> + sfc_warn(sa, "management EVQ must be polled indirectly using no-wait link status update");
Who is the audience of this message, I am just trying to understand.
If this is for application developer, perhaps function should return and
error and log should be a debug log.
Or if it is for end user of the application, and this issue is something
that prevents app run properly, perhaps application should return error
instead of logging warning.
Overall I am a little suspicious about warn/err level of logs that does
not alter the execution path. I would like to hear more comments indeed.
> + } else if (rc != 0) {
> + sfc_err(sa,
> + "cannot rearm management EVQ polling alarm (rc=%d)",
> + rc);
> + }
> }
>
> static void
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH v2] net/sfc: do not panic if alarms are not supported
2017-01-20 12:36 ` Ferruh Yigit
@ 2017-01-20 12:55 ` Andrew Rybchenko
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Rybchenko @ 2017-01-20 12:55 UTC (permalink / raw)
To: Ferruh Yigit, dev
On 01/20/2017 03:36 PM, Ferruh Yigit wrote:
> On 1/19/2017 11:12 AM, Andrew Rybchenko wrote:
>> Alarms are not supported on the FreeBSD.
>> Application must poll link status periodically itself using
>> rte_eth_link_get_nowait() to avoid management event queue overflow.
>>
>> Fixes: 2de39f4e1310 ("net/sfc: periodic management EVQ polling using alarm")
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> Reviewed-by: Andrew Lee <alee@solarflare.com>
>> Reviewed-by: Andy Moreton <amoreton@solarflare.com>
>> ---
>> v2:
>> - fix spelling
>>
>> drivers/net/sfc/sfc_ev.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/sfc/sfc_ev.c b/drivers/net/sfc/sfc_ev.c
>> index c788986..fe6de6f 100644
>> --- a/drivers/net/sfc/sfc_ev.c
>> +++ b/drivers/net/sfc/sfc_ev.c
>> @@ -499,10 +499,14 @@
>>
>> rc = rte_eal_alarm_set(SFC_MGMT_EV_QPOLL_PERIOD_US,
>> sfc_ev_mgmt_periodic_qpoll, sa);
>> - if (rc != 0)
>> - sfc_panic(sa,
>> - "cannot rearm management EVQ polling alarm (rc=%d)",
>> - rc);
>> + if (rc == -ENOTSUP) {
>> + sfc_warn(sa, "alarms are not supported");
>> + sfc_warn(sa, "management EVQ must be polled indirectly using no-wait link status update");
> Who is the audience of this message, I am just trying to understand.
DPDK application developer
> If this is for application developer, perhaps function should return and
> error and log should be a debug log.
Unfortunately there is no way to know if application will poll the link
status with no-wait.
So, we cannot distinguish when it is OK to continue (yes alarms are not
supported, but
application is aware) and when is it not OK (alarms are not supported
and application is
unaware).
> Or if it is for end user of the application, and this issue is something
> that prevents app run properly, perhaps application should return error
> instead of logging warning.
>
> Overall I am a little suspicious about warn/err level of logs that does
> not alter the execution path. I would like to hear more comments indeed.
>
>
>> + } else if (rc != 0) {
>> + sfc_err(sa,
>> + "cannot rearm management EVQ polling alarm (rc=%d)",
>> + rc);
>> + }
>> }
>>
>> static void
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/sfc: do not panic if alarms are not supported
2017-01-19 11:12 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
2017-01-20 12:36 ` Ferruh Yigit
@ 2017-01-23 9:53 ` Ferruh Yigit
1 sibling, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2017-01-23 9:53 UTC (permalink / raw)
To: Andrew Rybchenko, dev
On 1/19/2017 11:12 AM, Andrew Rybchenko wrote:
> Alarms are not supported on the FreeBSD.
> Application must poll link status periodically itself using
> rte_eth_link_get_nowait() to avoid management event queue overflow.
>
> Fixes: 2de39f4e1310 ("net/sfc: periodic management EVQ polling using alarm")
>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Andrew Lee <alee@solarflare.com>
> Reviewed-by: Andy Moreton <amoreton@solarflare.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-23 9:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-19 10:44 [dpdk-dev] [PATCH] net/sfc: do not panic if alarms are not supported Andrew Rybchenko
2017-01-19 11:12 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
2017-01-20 12:36 ` Ferruh Yigit
2017-01-20 12:55 ` Andrew Rybchenko
2017-01-23 9:53 ` 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).