[dpdk-stable] [PATCH 01/30] net/sfc: log port ID as 16-bit unsigned integer on panic

[dpdk-stable] [PATCH 01/30] net/sfc: log port ID as 16-bit unsigned integer on panic

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: stable@dpdk.org

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc_debug.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/sfc/sfc_debug.h b/drivers/net/sfc/sfc_debug.h
index 6b600ff4d..62f3937e8 100644
--- a/drivers/net/sfc/sfc_debug.h
+++ b/drivers/net/sfc/sfc_debug.h
@@ -27,7 +27,8 @@
 	do {								\
 		const struct sfc_adapter *_sa = (sa);			\
 									\
-		rte_panic("sfc " PCI_PRI_FMT " #%" PRIu8 ": " fmt "\n",	\
+		rte_panic("sfc " PCI_PRI_FMT				\
+			  " #%" PRIu16 ": " fmt "\n",			\
 			  _sa->pci_addr.domain, _sa->pci_addr.bus,	\
 			  _sa->pci_addr.devid, _sa->pci_addr.function,	\
 			  _sa->port_id, ##args);			\
-- 
2.17.1

[dpdk-stable] [PATCH 02/30] net/sfc: remove control path logging from Rx queue count

[Andrew Rybchenko]

Rx queue count may be used from data core and it should not
generate control path logs.

Fixes: 04aa6b9c5fd7 ("net/sfc: get RxQ pending descriptors count")
Cc: stable@dpdk.org

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc_ethdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index a7322a1eb..67ce5a22f 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1124,8 +1124,6 @@ sfc_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 {
 	struct sfc_adapter *sa = dev->data->dev_private;
 
-	sfc_log_init(sa, "RxQ=%u", rx_queue_id);
-
 	return sfc_rx_qdesc_npending(sa, rx_queue_id);
 }
 
-- 
2.17.1

[dpdk-stable] [PATCH 03/30] net/sfc: fix logging from secondary process

[Andrew Rybchenko]

Dynamic log type value may differ in different processes.

Fixes: f28ede500c2e ("net/sfc: support multi-process")
Cc: stable@dpdk.org

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc.c        |  6 +++---
 drivers/net/sfc/sfc.h        |  2 +-
 drivers/net/sfc/sfc_ethdev.c | 25 ++++++++++++++++---------
 drivers/net/sfc/sfc_mcdi.c   |  3 ++-
 4 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 6690053f2..0d7311d68 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -1071,8 +1071,8 @@ sfc_unprobe(struct sfc_adapter *sa)
 }
 
 uint32_t
-sfc_register_logtype(struct sfc_adapter *sa, const char *lt_prefix_str,
-		     uint32_t ll_default)
+sfc_register_logtype(const struct rte_pci_addr *pci_addr,
+		     const char *lt_prefix_str, uint32_t ll_default)
 {
 	size_t lt_prefix_str_size = strlen(lt_prefix_str);
 	size_t lt_str_size_max;
@@ -1092,7 +1092,7 @@ sfc_register_logtype(struct sfc_adapter *sa, const char *lt_prefix_str,
 
 	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
 	lt_str[lt_prefix_str_size - 1] = '.';
-	rte_pci_device_name(&sa->pci_addr, lt_str + lt_prefix_str_size,
+	rte_pci_device_name(pci_addr, lt_str + lt_prefix_str_size,
 			    lt_str_size_max - lt_prefix_str_size);
 	lt_str[lt_str_size_max - 1] = '\0';
 
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index a94ca8e75..f1cb83002 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -311,7 +311,7 @@ int sfc_dma_alloc(const struct sfc_adapter *sa, const char *name, uint16_t id,
 		  size_t len, int socket_id, efsys_mem_t *esmp);
 void sfc_dma_free(const struct sfc_adapter *sa, efsys_mem_t *esmp);
 
-uint32_t sfc_register_logtype(struct sfc_adapter *sa,
+uint32_t sfc_register_logtype(const struct rte_pci_addr *pci_addr,
 			      const char *lt_prefix_str,
 			      uint32_t ll_default);
 
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 67ce5a22f..10e032400 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1875,7 +1875,7 @@ static const struct eth_dev_ops sfc_eth_dev_secondary_ops = {
 };
 
 static int
-sfc_eth_dev_secondary_set_ops(struct rte_eth_dev *dev)
+sfc_eth_dev_secondary_set_ops(struct rte_eth_dev *dev, uint32_t logtype_main)
 {
 	/*
 	 * Device private data has really many process-local pointers.
@@ -1889,12 +1889,14 @@ sfc_eth_dev_secondary_set_ops(struct rte_eth_dev *dev)
 
 	dp_rx = sfc_dp_find_rx_by_name(&sfc_dp_head, sa->dp_rx_name);
 	if (dp_rx == NULL) {
-		sfc_err(sa, "cannot find %s Rx datapath", sa->dp_rx_name);
+		SFC_LOG(sa, RTE_LOG_ERR, logtype_main,
+			"cannot find %s Rx datapath", sa->dp_rx_name);
 		rc = ENOENT;
 		goto fail_dp_rx;
 	}
 	if (~dp_rx->features & SFC_DP_RX_FEAT_MULTI_PROCESS) {
-		sfc_err(sa, "%s Rx datapath does not support multi-process",
+		SFC_LOG(sa, RTE_LOG_ERR, logtype_main,
+			"%s Rx datapath does not support multi-process",
 			sa->dp_rx_name);
 		rc = EINVAL;
 		goto fail_dp_rx_multi_process;
@@ -1902,12 +1904,14 @@ sfc_eth_dev_secondary_set_ops(struct rte_eth_dev *dev)
 
 	dp_tx = sfc_dp_find_tx_by_name(&sfc_dp_head, sa->dp_tx_name);
 	if (dp_tx == NULL) {
-		sfc_err(sa, "cannot find %s Tx datapath", sa->dp_tx_name);
+		SFC_LOG(sa, RTE_LOG_ERR, logtype_main,
+			"cannot find %s Tx datapath", sa->dp_tx_name);
 		rc = ENOENT;
 		goto fail_dp_tx;
 	}
 	if (~dp_tx->features & SFC_DP_TX_FEAT_MULTI_PROCESS) {
-		sfc_err(sa, "%s Tx datapath does not support multi-process",
+		SFC_LOG(sa, RTE_LOG_ERR, logtype_main,
+			"%s Tx datapath does not support multi-process",
 			sa->dp_tx_name);
 		rc = EINVAL;
 		goto fail_dp_tx_multi_process;
@@ -1955,27 +1959,30 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
 {
 	struct sfc_adapter *sa = dev->data->dev_private;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	uint32_t logtype_main;
 	int rc;
 	const efx_nic_cfg_t *encp;
 	const struct ether_addr *from;
 
 	sfc_register_dp();
 
+	logtype_main = sfc_register_logtype(&pci_dev->addr,
+					    SFC_LOGTYPE_MAIN_STR,
+					    RTE_LOG_NOTICE);
+
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -sfc_eth_dev_secondary_set_ops(dev);
+		return -sfc_eth_dev_secondary_set_ops(dev, logtype_main);
 
 	/* Required for logging */
 	sa->pci_addr = pci_dev->addr;
 	sa->port_id = dev->data->port_id;
+	sa->logtype_main = logtype_main;
 
 	sa->eth_dev = dev;
 
 	/* Copy PCI device info to the dev->data */
 	rte_eth_copy_pci_info(dev, pci_dev);
 
-	sa->logtype_main = sfc_register_logtype(sa, SFC_LOGTYPE_MAIN_STR,
-						RTE_LOG_NOTICE);
-
 	rc = sfc_kvargs_parse(sa);
 	if (rc != 0)
 		goto fail_kvargs_parse;
diff --git a/drivers/net/sfc/sfc_mcdi.c b/drivers/net/sfc/sfc_mcdi.c
index 007506b4e..e485e07d6 100644
--- a/drivers/net/sfc/sfc_mcdi.c
+++ b/drivers/net/sfc/sfc_mcdi.c
@@ -256,7 +256,8 @@ sfc_mcdi_init(struct sfc_adapter *sa)
 	if (rc != 0)
 		goto fail_dma_alloc;
 
-	mcdi->logtype = sfc_register_logtype(sa, SFC_LOGTYPE_MCDI_STR,
+	mcdi->logtype = sfc_register_logtype(&sa->pci_addr,
+					     SFC_LOGTYPE_MCDI_STR,
 					     RTE_LOG_NOTICE);
 
 	emtp = &mcdi->transport;
-- 
2.17.1

Re: [dpdk-stable] [dpdk-dev] [PATCH 01/30] net/sfc: log port ID as 16-bit unsigned integer on panic

[Ferruh Yigit]

On 2/7/2019 12:17 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> Fixes: f8244c6399d9 ("ethdev: increase port id range")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/sfc/sfc_debug.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/sfc/sfc_debug.h b/drivers/net/sfc/sfc_debug.h
> index 6b600ff4d..62f3937e8 100644
> --- a/drivers/net/sfc/sfc_debug.h
> +++ b/drivers/net/sfc/sfc_debug.h
> @@ -27,7 +27,8 @@
>  	do {								\
>  		const struct sfc_adapter *_sa = (sa);			\
>  									\
> -		rte_panic("sfc " PCI_PRI_FMT " #%" PRIu8 ": " fmt "\n",	\
> +		rte_panic("sfc " PCI_PRI_FMT				\
> +			  " #%" PRIu16 ": " fmt "\n",			\
>  			  _sa->pci_addr.domain, _sa->pci_addr.bus,	\
>  			  _sa->pci_addr.devid, _sa->pci_addr.function,	\
>  			  _sa->port_id, ##args);			\
> 

Overall it may not be good idea to call rte_panic() from driver. rte_panic()
cause the DPDK application to terminate, if a driver fails application may
prefer to handle that error and continue to run, so terminate decision should be
from application not library.

Can you please double check if rte_panic() usages in the driver can be converted
to error reporting?

Re: [dpdk-stable] [dpdk-dev] [PATCH 01/30] net/sfc: log port ID as 16-bit unsigned integer on panic

[Andrew Rybchenko]

Hi Ferruh,

On 2/8/19 1:13 PM, Ferruh Yigit wrote:
> On 2/7/2019 12:17 PM, Andrew Rybchenko wrote:
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>
>> Fixes: f8244c6399d9 ("ethdev: increase port id range")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>   drivers/net/sfc/sfc_debug.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/sfc/sfc_debug.h b/drivers/net/sfc/sfc_debug.h
>> index 6b600ff4d..62f3937e8 100644
>> --- a/drivers/net/sfc/sfc_debug.h
>> +++ b/drivers/net/sfc/sfc_debug.h
>> @@ -27,7 +27,8 @@
>>   	do {								\
>>   		const struct sfc_adapter *_sa = (sa);			\
>>   									\
>> -		rte_panic("sfc " PCI_PRI_FMT " #%" PRIu8 ": " fmt "\n",	\
>> +		rte_panic("sfc " PCI_PRI_FMT				\
>> +			  " #%" PRIu16 ": " fmt "\n",			\
>>   			  _sa->pci_addr.domain, _sa->pci_addr.bus,	\
>>   			  _sa->pci_addr.devid, _sa->pci_addr.function,	\
>>   			  _sa->port_id, ##args);			\
>>
> Overall it may not be good idea to call rte_panic() from driver. rte_panic()
> cause the DPDK application to terminate, if a driver fails application may
> prefer to handle that error and continue to run, so terminate decision should be
> from application not library.
>
> Can you please double check if rte_panic() usages in the driver can be converted
> to error reporting?

Thanks a lot for reminder. We use it in few places, but even those can be
improved. We'll do it.

Andrew.