* [dpdk-dev] [PATCH 01/13] crypto: replace rte_panic instances in crypto driver
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 13:10 ` Bruce Richardson
2018-04-04 11:27 ` [dpdk-dev] [PATCH 02/13] bond: replace rte_panic instances in bonding driver Arnon Warshavsky
` (11 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +++++---
drivers/crypto/dpaa_sec/dpaa_sec.c | 8 +++++---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index 784b96d..37a5727 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -2861,9 +2861,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for "
+ "private device data", __func__);
+ return -1;
+ }
}
dpaa2_dev->cryptodev = cryptodev;
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c b/drivers/crypto/dpaa_sec/dpaa_sec.c
index c5191ce..3b22d8a 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -2374,9 +2374,11 @@ struct rte_security_ops dpaa_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for private "
+ "device data", __func__);
+ return -1;
+ }
}
dpaa_dev->crypto_dev = cryptodev;
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [dpdk-dev] [PATCH 01/13] crypto: replace rte_panic instances in crypto driver
2018-04-04 11:27 ` [dpdk-dev] [PATCH 01/13] crypto: replace rte_panic instances in crypto driver Arnon Warshavsky
@ 2018-04-04 13:10 ` Bruce Richardson
2018-04-04 20:43 ` Arnon Warshavsky
0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2018-04-04 13:10 UTC (permalink / raw)
To: Arnon Warshavsky
Cc: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
ferruh.yigit, dev
On Wed, Apr 04, 2018 at 02:27:25PM +0300, Arnon Warshavsky wrote:
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
> drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +++++---
> drivers/crypto/dpaa_sec/dpaa_sec.c | 8 +++++---
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> index 784b96d..37a5727 100644
> --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> @@ -2861,9 +2861,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
> RTE_CACHE_LINE_SIZE,
> rte_socket_id());
>
> - if (cryptodev->data->dev_private == NULL)
> - rte_panic("Cannot allocate memzone for private "
> - "device data");
> + if (cryptodev->data->dev_private == NULL) {
> + RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for "
> + "private device data", __func__);
> + return -1;
> + }
> }
This is the right change to make, but can you please avoid wrapping literal
strings across lines. It means that the error message cannot be grepped.
Just put the __func__ on the second line.
Checkpatch and similar tools should ignore long lines if the long line is
simply caused by a literal string.
/Bruce
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [dpdk-dev] [PATCH 01/13] crypto: replace rte_panic instances in crypto driver
2018-04-04 13:10 ` Bruce Richardson
@ 2018-04-04 20:43 ` Arnon Warshavsky
0 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 20:43 UTC (permalink / raw)
To: Bruce Richardson
Cc: Thomas Monjalon, Burakov, Anatoly, wenzhuo.lu, declan.doherty,
jerin.jacob, ferruh.yigit, dev
Cool.I assumed worst than needed when the checkpatches lady was not
satisfied.
Will fix that.
On Wed, Apr 4, 2018 at 4:10 PM, Bruce Richardson <bruce.richardson@intel.com
> wrote:
> On Wed, Apr 04, 2018 at 02:27:25PM +0300, Arnon Warshavsky wrote:
> > Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> > ---
> > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +++++---
> > drivers/crypto/dpaa_sec/dpaa_sec.c | 8 +++++---
> > 2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > index 784b96d..37a5727 100644
> > --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > @@ -2861,9 +2861,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
> > RTE_CACHE_LINE_SIZE,
> > rte_socket_id());
> >
> > - if (cryptodev->data->dev_private == NULL)
> > - rte_panic("Cannot allocate memzone for private "
> > - "device data");
> > + if (cryptodev->data->dev_private == NULL) {
> > + RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone
> for "
> > + "private device data", __func__);
> > + return -1;
> > + }
> > }
> This is the right change to make, but can you please avoid wrapping literal
> strings across lines. It means that the error message cannot be grepped.
> Just put the __func__ on the second line.
>
> Checkpatch and similar tools should ignore long lines if the long line is
> simply caused by a literal string.
>
> /Bruce
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon@qwilt.com
<arnon@qwilt.com>*
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [PATCH 02/13] bond: replace rte_panic instances in bonding driver
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 01/13] crypto: replace rte_panic instances in crypto driver Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 03/13] e1000: replace rte_panic instances in e1000 driver Arnon Warshavsky
` (10 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Local functions to this file,
changing from void to int are non-abi-breaking
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 30 +++++++++++++++--------
drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +-
drivers/net/bonding/rte_eth_bond_api.c | 20 ++++++++++-----
drivers/net/bonding/rte_eth_bond_pmd.c | 10 +++++---
drivers/net/bonding/rte_eth_bond_private.h | 2 +-
5 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index c452318..310118c 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -893,7 +893,7 @@
bond_mode_8023ad_periodic_cb, arg);
}
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,
uint16_t slave_id)
{
@@ -939,7 +939,7 @@
timer_cancel(&port->warning_timer);
if (port->mbuf_pool != NULL)
- return;
+ return 0;
RTE_ASSERT(port->rx_ring == NULL);
RTE_ASSERT(port->tx_ring == NULL);
@@ -968,8 +968,10 @@
/* Any memory allocation failure in initialization is critical because
* resources can't be free, so reinitialization is impossible. */
if (port->mbuf_pool == NULL) {
- rte_panic("Slave %u: Failed to create memory pool '%s': %s\n",
- slave_id, mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create memory"
+ " pool '%s': %s\n", __func__,
+ slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
snprintf(mem_name, RTE_DIM(mem_name), "slave_%u_rx", slave_id);
@@ -977,8 +979,9 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_RX_PKTS), socket_id, 0);
if (port->rx_ring == NULL) {
- rte_panic("Slave %u: Failed to create rx ring '%s': %s\n", slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create rx ring '%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
/* TX ring is at least one pkt longer to make room for marker packet. */
@@ -987,9 +990,13 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_TX_PKTS + 1), socket_id, 0);
if (port->tx_ring == NULL) {
- rte_panic("Slave %u: Failed to create tx ring '%s': %s\n", slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Fail to create tx ring "
+ "'%s': %s\n", __func__,
+ slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
+
+ return 0;
}
int
@@ -1143,9 +1150,12 @@
struct bond_dev_private *internals = bond_dev->data->dev_private;
uint8_t i;
- for (i = 0; i < internals->active_slave_count; i++)
- bond_mode_8023ad_activate_slave(bond_dev,
+ for (i = 0; i < internals->active_slave_count; i++) {
+ int rc = bond_mode_8023ad_activate_slave(bond_dev,
internals->active_slaves[i]);
+ if (rc != 0)
+ return rc;
+ }
return 0;
}
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
index 0f490a5..96a42f2 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
+++ b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
@@ -263,7 +263,7 @@ struct mode8023ad_private {
* @return
* 0 on success, negative value otherwise.
*/
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *dev, uint16_t port_id);
/**
diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index f854b73..6bc5887 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -69,14 +69,15 @@
return 0;
}
-void
+int
activate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id)
{
struct bond_dev_private *internals = eth_dev->data->dev_private;
uint8_t active_count = internals->active_slave_count;
if (internals->mode == BONDING_MODE_8023AD)
- bond_mode_8023ad_activate_slave(eth_dev, port_id);
+ if (bond_mode_8023ad_activate_slave(eth_dev, port_id) != 0)
+ return -1;
if (internals->mode == BONDING_MODE_TLB
|| internals->mode == BONDING_MODE_ALB) {
@@ -349,10 +350,17 @@
bond_ethdev_primary_set(internals,
slave_port_id);
- if (find_slave_by_id(internals->active_slaves,
- internals->active_slave_count,
- slave_port_id) == internals->active_slave_count)
- activate_slave(bonded_eth_dev, slave_port_id);
+ int rc =
+ find_slave_by_id(internals->active_slaves,
+ internals->active_slave_count,
+ slave_port_id);
+
+ if (rc == internals->active_slave_count) {
+ int rc = activate_slave(bonded_eth_dev,
+ slave_port_id);
+ if (rc != 0)
+ return -1;
+ }
}
}
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b59ba9f..96f8b1a 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1740,8 +1740,11 @@ struct bwg_slave {
/* Any memory allocation failure in initialization is critical because
* resources can't be free, so reinitialization is impossible. */
if (port->slow_pool == NULL) {
- rte_panic("Slave %u: Failed to create memory pool '%s': %s\n",
- slave_id, mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create"
+ " memory pool '%s': %s\n",
+ __func__, slave_id, mem_name,
+ rte_strerror(rte_errno));
+ return -1;
}
}
@@ -2660,7 +2663,8 @@ struct bwg_slave {
mac_address_slaves_update(bonded_eth_dev);
}
- activate_slave(bonded_eth_dev, port_id);
+ if (activate_slave(bonded_eth_dev, port_id) != 0)
+ return -1;
/* If user has defined the primary port then default to using it */
if (internals->user_defined_primary_port &&
diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h
index 92e15f8..65453aa 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -185,7 +185,7 @@ struct bond_dev_private {
void
deactivate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id);
-void
+int
activate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id);
void
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 03/13] e1000: replace rte_panic instances in e1000 driver
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 01/13] crypto: replace rte_panic instances in crypto driver Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 02/13] bond: replace rte_panic instances in bonding driver Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 04/13] ixgbe: replace rte_panic instances in ixgbe driver Arnon Warshavsky
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
drivers/net/e1000/e1000_ethdev.h | 2 +-
drivers/net/e1000/igb_ethdev.c | 3 ++-
drivers/net/e1000/igb_pf.c | 15 +++++++++------
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 23b089c..a66ff42 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -405,7 +405,7 @@ int eth_igb_rss_hash_conf_get(struct rte_eth_dev *dev,
/*
* misc function prototypes
*/
-void igb_pf_host_init(struct rte_eth_dev *eth_dev);
+int igb_pf_host_init(struct rte_eth_dev *eth_dev);
void igb_pf_mbx_process(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index d7eef9a..994bb5a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -833,7 +833,8 @@ static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
}
/* initialize PF if max_vfs not zero */
- igb_pf_host_init(eth_dev);
+ if (igb_pf_host_init(eth_dev) != 0)
+ goto err_late;
ctrl_ext = E1000_READ_REG(hw, E1000_CTRL_EXT);
/* Set PF Reset Done bit so PF/VF Mail Ops can work */
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index b9f2e53..dfa63c9 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -63,7 +63,7 @@ int igb_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t vf_num)
return 0;
}
-void igb_pf_host_init(struct rte_eth_dev *eth_dev)
+int igb_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct e1000_vf_info **vfinfo =
E1000_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -74,7 +74,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
if (0 == (vf_num = dev_num_vf(eth_dev)))
- return;
+ return 0;
if (hw->mac.type == e1000_i350)
nb_queue = 1;
@@ -82,11 +82,14 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct e1000_vf_info) * vf_num, 0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ RTE_LOG(CRIT, PMD, "%s(): Cannot allocate memory for private "
+ "VF data\n", __func__);
+ return -1;
+ }
RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_8_POOLS;
RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = nb_queue;
@@ -98,7 +101,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
igb_mb_intr_setup(eth_dev);
- return;
+ return 0;
}
void igb_pf_host_uninit(struct rte_eth_dev *dev)
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 04/13] ixgbe: replace rte_panic instances in ixgbe driver
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (2 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 03/13] e1000: replace rte_panic instances in e1000 driver Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 05/13] eal: replace rte_panic instances in eventdev Arnon Warshavsky
` (8 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
drivers/net/ixgbe/ixgbe_ethdev.h | 2 +-
drivers/net/ixgbe/ixgbe_pf.c | 13 +++++++++----
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 4df5c75..96188dc 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1224,7 +1224,8 @@ struct rte_ixgbe_xstats_name_off {
memset(hwstrip, 0, sizeof(*hwstrip));
/* initialize PF if max_vfs not zero */
- ixgbe_pf_host_init(eth_dev);
+ if (ixgbe_pf_host_init(eth_dev) != 0)
+ return -1;
ctrl_ext = IXGBE_READ_REG(hw, IXGBE_CTRL_EXT);
/* let hardware know driver is loaded */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index c56d652..82d7fd2 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -663,7 +663,7 @@ int ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
void ixgbe_vlan_hw_strip_disable_all(struct rte_eth_dev *dev);
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index ea99737..5c25de0 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -66,7 +66,7 @@ int ixgbe_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t vf_num)
return 0;
}
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct ixgbe_vf_info **vfinfo =
IXGBE_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -84,11 +84,14 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct ixgbe_vf_info) * vf_num, 0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memory for private VF data\n",
+ __func__);
+ return -1;
+ }
memset(mirror_info, 0, sizeof(struct ixgbe_mirror_info));
memset(uta_info, 0, sizeof(struct ixgbe_uta_info));
@@ -116,6 +119,8 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
ixgbe_mb_intr_setup(eth_dev);
+
+ return 0;
}
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 05/13] eal: replace rte_panic instances in eventdev
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (3 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 04/13] ixgbe: replace rte_panic instances in ixgbe driver Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 06/13] kni: replace rte_panic instances in kni Arnon Warshavsky
` (7 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_eventdev/rte_eventdev_pmd_pci.h | 8 +++++---
lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +++++---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
index 8fb6138..a3f70f7 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
@@ -66,9 +66,11 @@
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for private "
+ "device data", __func__);
+ return -1;
+ }
}
eventdev->dev = &pci_dev->device;
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
index 8c64a06..bf65420 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
@@ -61,9 +61,11 @@
RTE_CACHE_LINE_SIZE,
socket_id);
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private device"
- " data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for private "
+ "device data", __func__);
+ return NULL;
+ }
}
return eventdev;
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 06/13] kni: replace rte_panic instances in kni
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (4 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 05/13] eal: replace rte_panic instances in eventdev Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 07/13] eal: replace rte_panic instances in rte_malloc Arnon Warshavsky
` (6 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_kni/rte_kni.c | 18 ++++++++++++------
lib/librte_kni/rte_kni_fifo.h | 11 ++++++++---
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 2867411..54050c8 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -353,37 +353,43 @@ struct rte_kni *
/* TX RING */
mz = slot->m_tx_q;
ctx->tx_q = mz->addr;
- kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.tx_phys = mz->phys_addr;
/* RX RING */
mz = slot->m_rx_q;
ctx->rx_q = mz->addr;
- kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.rx_phys = mz->phys_addr;
/* ALLOC RING */
mz = slot->m_alloc_q;
ctx->alloc_q = mz->addr;
- kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.alloc_phys = mz->phys_addr;
/* FREE RING */
mz = slot->m_free_q;
ctx->free_q = mz->addr;
- kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.free_phys = mz->phys_addr;
/* Request RING */
mz = slot->m_req_q;
ctx->req_q = mz->addr;
- kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.req_phys = mz->phys_addr;
/* Response RING */
mz = slot->m_resp_q;
ctx->resp_q = mz->addr;
- kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.resp_phys = mz->phys_addr;
/* Req/Resp sync mem area */
diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
index ac26a8c..5052015 100644
--- a/lib/librte_kni/rte_kni_fifo.h
+++ b/lib/librte_kni/rte_kni_fifo.h
@@ -7,17 +7,22 @@
/**
* Initializes the kni fifo structure
*/
-static void
+static int
kni_fifo_init(struct rte_kni_fifo *fifo, unsigned size)
{
/* Ensure size is power of 2 */
- if (size & (size - 1))
- rte_panic("KNI fifo size must be power of 2\n");
+ if (size & (size - 1)) {
+ RTE_LOG(CRIT, EAL, "%s(): KNI fifo size must be power of 2\n",
+ __func__);
+ return -1;
+ }
fifo->write = 0;
fifo->read = 0;
fifo->len = size;
fifo->elem_size = sizeof(void *);
+
+ return 0;
}
/**
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 07/13] eal: replace rte_panic instances in rte_malloc
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (5 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 06/13] kni: replace rte_panic instances in kni Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:56 ` Burakov, Anatoly
2018-04-04 11:27 ` [dpdk-dev] [PATCH 08/13] eal: replace rte_panic instances in hugepage_info Arnon Warshavsky
` (5 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_eal/common/rte_malloc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
index e0e0d0b..34d438a 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -134,8 +134,11 @@ void rte_free(void *addr)
return rte_malloc(NULL, size, align);
struct malloc_elem *elem = malloc_elem_from_data(ptr);
- if (elem == NULL)
- rte_panic("Fatal error: memory corruption detected\n");
+ if (elem == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Fatal error: memory corruption detected\n",
+ __func__);
+ return NULL;
+ }
size = RTE_CACHE_LINE_ROUNDUP(size), align = RTE_CACHE_LINE_ROUNDUP(align);
/* check alignment matches first, and if ok, see if we can resize block */
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [dpdk-dev] [PATCH 07/13] eal: replace rte_panic instances in rte_malloc
2018-04-04 11:27 ` [dpdk-dev] [PATCH 07/13] eal: replace rte_panic instances in rte_malloc Arnon Warshavsky
@ 2018-04-04 11:56 ` Burakov, Anatoly
0 siblings, 0 replies; 21+ messages in thread
From: Burakov, Anatoly @ 2018-04-04 11:56 UTC (permalink / raw)
To: Arnon Warshavsky, thomas, wenzhuo.lu, declan.doherty,
jerin.jacob, bruce.richardson, ferruh.yigit
Cc: dev
On 04-Apr-18 12:27 PM, Arnon Warshavsky wrote:
> replace panic calls with log and retrun value.
>
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [PATCH 08/13] eal: replace rte_panic instances in hugepage_info
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (6 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 07/13] eal: replace rte_panic instances in rte_malloc Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:57 ` Burakov, Anatoly
2018-04-04 11:27 ` [dpdk-dev] [PATCH 09/13] eal: replace rte_panic instances in common_memzone Arnon Warshavsky
` (4 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index 8bbf771..43af5b5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -80,8 +80,11 @@
unsigned long long size = 0;
FILE *fd = fopen(proc_meminfo, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_meminfo);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_meminfo);
+ return 0;
+ }
while(fgets(buffer, sizeof(buffer), fd)){
if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
size = rte_str_to_size(&buffer[hugepagesz_len]);
@@ -89,8 +92,11 @@
}
}
fclose(fd);
- if (size == 0)
- rte_panic("Cannot get default hugepage size from %s\n", proc_meminfo);
+ if (size == 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size from %s\n",
+ __func__, proc_meminfo);
+ return 0;
+ }
return size;
}
@@ -116,8 +122,11 @@
char *retval = NULL;
FILE *fd = fopen(proc_mounts, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_mounts);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_mounts);
+ return NULL;
+ }
if (default_size == 0)
default_size = get_default_hp_size();
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [dpdk-dev] [PATCH 08/13] eal: replace rte_panic instances in hugepage_info
2018-04-04 11:27 ` [dpdk-dev] [PATCH 08/13] eal: replace rte_panic instances in hugepage_info Arnon Warshavsky
@ 2018-04-04 11:57 ` Burakov, Anatoly
0 siblings, 0 replies; 21+ messages in thread
From: Burakov, Anatoly @ 2018-04-04 11:57 UTC (permalink / raw)
To: Arnon Warshavsky, thomas, wenzhuo.lu, declan.doherty,
jerin.jacob, bruce.richardson, ferruh.yigit
Cc: dev
On 04-Apr-18 12:27 PM, Arnon Warshavsky wrote:
> replace panic calls with log and retrun value.
>
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
Commit message is misspelled, otherwise
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [PATCH 09/13] eal: replace rte_panic instances in common_memzone
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (7 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 08/13] eal: replace rte_panic instances in hugepage_info Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 12:09 ` Burakov, Anatoly
2018-04-04 11:27 ` [dpdk-dev] [PATCH 10/13] eal: replace rte_panic instances in interrupts thread Arnon Warshavsky
` (3 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_eal/common/eal_common_memzone.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
index 1ab3ade..fa0a407 100644
--- a/lib/librte_eal/common/eal_common_memzone.c
+++ b/lib/librte_eal/common/eal_common_memzone.c
@@ -314,8 +314,9 @@
if (addr == NULL)
ret = -EINVAL;
else if (mcfg->memzone_cnt == 0) {
- rte_panic("%s(): memzone address not NULL but memzone_cnt is 0!\n",
- __func__);
+ RTE_LOG(CRIT, EAL, "%s(): memzone address not NULL but memzone_cnt"
+ " is 0!\n", __func__);
+ return -1;
} else {
memset(&mcfg->memzone[idx], 0, sizeof(mcfg->memzone[idx]));
mcfg->memzone_cnt--;
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [dpdk-dev] [PATCH 09/13] eal: replace rte_panic instances in common_memzone
2018-04-04 11:27 ` [dpdk-dev] [PATCH 09/13] eal: replace rte_panic instances in common_memzone Arnon Warshavsky
@ 2018-04-04 12:09 ` Burakov, Anatoly
2018-04-04 12:34 ` Thomas Monjalon
0 siblings, 1 reply; 21+ messages in thread
From: Burakov, Anatoly @ 2018-04-04 12:09 UTC (permalink / raw)
To: Arnon Warshavsky, thomas, wenzhuo.lu, declan.doherty,
jerin.jacob, bruce.richardson, ferruh.yigit
Cc: dev
On 04-Apr-18 12:27 PM, Arnon Warshavsky wrote:
> replace panic calls with log and retrun value.
>
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
> lib/librte_eal/common/eal_common_memzone.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
> index 1ab3ade..fa0a407 100644
> --- a/lib/librte_eal/common/eal_common_memzone.c
> +++ b/lib/librte_eal/common/eal_common_memzone.c
> @@ -314,8 +314,9 @@
> if (addr == NULL)
> ret = -EINVAL;
> else if (mcfg->memzone_cnt == 0) {
> - rte_panic("%s(): memzone address not NULL but memzone_cnt is 0!\n",
> - __func__);
> + RTE_LOG(CRIT, EAL, "%s(): memzone address not NULL but memzone_cnt"
> + " is 0!\n", __func__);
> + return -1;
> } else {
> memset(&mcfg->memzone[idx], 0, sizeof(mcfg->memzone[idx]));
> mcfg->memzone_cnt--;
>
This changes public API. For now, memzone docs mention either return
value of 0, or return value of -EINVAL in case of invalid arguments:
/**
* Free a memzone.
*
* @param mz
* A pointer to the memzone
* @return
* -EINVAL - invalid parameter.
* 0 - success
*/
I'm not sure returning -EINVAL is suitable in this case (the parameter
was valid, but an internal error happened - I can't think of any
suitable errno value off hand), and adding a new return value changes
API, which presumably would require a deprecation notice.
Thomas?
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [dpdk-dev] [PATCH 09/13] eal: replace rte_panic instances in common_memzone
2018-04-04 12:09 ` Burakov, Anatoly
@ 2018-04-04 12:34 ` Thomas Monjalon
2018-04-04 20:44 ` Arnon Warshavsky
0 siblings, 1 reply; 21+ messages in thread
From: Thomas Monjalon @ 2018-04-04 12:34 UTC (permalink / raw)
To: Burakov, Anatoly, Arnon Warshavsky
Cc: wenzhuo.lu, declan.doherty, jerin.jacob, bruce.richardson,
ferruh.yigit, dev
04/04/2018 14:09, Burakov, Anatoly:
> On 04-Apr-18 12:27 PM, Arnon Warshavsky wrote:
> > replace panic calls with log and retrun value.
> >
> > Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> > ---
> > lib/librte_eal/common/eal_common_memzone.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
> > index 1ab3ade..fa0a407 100644
> > --- a/lib/librte_eal/common/eal_common_memzone.c
> > +++ b/lib/librte_eal/common/eal_common_memzone.c
> > @@ -314,8 +314,9 @@
> > if (addr == NULL)
> > ret = -EINVAL;
> > else if (mcfg->memzone_cnt == 0) {
> > - rte_panic("%s(): memzone address not NULL but memzone_cnt is 0!\n",
> > - __func__);
> > + RTE_LOG(CRIT, EAL, "%s(): memzone address not NULL but memzone_cnt"
> > + " is 0!\n", __func__);
> > + return -1;
> > } else {
> > memset(&mcfg->memzone[idx], 0, sizeof(mcfg->memzone[idx]));
> > mcfg->memzone_cnt--;
> >
>
> This changes public API. For now, memzone docs mention either return
> value of 0, or return value of -EINVAL in case of invalid arguments:
>
> /**
> * Free a memzone.
> *
> * @param mz
> * A pointer to the memzone
> * @return
> * -EINVAL - invalid parameter.
> * 0 - success
> */
>
> I'm not sure returning -EINVAL is suitable in this case (the parameter
> was valid, but an internal error happened - I can't think of any
> suitable errno value off hand), and adding a new return value changes
> API, which presumably would require a deprecation notice.
>
> Thomas?
It does not fully change the API, since the success value is not changed.
I think we can accept one more error value if doxygen is properly updated.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [dpdk-dev] [PATCH 09/13] eal: replace rte_panic instances in common_memzone
2018-04-04 12:34 ` Thomas Monjalon
@ 2018-04-04 20:44 ` Arnon Warshavsky
0 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 20:44 UTC (permalink / raw)
To: Thomas Monjalon
Cc: Burakov, Anatoly, wenzhuo.lu, declan.doherty, jerin.jacob,
Bruce Richardson, ferruh.yigit, dev
Thanks. Will update the doxigen
On Wed, Apr 4, 2018 at 3:34 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 04/04/2018 14:09, Burakov, Anatoly:
> > On 04-Apr-18 12:27 PM, Arnon Warshavsky wrote:
> > > replace panic calls with log and retrun value.
> > >
> > > Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> > > ---
> > > lib/librte_eal/common/eal_common_memzone.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/librte_eal/common/eal_common_memzone.c
> b/lib/librte_eal/common/eal_common_memzone.c
> > > index 1ab3ade..fa0a407 100644
> > > --- a/lib/librte_eal/common/eal_common_memzone.c
> > > +++ b/lib/librte_eal/common/eal_common_memzone.c
> > > @@ -314,8 +314,9 @@
> > > if (addr == NULL)
> > > ret = -EINVAL;
> > > else if (mcfg->memzone_cnt == 0) {
> > > - rte_panic("%s(): memzone address not NULL but memzone_cnt
> is 0!\n",
> > > - __func__);
> > > + RTE_LOG(CRIT, EAL, "%s(): memzone address not NULL but
> memzone_cnt"
> > > + " is 0!\n", __func__);
> > > + return -1;
> > > } else {
> > > memset(&mcfg->memzone[idx], 0, sizeof(mcfg->memzone[idx]));
> > > mcfg->memzone_cnt--;
> > >
> >
> > This changes public API. For now, memzone docs mention either return
> > value of 0, or return value of -EINVAL in case of invalid arguments:
> >
> > /**
> > * Free a memzone.
> > *
> > * @param mz
> > * A pointer to the memzone
> > * @return
> > * -EINVAL - invalid parameter.
> > * 0 - success
> > */
> >
> > I'm not sure returning -EINVAL is suitable in this case (the parameter
> > was valid, but an internal error happened - I can't think of any
> > suitable errno value off hand), and adding a new return value changes
> > API, which presumably would require a deprecation notice.
> >
> > Thomas?
>
> It does not fully change the API, since the success value is not changed.
> I think we can accept one more error value if doxygen is properly updated.
>
>
>
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon@qwilt.com
<arnon@qwilt.com>*
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [PATCH 10/13] eal: replace rte_panic instances in interrupts thread
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (8 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 09/13] eal: replace rte_panic instances in common_memzone Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 11/13] eal: replace rte_panic instances in ethdev Arnon Warshavsky
` (2 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
replace panic calls with log and retrun value.
Thread function removes the noretrun attribute.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_eal/linuxapp/eal/eal_interrupts.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index f86f22f..7b1f530 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -776,7 +776,7 @@ struct rte_intr_source {
* @return
* never return;
*/
-static __attribute__((noreturn)) void *
+static void *
eal_intr_thread_main(__rte_unused void *arg)
{
struct epoll_event ev;
@@ -794,8 +794,11 @@ static __attribute__((noreturn)) void *
/* create epoll fd */
int pfd = epoll_create(1);
- if (pfd < 0)
- rte_panic("Cannot create epoll instance\n");
+ if (pfd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create epoll instance\n",
+ __func__);
+ return NULL;
+ }
pipe_event.data.fd = intr_pipe.readfd;
/**
@@ -804,8 +807,11 @@ static __attribute__((noreturn)) void *
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD, intr_pipe.readfd,
&pipe_event) < 0) {
- rte_panic("Error adding fd to %d epoll_ctl, %s\n",
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd to %d "
+ "epoll_ctl, %s\n",
+ __func__,
intr_pipe.readfd, strerror(errno));
+ return NULL;
}
numfds++;
@@ -822,9 +828,14 @@ static __attribute__((noreturn)) void *
* into wait list.
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD,
- src->intr_handle.fd, &ev) < 0){
- rte_panic("Error adding fd %d epoll_ctl, %s\n",
- src->intr_handle.fd, strerror(errno));
+ src->intr_handle.fd, &ev) < 0) {
+ RTE_LOG(CRIT, EAL,
+ "%s(): Error adding fd %d "
+ "epoll_ctl, %s\n",
+ __func__,
+ src->intr_handle.fd,
+ strerror(errno));
+ return NULL;
}
else
numfds++;
@@ -839,6 +850,8 @@ static __attribute__((noreturn)) void *
*/
close(pfd);
}
+
+ return NULL;
}
int
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 11/13] eal: replace rte_panic instances in ethdev
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (9 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 10/13] eal: replace rte_panic instances in interrupts thread Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 12/13] eal: replace rte_panic instances in init sequence Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 13/13] devtools: prevent new instances of rte_panic and rte_exit Arnon Warshavsky
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_ether/rte_ethdev.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 2c74f7e..57e1e6b 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -194,7 +194,7 @@ enum {
return port_id;
}
-static void
+static int
rte_eth_dev_shared_data_prepare(void)
{
const unsigned flags = 0;
@@ -210,8 +210,12 @@ enum {
rte_socket_id(), flags);
} else
mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
- if (mz == NULL)
- rte_panic("Cannot allocate ethdev shared data\n");
+ if (mz == NULL) {
+ rte_spinlock_unlock(&rte_eth_shared_data_lock);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate ethdev shared data\n",
+ __func__);
+ return -1;
+ }
rte_eth_dev_shared_data = mz->addr;
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
@@ -224,6 +228,8 @@ enum {
}
rte_spinlock_unlock(&rte_eth_shared_data_lock);
+
+ return 0;
}
struct rte_eth_dev *
@@ -274,7 +280,8 @@ struct rte_eth_dev *
uint16_t port_id;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port creation between primary and secondary threads. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -317,7 +324,8 @@ struct rte_eth_dev *
uint16_t i;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port attachment to primary port creation and release. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -345,7 +353,8 @@ struct rte_eth_dev *
if (eth_dev == NULL)
return -EINVAL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -399,7 +408,8 @@ struct rte_eth_dev *
int __rte_experimental
rte_eth_dev_owner_new(uint64_t *owner_id)
{
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -450,7 +460,8 @@ struct rte_eth_dev *
{
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -467,7 +478,8 @@ struct rte_eth_dev *
{.id = RTE_ETH_DEV_NO_OWNER, .name = ""};
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -482,7 +494,8 @@ struct rte_eth_dev *
{
uint16_t port_id;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -502,7 +515,8 @@ struct rte_eth_dev *
{
int ret = 0;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 12/13] eal: replace rte_panic instances in init sequence
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (10 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 11/13] eal: replace rte_panic instances in ethdev Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
2018-04-04 11:27 ` [dpdk-dev] [PATCH 13/13] devtools: prevent new instances of rte_panic and rte_exit Arnon Warshavsky
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
Local functions to this file,
changing from void to int are non-abi-breaking.
For handling the single function that cannot
change from void to int due to abi,
where this is the only place it is called in,
I added a state variable that is being checked
right after the call to this function.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_eal/bsdapp/eal/eal.c | 87 ++++++++++++++-------
lib/librte_eal/bsdapp/eal/eal_thread.c | 65 +++++++++++-----
lib/librte_eal/common/eal_common_launch.c | 21 ++++++
lib/librte_eal/common/include/rte_debug.h | 12 +++
lib/librte_eal/linuxapp/eal/eal.c | 121 ++++++++++++++++++++----------
lib/librte_eal/linuxapp/eal/eal_thread.c | 65 +++++++++++-----
6 files changed, 272 insertions(+), 99 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 4eafcb5..f6aa3b2 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -150,7 +150,7 @@ enum rte_iova_mode
* We also don't lock the whole file, so that in future we can use read-locks
* on other parts, e.g. memzones, to detect if there are running secondary
* processes. */
-static void
+static int
rte_eal_config_create(void)
{
void *rte_mem_cfg_addr;
@@ -159,60 +159,79 @@ enum rte_iova_mode
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n", pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
if (retval < 0){
close(mem_cfg_fd);
- rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot resize '%s' for rte_mem_config\n",
+ __func__, pathname);
+ return -1;
}
retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
if (retval < 0){
close(mem_cfg_fd);
- rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another primary "
- "process running?\n", pathname);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create lock on '%s'."
+ " Is another primary process running?\n",
+ __func__, pathname);
+ return -1;
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
if (rte_mem_cfg_addr == MAP_FAILED){
- rte_panic("Cannot mmap memory for rte_config\n");
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
}
memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* attach to an existing shared memory config */
-static void
+static int
rte_eal_config_attach(void)
{
void *rte_mem_cfg_addr;
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n", pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
close(mem_cfg_fd);
- if (rte_mem_cfg_addr == MAP_FAILED)
- rte_panic("Cannot mmap memory for rte_config\n");
+ if (rte_mem_cfg_addr == MAP_FAILED) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
+ }
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* Detect if we are a primary or a secondary process */
@@ -236,23 +255,28 @@ enum rte_proc_type_t
}
/* Sets up rte_config structure with the pointer to shared memory config.*/
-static void
+static int
rte_config_init(void)
{
rte_config.process_type = internal_config.process_type;
switch (rte_config.process_type){
case RTE_PROC_PRIMARY:
- rte_eal_config_create();
+ if (rte_eal_config_create())
+ return -1;
break;
case RTE_PROC_SECONDARY:
- rte_eal_config_attach();
+ if (rte_eal_config_attach())
+ return -1;
rte_eal_mcfg_wait_complete(rte_config.mem_config);
break;
case RTE_PROC_AUTO:
case RTE_PROC_INVALID:
- rte_panic("Invalid process type\n");
+ RTE_LOG(CRIT, EAL, "%s(): Invalid process type %d\n",
+ __func__, rte_config.process_type);
+ return -1;
}
+ return 0;
}
/* display usage */
@@ -583,7 +607,8 @@ static void rte_eal_init_alert(const char *msg)
rte_srand(rte_rdtsc());
- rte_config_init();
+ if (rte_config_init() != 0)
+ return -1;
if (rte_mp_channel_init() < 0) {
rte_eal_init_alert("failed to init mp channel\n");
@@ -630,7 +655,8 @@ static void rte_eal_init_alert(const char *msg)
eal_check_mem_on_local_socket();
- eal_thread_init_master(rte_config.master_lcore);
+ if (eal_thread_init_master(rte_config.master_lcore) != 0)
+ return -1;
ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
@@ -644,18 +670,27 @@ static void rte_eal_init_alert(const char *msg)
* create communication pipes between master thread
* and children
*/
- if (pipe(lcore_config[i].pipe_master2slave) < 0)
- rte_panic("Cannot create pipe\n");
- if (pipe(lcore_config[i].pipe_slave2master) < 0)
- rte_panic("Cannot create pipe\n");
+ if (pipe(lcore_config[i].pipe_master2slave) < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create pipe\n",
+ __func__);
+ return -1;
+ }
+ if (pipe(lcore_config[i].pipe_slave2master) < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create pipe\n",
+ __func__);
+ return -1;
+ }
lcore_config[i].state = WAIT;
/* create a thread for each lcore */
ret = pthread_create(&lcore_config[i].thread_id, NULL,
eal_thread_loop, NULL);
- if (ret != 0)
- rte_panic("Cannot create thread\n");
+ if (ret != 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create thread\n",
+ __func__);
+ return -1;
+ }
/* Set thread_name for aid in debugging. */
snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
index d602daf..5c3947c 100644
--- a/lib/librte_eal/bsdapp/eal/eal_thread.c
+++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
@@ -51,16 +51,22 @@
n = 0;
while (n == 0 || (n < 0 && errno == EINTR))
n = write(m2s, &c, 1);
- if (n < 0)
- rte_panic("cannot write on configuration pipe\n");
+ if (n < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot write on configuration pipe\n",
+ __func__);
+ return -1;
+ }
/* wait ack */
do {
n = read(s2m, &c, 1);
} while (n < 0 && errno == EINTR);
- if (n <= 0)
- rte_panic("cannot read on configuration pipe\n");
+ if (n <= 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot read on configuration pipe\n",
+ __func__);
+ return -1;
+ }
return 0;
}
@@ -84,8 +90,19 @@ void eal_thread_init_master(unsigned lcore_id)
RTE_PER_LCORE(_lcore_id) = lcore_id;
/* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
+ if (eal_thread_set_affinity() < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot set affinity\n", __func__);
+ rte_move_to_panic_state();
+ }
+}
+
+/* move to panic state and do not return */
+static __attribute__((noreturn)) void
+defunct_and_remain_in_endless_loop(void)
+{
+ rte_move_to_panic_state();
+ while (1)
+ sleep(1);
}
/* main loop of threads */
@@ -106,8 +123,11 @@ void eal_thread_init_master(unsigned lcore_id)
if (thread_id == lcore_config[lcore_id].thread_id)
break;
}
- if (lcore_id == RTE_MAX_LCORE)
- rte_panic("cannot retrieve lcore id\n");
+ if (lcore_id == RTE_MAX_LCORE) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot retrieve lcore id\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
m2s = lcore_config[lcore_id].pipe_master2slave[0];
s2m = lcore_config[lcore_id].pipe_slave2master[1];
@@ -116,8 +136,10 @@ void eal_thread_init_master(unsigned lcore_id)
RTE_PER_LCORE(_lcore_id) = lcore_id;
/* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
+ if (eal_thread_set_affinity() < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot set affinity\n", __func__);
+ defunct_and_remain_in_endless_loop();
+ }
ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
@@ -133,8 +155,11 @@ void eal_thread_init_master(unsigned lcore_id)
n = read(m2s, &c, 1);
} while (n < 0 && errno == EINTR);
- if (n <= 0)
- rte_panic("cannot read on configuration pipe\n");
+ if (n <= 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot read on configuration pipe\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
lcore_config[lcore_id].state = RUNNING;
@@ -142,11 +167,17 @@ void eal_thread_init_master(unsigned lcore_id)
n = 0;
while (n == 0 || (n < 0 && errno == EINTR))
n = write(s2m, &c, 1);
- if (n < 0)
- rte_panic("cannot write on configuration pipe\n");
-
- if (lcore_config[lcore_id].f == NULL)
- rte_panic("NULL function pointer\n");
+ if (n < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot write on configuration pipe\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
+
+ if (lcore_config[lcore_id].f == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): NULL function pointer\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
/* call the function and store the return value */
fct_arg = lcore_config[lcore_id].arg;
diff --git a/lib/librte_eal/common/eal_common_launch.c b/lib/librte_eal/common/eal_common_launch.c
index fe0ba3f..6f8bd46 100644
--- a/lib/librte_eal/common/eal_common_launch.c
+++ b/lib/librte_eal/common/eal_common_launch.c
@@ -14,6 +14,7 @@
#include <rte_pause.h>
#include <rte_per_lcore.h>
#include <rte_lcore.h>
+#include <rte_debug.h>
/*
* Wait until a lcore finished its job.
@@ -88,3 +89,23 @@ enum rte_lcore_state_t
rte_eal_wait_lcore(lcore_id);
}
}
+
+/* panic state */
+static int _panic_state;
+
+/**
+ * Check if the system is in panic state
+ * @return int
+ */
+int rte_get_panic_state(void)
+{
+ return _panic_state;
+}
+
+/**
+ * Move the system to be in panic state
+ */
+void rte_move_to_panic_state(void)
+{
+ _panic_state = 1;
+}
diff --git a/lib/librte_eal/common/include/rte_debug.h b/lib/librte_eal/common/include/rte_debug.h
index 272df49..b421d33 100644
--- a/lib/librte_eal/common/include/rte_debug.h
+++ b/lib/librte_eal/common/include/rte_debug.h
@@ -79,4 +79,16 @@ void __rte_panic(const char *funcname , const char *format, ...)
}
#endif
+/**
+ * Check if the system is in panic state
+ * @return int
+ */
+int rte_get_panic_state(void);
+
+/**
+ * Move the system to be in panic state
+ */
+void rte_move_to_panic_state(void);
+
+
#endif /* _RTE_DEBUG_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 2ecd07b..b7b950a 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -160,7 +160,7 @@ enum rte_iova_mode
* We also don't lock the whole file, so that in future we can use read-locks
* on other parts, e.g. memzones, to detect if there are running secondary
* processes. */
-static void
+static int
rte_eal_config_create(void)
{
void *rte_mem_cfg_addr;
@@ -169,7 +169,7 @@ enum rte_iova_mode
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
/* map the config before hugepage address so that we don't waste a page */
if (internal_config.base_virtaddr != 0)
@@ -179,30 +179,39 @@ enum rte_iova_mode
else
rte_mem_cfg_addr = NULL;
- if (mem_cfg_fd < 0){
+ if (mem_cfg_fd < 0) {
mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n", pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for "
+ "rte_mem_config\n", __func__, pathname);
+ return -1;
+ }
}
retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
- if (retval < 0){
+ if (retval < 0) {
close(mem_cfg_fd);
- rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot resize '%s' for rte_mem_config\n",
+ __func__, pathname);
+ return -1;
}
retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
- if (retval < 0){
+ if (retval < 0) {
close(mem_cfg_fd);
- rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another primary "
- "process running?\n", pathname);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create lock on '%s'."
+ " Is another primary process running?\n",
+ __func__, pathname);
+ return -1;
}
rte_mem_cfg_addr = mmap(rte_mem_cfg_addr, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
- if (rte_mem_cfg_addr == MAP_FAILED){
- rte_panic("Cannot mmap memory for rte_config\n");
+ if (rte_mem_cfg_addr == MAP_FAILED) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for "
+ "rte_config\n", __func__);
+ return -1;
}
memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
rte_config.mem_config = rte_mem_cfg_addr;
@@ -211,10 +220,11 @@ enum rte_iova_mode
* processes could later map the config into this exact location */
rte_config.mem_config->mem_cfg_addr = (uintptr_t) rte_mem_cfg_addr;
+ return 0;
}
/* attach to an existing shared memory config */
-static void
+static int
rte_eal_config_attach(void)
{
struct rte_mem_config *mem_config;
@@ -222,33 +232,41 @@ enum rte_iova_mode
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
- if (mem_cfg_fd < 0){
+ if (mem_cfg_fd < 0) {
mem_cfg_fd = open(pathname, O_RDWR);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n", pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
/* map it as read-only first */
mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config),
PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
- if (mem_config == MAP_FAILED)
- rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
- errno, strerror(errno));
+ if (mem_config == MAP_FAILED) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for "
+ "rte_config! error %i (%s)\n",
+ __func__, errno, strerror(errno));
+ return -1;
+ }
rte_config.mem_config = mem_config;
+
+ return 0;
}
/* reattach the shared config at exact memory location primary process has it */
-static void
+static int
rte_eal_config_reattach(void)
{
struct rte_mem_config *mem_config;
void *rte_mem_cfg_addr;
if (internal_config.no_shconf)
- return;
+ return 0;
/* save the address primary process has mapped shared config to */
rte_mem_cfg_addr = (void *) (uintptr_t) rte_config.mem_config->mem_cfg_addr;
@@ -263,16 +281,21 @@ enum rte_iova_mode
if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
if (mem_config != MAP_FAILED)
/* errno is stale, don't use */
- rte_panic("Cannot mmap memory for rte_config at [%p], got [%p]"
- " - please use '--base-virtaddr' option\n",
- rte_mem_cfg_addr, mem_config);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for "
+ "rte_config at [%p], got [%p] - please use "
+ "'--base-virtaddr' option\n",
+ __func__, rte_mem_cfg_addr, mem_config);
else
- rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
- errno, strerror(errno));
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for "
+ "rte_config! error %i (%s)\n",
+ __func__, errno, strerror(errno));
+ return -1;
}
close(mem_cfg_fd);
rte_config.mem_config = mem_config;
+
+ return 0;
}
/* Detect if we are a primary or a secondary process */
@@ -296,24 +319,31 @@ enum rte_proc_type_t
}
/* Sets up rte_config structure with the pointer to shared memory config.*/
-static void
+static int
rte_config_init(void)
{
rte_config.process_type = internal_config.process_type;
switch (rte_config.process_type){
case RTE_PROC_PRIMARY:
- rte_eal_config_create();
+ if (rte_eal_config_create() != 0)
+ return -1;
break;
case RTE_PROC_SECONDARY:
- rte_eal_config_attach();
+ if (rte_eal_config_attach() != 0)
+ return -1;
rte_eal_mcfg_wait_complete(rte_config.mem_config);
- rte_eal_config_reattach();
+ if (rte_eal_config_reattach() != 0)
+ return -1;
break;
case RTE_PROC_AUTO:
case RTE_PROC_INVALID:
- rte_panic("Invalid process type\n");
+ RTE_LOG(CRIT, EAL, "%s(): Invalid process type %d\n",
+ __func__, rte_config.process_type);
+ return -1;
}
+
+ return 0;
}
/* Unlocks hugepage directories that were locked by eal_hugepage_info_init */
@@ -827,7 +857,8 @@ static void rte_eal_init_alert(const char *msg)
rte_srand(rte_rdtsc());
- rte_config_init();
+ if (rte_config_init() != 0)
+ return -1;
if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0) {
rte_eal_init_alert("Cannot init logging.");
@@ -890,6 +921,9 @@ static void rte_eal_init_alert(const char *msg)
eal_thread_init_master(rte_config.master_lcore);
+ if (rte_get_panic_state())
+ return -1;
+
ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%x;cpuset=[%s%s])\n",
@@ -907,18 +941,27 @@ static void rte_eal_init_alert(const char *msg)
* create communication pipes between master thread
* and children
*/
- if (pipe(lcore_config[i].pipe_master2slave) < 0)
- rte_panic("Cannot create pipe\n");
- if (pipe(lcore_config[i].pipe_slave2master) < 0)
- rte_panic("Cannot create pipe\n");
+ if (pipe(lcore_config[i].pipe_master2slave) < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create pipe\n",
+ __func__);
+ return -1;
+ }
+ if (pipe(lcore_config[i].pipe_slave2master) < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create pipe\n",
+ __func__);
+ return -1;
+ }
lcore_config[i].state = WAIT;
/* create a thread for each lcore */
ret = pthread_create(&lcore_config[i].thread_id, NULL,
eal_thread_loop, NULL);
- if (ret != 0)
- rte_panic("Cannot create thread\n");
+ if (ret != 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create thread\n",
+ __func__);
+ return -1;
+ }
/* Set thread_name for aid in debugging. */
snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c
index 08e150b..3afcee5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_thread.c
+++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
@@ -51,16 +51,22 @@
n = 0;
while (n == 0 || (n < 0 && errno == EINTR))
n = write(m2s, &c, 1);
- if (n < 0)
- rte_panic("cannot write on configuration pipe\n");
+ if (n < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot write on configuration pipe\n",
+ __func__);
+ return -1;
+ }
/* wait ack */
do {
n = read(s2m, &c, 1);
} while (n < 0 && errno == EINTR);
- if (n <= 0)
- rte_panic("cannot read on configuration pipe\n");
+ if (n <= 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot read on configuration pipe\n",
+ __func__);
+ return -1;
+ }
return 0;
}
@@ -84,8 +90,19 @@ void eal_thread_init_master(unsigned lcore_id)
RTE_PER_LCORE(_lcore_id) = lcore_id;
/* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
+ if (eal_thread_set_affinity() < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot set affinity\n", __func__);
+ rte_move_to_panic_state();
+ }
+}
+
+/* move to panic state and do not return */
+static __attribute__((noreturn)) void
+defunct_and_remain_in_endless_loop(void)
+{
+ rte_move_to_panic_state();
+ while (1)
+ sleep(1);
}
/* main loop of threads */
@@ -106,8 +123,11 @@ void eal_thread_init_master(unsigned lcore_id)
if (thread_id == lcore_config[lcore_id].thread_id)
break;
}
- if (lcore_id == RTE_MAX_LCORE)
- rte_panic("cannot retrieve lcore id\n");
+ if (lcore_id == RTE_MAX_LCORE) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot retrieve lcore id\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
m2s = lcore_config[lcore_id].pipe_master2slave[0];
s2m = lcore_config[lcore_id].pipe_slave2master[1];
@@ -116,8 +136,10 @@ void eal_thread_init_master(unsigned lcore_id)
RTE_PER_LCORE(_lcore_id) = lcore_id;
/* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
+ if (eal_thread_set_affinity() < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot set affinity\n", __func__);
+ defunct_and_remain_in_endless_loop();
+ }
ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
@@ -133,8 +155,11 @@ void eal_thread_init_master(unsigned lcore_id)
n = read(m2s, &c, 1);
} while (n < 0 && errno == EINTR);
- if (n <= 0)
- rte_panic("cannot read on configuration pipe\n");
+ if (n <= 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot read on configuration pipe\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
lcore_config[lcore_id].state = RUNNING;
@@ -142,11 +167,17 @@ void eal_thread_init_master(unsigned lcore_id)
n = 0;
while (n == 0 || (n < 0 && errno == EINTR))
n = write(s2m, &c, 1);
- if (n < 0)
- rte_panic("cannot write on configuration pipe\n");
-
- if (lcore_config[lcore_id].f == NULL)
- rte_panic("NULL function pointer\n");
+ if (n < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot write on configuration pipe\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
+
+ if (lcore_config[lcore_id].f == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): NULL function pointer\n",
+ __func__);
+ defunct_and_remain_in_endless_loop();
+ }
/* call the function and store the return value */
fct_arg = lcore_config[lcore_id].arg;
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [dpdk-dev] [PATCH 13/13] devtools: prevent new instances of rte_panic and rte_exit
2018-04-04 11:27 [dpdk-dev] eal: replace calls to rte_panic and refrain from new instances Arnon Warshavsky
` (11 preceding siblings ...)
2018-04-04 11:27 ` [dpdk-dev] [PATCH 12/13] eal: replace rte_panic instances in init sequence Arnon Warshavsky
@ 2018-04-04 11:27 ` Arnon Warshavsky
12 siblings, 0 replies; 21+ messages in thread
From: Arnon Warshavsky @ 2018-04-04 11:27 UTC (permalink / raw)
To: thomas, anatoly.burakov, wenzhuo.lu, declan.doherty, jerin.jacob,
bruce.richardson, ferruh.yigit
Cc: dev, arnon
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
devtools/checkpatches.sh | 94 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 1 deletion(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 7676a6b..fb37838 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -61,6 +61,90 @@ print_usage () {
END_OF_HELP
}
+check_forbidden_additions() { # <file>
+ # ---------------------------------
+ #This awk script receives a list of expressions to monitor
+ #and a list of folders to search these expressions in
+ # - No search is done inside comments
+ # - Both additions and removals of the expressions are checked
+ # A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN{
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+ }
+ # search for add/remove instances in current file
+ # state machine assumes the comments structure is enforced by
+ # checkpatches.pl
+ (in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+ }
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ print "Forbidden expressions in " substr(last_file,6) ":"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit 1
+ }
+ }
+EOF
+ # ---------------------------------
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -89,11 +173,19 @@ check () { # <patch> <commit> <title>
total=$(($total + 1))
! $verbose || printf '\n### %s\n\n' "$3"
if [ -n "$1" ] ; then
+ cat "$1" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options "$1" 2>/dev/null)
elif [ -n "$2" ] ; then
- report=$(git format-patch --find-renames --no-stat --stdout -1 $commit |
+ params=$(echo "--find-renames --no-stat --stdout -1")
+ body=$(git format-patch $params $commit)
+ echo "$body" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
+ report=$(echo "$body" |
$DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
else
+ check_forbidden_additions -
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
fi
[ $? -ne 0 ] || return 0
--
1.8.3.1
^ permalink raw reply [flat|nested] 21+ messages in thread