On 04/26/2018 07:20 AM, Arnon Warshavsky wrote: > Replace panic calls with log and return value. > Local functions to this file, > changing from void to int are non-abi-breaking > > Signed-off-by: Arnon Warshavsky <ar...@qwilt.com> > --- > drivers/net/bonding/rte_eth_bond_8023ad.c | 29 > ++++++++++++++--------- > drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +- > drivers/net/bonding/rte_eth_bond_api.c | 22 ++++++++++++----- > drivers/net/bonding/rte_eth_bond_pmd.c | 9 ++++--- > drivers/net/bonding/rte_eth_bond_private.h | 2 +- > 5 files changed, 42 insertions(+), 22 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c > b/drivers/net/bonding/rte_eth_bond_8023ad.c > index c452318..308e623 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,9 @@ > /* Any memory allocation failure in initialization is critical because > * resources can't be free, so reinitialization is impossible. */
What about this comment? Not sure if it is just stale or if there is something else you need to do. > 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_BOND_LOG(ERR, "%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 +978,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_BOND_LOG(ERR, "%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 +989,12 @@ > 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_BOND_LOG(ERR, "%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 +1148,11 @@ > 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, > - internals->active_slaves[i]); > + for (i = 0; i < internals->active_slave_count; i++) { > + if (!bond_mode_8023ad_activate_slave(bond_dev, > + internals->active_slaves[i])) > + return -1; > + } > > 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. ^^^ you inadvertently fixed a doxygen bug :-) > */ > -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 aa89425..657fd74 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) { > @@ -94,6 +95,8 @@ > bond_tlb_activate_slave(internals); > if (internals->mode == BONDING_MODE_ALB) > bond_mode_alb_client_list_upd(eth_dev); > + > + return 0; > } > > void > @@ -357,10 +360,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; > + } Not functionally wrong, but commented about no need for variables here in v4 and seems to have been missed > } > } > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > b/drivers/net/bonding/rte_eth_bond_pmd.c > index 09696ea..d2dbe4a 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1741,8 +1741,10 @@ 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_BOND_LOG(ERR, "%s() Slave %u: Failed to create > memory pool '%s': %s\n", > + __func__, slave_id, > + mem_name, rte_strerror(rte_errno)); > + return -1; > } > } > > @@ -2673,7 +2675,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 94eca88..d99d42c 100644 > --- a/drivers/net/bonding/rte_eth_bond_private.h > +++ b/drivers/net/bonding/rte_eth_bond_private.h > @@ -187,7 +187,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 >