DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD
@ 2019-02-26 21:34 Stephen Hemminger
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 1/5] eal: drop unused RTE_PROC_PRIMARY_OR macros Stephen Hemminger
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Stephen Hemminger @ 2019-02-26 21:34 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Almost all usages of RTE_LOGTYPE_PMD have been replaced by per
driver log types in current version. This patch series fixes the
few remaining stragglers to use local logging.

Stephen Hemminger (5):
  eal: drop unused RTE_PROC_PRIMARY_OR macros
  crypto/virtio: use local log type
  eventdev: use same log macro for all unsupported calls
  eal: remove RTE_PMD_DEBUG_TRACE
  sfc: don't use RTE_LOGTYPE_PMD

 drivers/crypto/virtio/virtio_logs.h     |  4 +-
 drivers/net/sfc/sfc.c                   |  4 +-
 lib/librte_eal/common/include/rte_dev.h | 56 +------------------------
 lib/librte_eventdev/rte_eventdev.c      |  4 +-
 4 files changed, 9 insertions(+), 59 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 1/5] eal: drop unused RTE_PROC_PRIMARY_OR macros
  2019-02-26 21:34 [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Stephen Hemminger
@ 2019-02-26 21:34 ` Stephen Hemminger
  2019-02-28  6:05   ` Rami Rosen
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 2/5] crypto/virtio: use local log type Stephen Hemminger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2019-02-26 21:34 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

No usage in current DPDK code base.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/include/rte_dev.h | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index a9724dc9181c..5e16a4f86b70 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -76,21 +76,6 @@ rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
 #define RTE_PMD_DEBUG_TRACE(...) (void)0
 #endif
 
-/* Macros for checking for restricting functions to primary instance only */
-#define RTE_PROC_PRIMARY_OR_ERR_RET(retval) do { \
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
-		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
-		return retval; \
-	} \
-} while (0)
-
-#define RTE_PROC_PRIMARY_OR_RET() do { \
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
-		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
-		return; \
-	} \
-} while (0)
-
 /* Macros to check for invalid function pointers */
 #define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
 	if ((func) == NULL) { \
-- 
2.17.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 2/5] crypto/virtio: use local log type
  2019-02-26 21:34 [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Stephen Hemminger
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 1/5] eal: drop unused RTE_PROC_PRIMARY_OR macros Stephen Hemminger
@ 2019-02-26 21:34 ` Stephen Hemminger
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 3/5] eventdev: use same log macro for all unsupported calls Stephen Hemminger
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2019-02-26 21:34 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The virtio crypto driver was using PMD log type and it should
be using the local log type.

Fixes: 25500d4b8076 ("crypto/virtio: support device init")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/virtio/virtio_logs.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_logs.h b/drivers/crypto/virtio/virtio_logs.h
index 26a286cf81ec..1ee3819309f7 100644
--- a/drivers/crypto/virtio/virtio_logs.h
+++ b/drivers/crypto/virtio/virtio_logs.h
@@ -7,8 +7,10 @@
 
 #include <rte_log.h>
 
+extern int virtio_crypto_logtype_init;
+
 #define PMD_INIT_LOG(level, fmt, args...) \
-	rte_log(RTE_LOG_ ## level, RTE_LOGTYPE_PMD, \
+	rte_log(RTE_LOG_ ## level, virtio_crypto_logtype_init, \
 		"PMD: %s(): " fmt "\n", __func__, ##args)
 
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
-- 
2.17.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 3/5] eventdev: use same log macro for all unsupported calls
  2019-02-26 21:34 [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Stephen Hemminger
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 1/5] eal: drop unused RTE_PROC_PRIMARY_OR macros Stephen Hemminger
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 2/5] crypto/virtio: use local log type Stephen Hemminger
@ 2019-02-26 21:34 ` Stephen Hemminger
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 4/5] eal: remove RTE_PMD_DEBUG_TRACE Stephen Hemminger
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2019-02-26 21:34 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The driver already hase RTE_EDEV_XXX log macros so use
them in two more places.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eventdev/rte_eventdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index ebaf3087d925..4d4f07919554 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -892,7 +892,7 @@ rte_event_port_link(uint8_t dev_id, uint8_t port_id,
 	dev = &rte_eventdevs[dev_id];
 
 	if (*dev->dev_ops->port_link == NULL) {
-		RTE_PMD_DEBUG_TRACE("Function not supported\n");
+		RTE_EDEV_LOG_ERR("Function not supported\n");
 		rte_errno = -ENOTSUP;
 		return 0;
 	}
@@ -951,7 +951,7 @@ rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
 	dev = &rte_eventdevs[dev_id];
 
 	if (*dev->dev_ops->port_unlink == NULL) {
-		RTE_PMD_DEBUG_TRACE("Function not supported\n");
+		RTE_EDEV_LOG_ERR("Function not supported");
 		rte_errno = -ENOTSUP;
 		return 0;
 	}
-- 
2.17.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 4/5] eal: remove RTE_PMD_DEBUG_TRACE
  2019-02-26 21:34 [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Stephen Hemminger
                   ` (2 preceding siblings ...)
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 3/5] eventdev: use same log macro for all unsupported calls Stephen Hemminger
@ 2019-02-26 21:34 ` Stephen Hemminger
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD Stephen Hemminger
  2019-02-28 17:46 ` [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Ferruh Yigit
  5 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2019-02-26 21:34 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The RTE_PMD_DEBUG_TRACE was only enabled for EVENTDEV_DEBUG and
that configuration is now handled by RTE_EDEV_LOG macros.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/include/rte_dev.h | 41 ++-----------------------
 1 file changed, 2 insertions(+), 39 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 5e16a4f86b70..3cad4bce57bf 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -43,52 +43,15 @@ typedef void (*rte_dev_event_cb_fn)(const char *device_name,
 					enum rte_dev_event_type event,
 					void *cb_arg);
 
-__attribute__((format(printf, 2, 0)))
-static inline void
-rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-
-	{
-		char buffer[vsnprintf(NULL, 0, fmt, ap) + 1];
-
-		va_end(ap);
-
-		va_start(ap, fmt);
-		vsnprintf(buffer, sizeof(buffer), fmt, ap);
-		va_end(ap);
-
-		rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s",
-			func_name, buffer);
-	}
-}
-
-/*
- * Enable RTE_PMD_DEBUG_TRACE() when at least one component relying on the
- * RTE_*_RET() macros defined below is compiled in debug mode.
- */
-#if defined(RTE_LIBRTE_EVENTDEV_DEBUG)
-#define RTE_PMD_DEBUG_TRACE(...) \
-	rte_pmd_debug_trace(__func__, __VA_ARGS__)
-#else
-#define RTE_PMD_DEBUG_TRACE(...) (void)0
-#endif
-
 /* Macros to check for invalid function pointers */
 #define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
-	if ((func) == NULL) { \
-		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
+	if ((func) == NULL) \
 		return retval; \
-	} \
 } while (0)
 
 #define RTE_FUNC_PTR_OR_RET(func) do { \
-	if ((func) == NULL) { \
-		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
+	if ((func) == NULL) \
 		return; \
-	} \
 } while (0)
 
 /**
-- 
2.17.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-26 21:34 [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Stephen Hemminger
                   ` (3 preceding siblings ...)
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 4/5] eal: remove RTE_PMD_DEBUG_TRACE Stephen Hemminger
@ 2019-02-26 21:34 ` Stephen Hemminger
  2019-02-27 11:21   ` Ferruh Yigit
  2019-02-27 16:08   ` [dpdk-dev] [PATCH v2] net/sfc: do not use PMD logtype Ferruh Yigit
  2019-02-28 17:46 ` [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Ferruh Yigit
  5 siblings, 2 replies; 18+ messages in thread
From: Stephen Hemminger @ 2019-02-26 21:34 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
by local logging.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/sfc/sfc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 898603884fa0..2cd7126015fd 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
 		++lt_prefix_str_size; /* Reserve space for prefix separator */
 		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
 	} else {
-		return RTE_LOGTYPE_PMD;
+		return sfc_logtype_driver;
 	}
 
 	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
 	if (lt_str == NULL)
-		return RTE_LOGTYPE_PMD;
+		return sfc_logtype_driver;
 
 	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
 	lt_str[lt_prefix_str_size - 1] = '.';
-- 
2.17.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD Stephen Hemminger
@ 2019-02-27 11:21   ` Ferruh Yigit
  2019-02-27 11:24     ` Andrew Rybchenko
  2019-02-27 16:08   ` [dpdk-dev] [PATCH v2] net/sfc: do not use PMD logtype Ferruh Yigit
  1 sibling, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2019-02-27 11:21 UTC (permalink / raw)
  To: Stephen Hemminger, dev, Andrew Rybchenko; +Cc: Igor Romanov

On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
> by local logging.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/sfc/sfc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
> index 898603884fa0..2cd7126015fd 100644
> --- a/drivers/net/sfc/sfc.c
> +++ b/drivers/net/sfc/sfc.c
> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
>  		++lt_prefix_str_size; /* Reserve space for prefix separator */
>  		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
>  	} else {
> -		return RTE_LOGTYPE_PMD;
> +		return sfc_logtype_driver;
>  	}
>  
>  	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
>  	if (lt_str == NULL)
> -		return RTE_LOGTYPE_PMD;
> +		return sfc_logtype_driver;
>  
>  	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
>  	lt_str[lt_prefix_str_size - 1] = '.';
> 

Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few
more usage of it around same manner, as a fallback value if allocating dynamic
one fails.


Andrew,

Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD
usage? What do you think?

Thanks,
ferruh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-27 11:21   ` Ferruh Yigit
@ 2019-02-27 11:24     ` Andrew Rybchenko
  2019-02-27 11:50       ` Ferruh Yigit
  2019-02-27 18:30       ` Stephen Hemminger
  0 siblings, 2 replies; 18+ messages in thread
From: Andrew Rybchenko @ 2019-02-27 11:24 UTC (permalink / raw)
  To: Ferruh Yigit, Stephen Hemminger, dev; +Cc: Igor Romanov

On 2/27/19 2:21 PM, Ferruh Yigit wrote:
> On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
>> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
>> by local logging.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> ---
>>   drivers/net/sfc/sfc.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
>> index 898603884fa0..2cd7126015fd 100644
>> --- a/drivers/net/sfc/sfc.c
>> +++ b/drivers/net/sfc/sfc.c
>> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
>>   		++lt_prefix_str_size; /* Reserve space for prefix separator */
>>   		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
>>   	} else {
>> -		return RTE_LOGTYPE_PMD;
>> +		return sfc_logtype_driver;
>>   	}
>>   
>>   	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
>>   	if (lt_str == NULL)
>> -		return RTE_LOGTYPE_PMD;
>> +		return sfc_logtype_driver;
>>   
>>   	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
>>   	lt_str[lt_prefix_str_size - 1] = '.';
>>
> Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few
> more usage of it around same manner, as a fallback value if allocating dynamic
> one fails.
>
>
> Andrew,
>
> Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD
> usage? What do you think?
>
> Thanks,
> ferruh

I'm OK to use  sfc_logtype_driverif dynamic log type register fails, but
what should I do if sfc_logtype_driverregister fails?

Andrew.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-27 11:24     ` Andrew Rybchenko
@ 2019-02-27 11:50       ` Ferruh Yigit
  2019-02-27 12:19         ` Ferruh Yigit
  2019-02-27 18:30       ` Stephen Hemminger
  1 sibling, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2019-02-27 11:50 UTC (permalink / raw)
  To: Andrew Rybchenko, Stephen Hemminger, dev; +Cc: Igor Romanov, Olivier MATZ

On 2/27/2019 11:24 AM, Andrew Rybchenko wrote:
> On 2/27/19 2:21 PM, Ferruh Yigit wrote:
>> On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
>>> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
>>> by local logging.
>>>
>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>> ---
>>>   drivers/net/sfc/sfc.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
>>> index 898603884fa0..2cd7126015fd 100644
>>> --- a/drivers/net/sfc/sfc.c
>>> +++ b/drivers/net/sfc/sfc.c
>>> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
>>>   		++lt_prefix_str_size; /* Reserve space for prefix separator */
>>>   		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
>>>   	} else {
>>> -		return RTE_LOGTYPE_PMD;
>>> +		return sfc_logtype_driver;
>>>   	}
>>>   
>>>   	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
>>>   	if (lt_str == NULL)
>>> -		return RTE_LOGTYPE_PMD;
>>> +		return sfc_logtype_driver;
>>>   
>>>   	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
>>>   	lt_str[lt_prefix_str_size - 1] = '.';
>>>
>> Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few
>> more usage of it around same manner, as a fallback value if allocating dynamic
>> one fails.
>>
>>
>> Andrew,
>>
>> Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD
>> usage? What do you think?
>>
>> Thanks,
>> ferruh
> 
> I'm OK to use  sfc_logtype_driverif dynamic log type register fails, but
> what should I do if sfc_logtype_driverregister fails?

Right. Same concern is valid for all dynamic log registration but we are
ignoring it.

What do you think instead of each pmd/lib cover the error case,
'rte_log_register_type_and_pick_level()' & 'rte_log_register()' always return
valid value. Like if they fail internally, return 'RTE_LOGTYPE_GENERIC' kind of
value.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-27 11:50       ` Ferruh Yigit
@ 2019-02-27 12:19         ` Ferruh Yigit
  2019-02-27 13:05           ` Andrew Rybchenko
  0 siblings, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2019-02-27 12:19 UTC (permalink / raw)
  To: Andrew Rybchenko, Stephen Hemminger, dev; +Cc: Igor Romanov, Olivier MATZ

On 2/27/2019 11:50 AM, Ferruh Yigit wrote:
> On 2/27/2019 11:24 AM, Andrew Rybchenko wrote:
>> On 2/27/19 2:21 PM, Ferruh Yigit wrote:
>>> On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
>>>> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
>>>> by local logging.
>>>>
>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>> ---
>>>>   drivers/net/sfc/sfc.c | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
>>>> index 898603884fa0..2cd7126015fd 100644
>>>> --- a/drivers/net/sfc/sfc.c
>>>> +++ b/drivers/net/sfc/sfc.c
>>>> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
>>>>   		++lt_prefix_str_size; /* Reserve space for prefix separator */
>>>>   		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
>>>>   	} else {
>>>> -		return RTE_LOGTYPE_PMD;
>>>> +		return sfc_logtype_driver;
>>>>   	}
>>>>   
>>>>   	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
>>>>   	if (lt_str == NULL)
>>>> -		return RTE_LOGTYPE_PMD;
>>>> +		return sfc_logtype_driver;
>>>>   
>>>>   	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
>>>>   	lt_str[lt_prefix_str_size - 1] = '.';
>>>>
>>> Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few
>>> more usage of it around same manner, as a fallback value if allocating dynamic
>>> one fails.
>>>
>>>
>>> Andrew,
>>>
>>> Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD
>>> usage? What do you think?
>>>
>>> Thanks,
>>> ferruh
>>
>> I'm OK to use  sfc_logtype_driverif dynamic log type register fails, but
>> what should I do if sfc_logtype_driverregister fails?
> 
> Right. Same concern is valid for all dynamic log registration but we are
> ignoring it.
> 
> What do you think instead of each pmd/lib cover the error case,
> 'rte_log_register_type_and_pick_level()' & 'rte_log_register()' always return
> valid value. Like if they fail internally, return 'RTE_LOGTYPE_GENERIC' kind of
> value.

Changing the log functions is one above step, are you OK to replace all
occurrence of 'RTE_LOGTYPE_PMD' with 'sfc_logtype_driver' in
'sfc_register_logtype()', there is one more occurrence than this patch updates.

I see your point to keep the one in 'sfc_driver_register_logtype()', perhaps we
can address it later separately with above suggestion or something else.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-27 12:19         ` Ferruh Yigit
@ 2019-02-27 13:05           ` Andrew Rybchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Rybchenko @ 2019-02-27 13:05 UTC (permalink / raw)
  To: Ferruh Yigit, Stephen Hemminger, dev; +Cc: Igor Romanov, Olivier MATZ

On 2/27/19 3:19 PM, Ferruh Yigit wrote:
> On 2/27/2019 11:50 AM, Ferruh Yigit wrote:
>> On 2/27/2019 11:24 AM, Andrew Rybchenko wrote:
>>> On 2/27/19 2:21 PM, Ferruh Yigit wrote:
>>>> On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
>>>>> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
>>>>> by local logging.
>>>>>
>>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>>> ---
>>>>>    drivers/net/sfc/sfc.c | 4 ++--
>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
>>>>> index 898603884fa0..2cd7126015fd 100644
>>>>> --- a/drivers/net/sfc/sfc.c
>>>>> +++ b/drivers/net/sfc/sfc.c
>>>>> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
>>>>>    		++lt_prefix_str_size; /* Reserve space for prefix separator */
>>>>>    		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
>>>>>    	} else {
>>>>> -		return RTE_LOGTYPE_PMD;
>>>>> +		return sfc_logtype_driver;
>>>>>    	}
>>>>>    
>>>>>    	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
>>>>>    	if (lt_str == NULL)
>>>>> -		return RTE_LOGTYPE_PMD;
>>>>> +		return sfc_logtype_driver;
>>>>>    
>>>>>    	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
>>>>>    	lt_str[lt_prefix_str_size - 1] = '.';
>>>>>
>>>> Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few
>>>> more usage of it around same manner, as a fallback value if allocating dynamic
>>>> one fails.
>>>>
>>>>
>>>> Andrew,
>>>>
>>>> Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD
>>>> usage? What do you think?
>>>>
>>>> Thanks,
>>>> ferruh
>>> I'm OK to use  sfc_logtype_driverif dynamic log type register fails, but
>>> what should I do if sfc_logtype_driverregister fails?
>> Right. Same concern is valid for all dynamic log registration but we are
>> ignoring it.
>>
>> What do you think instead of each pmd/lib cover the error case,
>> 'rte_log_register_type_and_pick_level()' & 'rte_log_register()' always return
>> valid value. Like if they fail internally, return 'RTE_LOGTYPE_GENERIC' kind of
>> value.

Above makes sense.

> Changing the log functions is one above step, are you OK to replace all
> occurrence of 'RTE_LOGTYPE_PMD' with 'sfc_logtype_driver' in
> 'sfc_register_logtype()', there is one more occurrence than this patch updates.

Yes

> I see your point to keep the one in 'sfc_driver_register_logtype()', perhaps we
> can address it later separately with above suggestion or something else.

OK.

Thanks,
Andrew.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2] net/sfc: do not use PMD logtype
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD Stephen Hemminger
  2019-02-27 11:21   ` Ferruh Yigit
@ 2019-02-27 16:08   ` Ferruh Yigit
  2019-02-28 15:12     ` Andrew Rybchenko
  1 sibling, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2019-02-27 16:08 UTC (permalink / raw)
  To: dev, Andrew Rybchenko; +Cc: Stephen Hemminger

From: Stephen Hemminger <stephen@networkplumber.org>

The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
by local logging.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v2:
* Replaced all RTE_LOGTYPE_PMD instances in sfc_register_logtype()
* ternary replaced with if and multiple return because of the compiler
  error:
    error: operand of ?: changes signedness from ‘int’ to ‘uint32_t’ {aka
    ‘unsigned int’} due to unsignedness of other operand
    [-Werror=sign-compare]
---
 drivers/net/sfc/sfc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 898603884..fd4156f78 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
 		++lt_prefix_str_size; /* Reserve space for prefix separator */
 		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
 	} else {
-		return RTE_LOGTYPE_PMD;
+		return sfc_logtype_driver;
 	}
 
 	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
 	if (lt_str == NULL)
-		return RTE_LOGTYPE_PMD;
+		return sfc_logtype_driver;
 
 	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
 	lt_str[lt_prefix_str_size - 1] = '.';
@@ -1131,5 +1131,8 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
 	ret = rte_log_register_type_and_pick_level(lt_str, ll_default);
 	rte_free(lt_str);
 
-	return (ret < 0) ? RTE_LOGTYPE_PMD : ret;
+	if (ret < 0)
+		return sfc_logtype_driver;
+
+	return ret;
 }
-- 
2.20.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-27 11:24     ` Andrew Rybchenko
  2019-02-27 11:50       ` Ferruh Yigit
@ 2019-02-27 18:30       ` Stephen Hemminger
  2019-02-28  6:49         ` Andrew Rybchenko
  1 sibling, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2019-02-27 18:30 UTC (permalink / raw)
  To: Andrew Rybchenko; +Cc: Ferruh Yigit, dev, Igor Romanov

On Wed, 27 Feb 2019 14:24:21 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> On 2/27/19 2:21 PM, Ferruh Yigit wrote:
> > On 2/26/2019 9:34 PM, Stephen Hemminger wrote:  
> >> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
> >> by local logging.
> >>
> >> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >> ---
> >>   drivers/net/sfc/sfc.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
> >> index 898603884fa0..2cd7126015fd 100644
> >> --- a/drivers/net/sfc/sfc.c
> >> +++ b/drivers/net/sfc/sfc.c
> >> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
> >>   		++lt_prefix_str_size; /* Reserve space for prefix separator */
> >>   		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
> >>   	} else {
> >> -		return RTE_LOGTYPE_PMD;
> >> +		return sfc_logtype_driver;
> >>   	}
> >>   
> >>   	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
> >>   	if (lt_str == NULL)
> >> -		return RTE_LOGTYPE_PMD;
> >> +		return sfc_logtype_driver;
> >>   
> >>   	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
> >>   	lt_str[lt_prefix_str_size - 1] = '.';
> >>  
> > Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few
> > more usage of it around same manner, as a fallback value if allocating dynamic
> > one fails.
> >
> >
> > Andrew,
> >
> > Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD
> > usage? What do you think?
> >
> > Thanks,
> > ferruh  
> 
> I'm OK to use  sfc_logtype_driverif dynamic log type register fails, but
> what should I do if sfc_logtype_driverregister fails?
> 
> Andrew.

I don't like drivers that try to do something different than every other driver in DPDK.
The solarflare driver is doing lots of extra effort to have multiple fine grain log types.
Not sure if there is any value to this. In a real world situation for diagnosis you would
want to turn on logs across everything in the driver, then filter as needed later.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 1/5] eal: drop unused RTE_PROC_PRIMARY_OR macros
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 1/5] eal: drop unused RTE_PROC_PRIMARY_OR macros Stephen Hemminger
@ 2019-02-28  6:05   ` Rami Rosen
  0 siblings, 0 replies; 18+ messages in thread
From: Rami Rosen @ 2019-02-28  6:05 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

Reviewed-by: Rami Rosen <ramirose at gmail.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD
  2019-02-27 18:30       ` Stephen Hemminger
@ 2019-02-28  6:49         ` Andrew Rybchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Rybchenko @ 2019-02-28  6:49 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Ferruh Yigit, dev, Igor Romanov

On 2/27/19 9:30 PM, Stephen Hemminger wrote:
> On Wed, 27 Feb 2019 14:24:21 +0300
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>
>> On 2/27/19 2:21 PM, Ferruh Yigit wrote:
>>> On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
>>>> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
>>>> by local logging.
>>>>
>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>> ---
>>>>    drivers/net/sfc/sfc.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
>>>> index 898603884fa0..2cd7126015fd 100644
>>>> --- a/drivers/net/sfc/sfc.c
>>>> +++ b/drivers/net/sfc/sfc.c
>>>> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr,
>>>>    		++lt_prefix_str_size; /* Reserve space for prefix separator */
>>>>    		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
>>>>    	} else {
>>>> -		return RTE_LOGTYPE_PMD;
>>>> +		return sfc_logtype_driver;
>>>>    	}
>>>>    
>>>>    	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
>>>>    	if (lt_str == NULL)
>>>> -		return RTE_LOGTYPE_PMD;
>>>> +		return sfc_logtype_driver;
>>>>    
>>>>    	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
>>>>    	lt_str[lt_prefix_str_size - 1] = '.';
>>>>   
>>> Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few
>>> more usage of it around same manner, as a fallback value if allocating dynamic
>>> one fails.
>>>
>>>
>>> Andrew,
>>>
>>> Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD
>>> usage? What do you think?
>>>
>>> Thanks,
>>> ferruh
>> I'm OK to use  sfc_logtype_driverif dynamic log type register fails, but
>> what should I do if sfc_logtype_driverregister fails?
>>
>> Andrew.
> I don't like drivers that try to do something different than every other driver in DPDK.
> The solarflare driver is doing lots of extra effort to have multiple fine grain log types.
> Not sure if there is any value to this. In a real world situation for diagnosis you would
> want to turn on logs across everything in the driver, then filter as needed later.

We really use logging configuration facilities which we have in the 
driver and,
thanks to dynamic logging design (IMO really good design), both your and
our logging usage scenarios can coexist.

Andrew.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/sfc: do not use PMD logtype
  2019-02-27 16:08   ` [dpdk-dev] [PATCH v2] net/sfc: do not use PMD logtype Ferruh Yigit
@ 2019-02-28 15:12     ` Andrew Rybchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Rybchenko @ 2019-02-28 15:12 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: Stephen Hemminger

On 2/27/19 7:08 PM, Ferruh Yigit wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
>
> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded
> by local logging.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD
  2019-02-26 21:34 [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Stephen Hemminger
                   ` (4 preceding siblings ...)
  2019-02-26 21:34 ` [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD Stephen Hemminger
@ 2019-02-28 17:46 ` Ferruh Yigit
  2019-02-28 17:55   ` Ferruh Yigit
  5 siblings, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2019-02-28 17:46 UTC (permalink / raw)
  To: Stephen Hemminger, dev

On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
> Almost all usages of RTE_LOGTYPE_PMD have been replaced by per
> driver log types in current version. This patch series fixes the
> few remaining stragglers to use local logging.
> 
> Stephen Hemminger (5):
>   eal: drop unused RTE_PROC_PRIMARY_OR macros
>   crypto/virtio: use local log type
>   eventdev: use same log macro for all unsupported calls
>   eal: remove RTE_PMD_DEBUG_TRACE
>   sfc: don't use RTE_LOGTYPE_PMD

For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

(except 5/5 which has v2)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD
  2019-02-28 17:46 ` [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Ferruh Yigit
@ 2019-02-28 17:55   ` Ferruh Yigit
  0 siblings, 0 replies; 18+ messages in thread
From: Ferruh Yigit @ 2019-02-28 17:55 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Andrew Rybchenko

On 2/28/2019 5:46 PM, Ferruh Yigit wrote:
> On 2/26/2019 9:34 PM, Stephen Hemminger wrote:
>> Almost all usages of RTE_LOGTYPE_PMD have been replaced by per
>> driver log types in current version. This patch series fixes the
>> few remaining stragglers to use local logging.
>>
>> Stephen Hemminger (5):
>>   eal: drop unused RTE_PROC_PRIMARY_OR macros
>>   crypto/virtio: use local log type
>>   eventdev: use same log macro for all unsupported calls
>>   eal: remove RTE_PMD_DEBUG_TRACE
>>   sfc: don't use RTE_LOGTYPE_PMD
> 
> For series,
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> (except 5/5 which has v2)
> 

Series applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-02-28 17:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-26 21:34 [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Stephen Hemminger
2019-02-26 21:34 ` [dpdk-dev] [PATCH 1/5] eal: drop unused RTE_PROC_PRIMARY_OR macros Stephen Hemminger
2019-02-28  6:05   ` Rami Rosen
2019-02-26 21:34 ` [dpdk-dev] [PATCH 2/5] crypto/virtio: use local log type Stephen Hemminger
2019-02-26 21:34 ` [dpdk-dev] [PATCH 3/5] eventdev: use same log macro for all unsupported calls Stephen Hemminger
2019-02-26 21:34 ` [dpdk-dev] [PATCH 4/5] eal: remove RTE_PMD_DEBUG_TRACE Stephen Hemminger
2019-02-26 21:34 ` [dpdk-dev] [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD Stephen Hemminger
2019-02-27 11:21   ` Ferruh Yigit
2019-02-27 11:24     ` Andrew Rybchenko
2019-02-27 11:50       ` Ferruh Yigit
2019-02-27 12:19         ` Ferruh Yigit
2019-02-27 13:05           ` Andrew Rybchenko
2019-02-27 18:30       ` Stephen Hemminger
2019-02-28  6:49         ` Andrew Rybchenko
2019-02-27 16:08   ` [dpdk-dev] [PATCH v2] net/sfc: do not use PMD logtype Ferruh Yigit
2019-02-28 15:12     ` Andrew Rybchenko
2019-02-28 17:46 ` [dpdk-dev] [PATCH 0/5] no longer use RTE_LOGTYPE_PMD Ferruh Yigit
2019-02-28 17:55   ` 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).