Patch Comment List
List patch comments
GET /api/patches/418/comments/?format=api
[ { "id": 834, "web_url": "http://patches.dpdk.org/comment/834/", "msgid": "<20140917145102.GC4213@localhost.localdomain>", "list_archive_url": "https://inbox.dpdk.org/dev/20140917145102.GC4213@localhost.localdomain", "date": "2014-09-17T14:51:02", "subject": "Re: [dpdk-dev] [PATCH 1/2] bond: extract common code to separate\n\tfunctions", "submitter": { "id": 32, "url": "http://patches.dpdk.org/api/people/32/?format=api", "name": "Neil Horman", "email": "nhorman@tuxdriver.com" }, "content": "On Wed, Sep 17, 2014 at 03:21:52PM +0100, Pawel Wodkowski wrote:\n> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>\n> Reviewed-by: Declan Doherty <declan.doherty@intel.com>\n> ---\n> lib/librte_pmd_bond/rte_eth_bond_api.c | 59 +++++++++++++++++++++-------\n> lib/librte_pmd_bond/rte_eth_bond_pmd.c | 47 ++++++++++++++--------\n> lib/librte_pmd_bond/rte_eth_bond_private.h | 30 ++++++++++++--\n> 3 files changed, 102 insertions(+), 34 deletions(-)\n> \n> diff --git a/lib/librte_pmd_bond/rte_eth_bond_api.c b/lib/librte_pmd_bond/rte_eth_bond_api.c\n> index dd33119..460df65 100644\n> --- a/lib/librte_pmd_bond/rte_eth_bond_api.c\n> +++ b/lib/librte_pmd_bond/rte_eth_bond_api.c\n> @@ -31,6 +31,8 @@\n> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n> */\n> \n> +#include <string.h>\n> +\n> #include <rte_mbuf.h>\n> #include <rte_malloc.h>\n> #include <rte_ethdev.h>\n> @@ -104,6 +106,39 @@ valid_slave_port_id(uint8_t port_id)\n> \treturn 0;\n> }\n> \n> +void\n> +rte_eth_bond_activate_slave(struct rte_eth_dev *eth_dev, uint8_t port_id )\n> +{\n> +\tstruct bond_dev_private *internals = eth_dev->data->dev_private;\n> +\tuint8_t active_count = internals->active_slave_count;\n> +\n> +\tinternals->active_slaves[active_count] = port_id;\n> +\n> +\n> +\tinternals->active_slave_count = active_count + 1;\n> +}\n> +\n> +void\n> +rte_eth_bond_deactive_slave(struct rte_eth_dev *eth_dev,\nI think you intended for this to be deactivate, not deactive, yes?\n\n\n> +\tuint8_t slave_pos )\n> +{\n> +\tstruct bond_dev_private *internals = eth_dev->data->dev_private;\n> +\tuint8_t active_count = internals->active_slave_count;\n> +\n> +\tactive_count--;\n> +\n> +\t/* If slave was not at the end of the list\n> +\t * shift active slaves up active array list */\n> +\tif (slave_pos < active_count) {\n> +\t\tmemmove(internals->active_slaves + slave_pos,\n> +\t\t\t\tinternals->active_slaves + slave_pos + 1,\n> +\t\t\t\t(active_count - slave_pos) *\n> +\t\t\t\t\tsizeof(internals->active_slaves[0]));\n> +\t}\n> +\n> +\tinternals->active_slave_count = active_count;\n> +}\n> +\n> uint8_t\n> number_of_sockets(void)\nStatic?\n\n> {\n> @@ -356,10 +391,8 @@ rte_eth_bond_slave_add(uint8_t bonded_port_id, uint8_t slave_port_id)\n> \tif (bonded_eth_dev->data->dev_started) {\n> \t\trte_eth_link_get_nowait(slave_port_id, &link_props);\n> \n> -\t\t if (link_props.link_status == 1) {\n> -\t\t\tinternals->active_slaves[internals->active_slave_count++] =\n> -\t\t\t\t\tslave_port_id;\n> -\t\t}\n> +\t\t if (link_props.link_status == 1)\n> +\t\t\trte_eth_bond_activate_slave(bonded_eth_dev, slave_port_id);\n> \t}\n> \n> \treturn 0;\n> @@ -373,6 +406,7 @@ err_add:\n> int\n> rte_eth_bond_slave_remove(uint8_t bonded_port_id, uint8_t slave_port_id)\n> {\n> +\tstruct rte_eth_dev *eth_dev;\n> \tstruct bond_dev_private *internals;\n> \tstruct slave_conf *slave_conf;\n> \n> @@ -386,20 +420,15 @@ rte_eth_bond_slave_remove(uint8_t bonded_port_id, uint8_t slave_port_id)\n> \tif (valid_slave_port_id(slave_port_id) != 0)\n> \t\tgoto err_del;\n> \n> -\tinternals = rte_eth_devices[bonded_port_id].data->dev_private;\n> +\teth_dev = &rte_eth_devices[bonded_port_id];\n> +\tinternals = eth_dev->data->dev_private;\n> \n> \t/* first remove from active slave list */\n> -\tfor (i = 0; i < internals->active_slave_count; i++) {\n> -\t\tif (internals->active_slaves[i] == slave_port_id)\n> -\t\t\tpos = i;\n> -\n> -\t\t/* shift active slaves up active array list */\n> -\t\tif (pos >= 0 && i < (internals->active_slave_count - 1))\n> -\t\t\tinternals->active_slaves[i] = internals->active_slaves[i+1];\n> -\t}\n> +\tpos = find_slave_by_id(internals->active_slaves, internals->active_slave_count,\n> +\t\t\tslave_port_id);\n> \n> -\tif (pos >= 0)\n> -\t\tinternals->active_slave_count--;\n> +\tif (pos < internals->active_slave_count)\n> +\t\trte_eth_bond_deactive_slave(eth_dev, pos);\n> \n> \tpos = -1;\n> \t/* now remove from slave list */\n> diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c\n> index 38cc1ae..482ddb8 100644\n> --- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c\n> +++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c\n> @@ -447,6 +447,27 @@ link_properties_valid(struct rte_eth_link *bonded_dev_link,\n> }\n> \n> int\n> +mac_address_get(struct rte_eth_dev *eth_dev, struct ether_addr *dst_mac_addr)\n> +{\n> +\tstruct ether_addr *mac_addr;\n> +\n> +\tmac_addr = eth_dev->data->mac_addrs;\n> +\n> +\tif (eth_dev == NULL) {\n> +\t\tRTE_LOG(ERR, PMD, \"%s: NULL pointer eth_dev specified\\n\", __func__);\n> +\t\treturn -1;\n> +\t}\n> +\n> +\tif (dst_mac_addr == NULL) {\n> +\t\tRTE_LOG(ERR, PMD, \"%s: NULL pointer MAC specified\\n\", __func__);\n> +\t\treturn -1;\n> +\t}\n> +\n> +\tether_addr_copy(mac_addr, dst_mac_addr);\n> +\treturn 0;\n> +}\n> +\n> +int\n> mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr)\n> {\n> \tstruct ether_addr *mac_addr;\n> @@ -817,7 +838,7 @@ bond_ethdev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,\n> \t\t\t\t\t0, dev->pci_dev->numa_node);\n> \n> \tif (bd_tx_q == NULL)\n> -\t\t\treturn -1;\n> +\t\treturn -1;\n> \n> \tbd_tx_q->queue_id = tx_queue_id;\n> \tbd_tx_q->dev_private = dev->data->dev_private;\n> @@ -940,7 +961,6 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev)\n> \tcase BONDING_MODE_ACTIVE_BACKUP:\n> \tdefault:\n> \t\trte_eth_promiscuous_enable(internals->current_primary_port);\n> -\n> \t}\n> }\n> \n> @@ -975,7 +995,8 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,\n> \tstruct bond_dev_private *internals;\n> \tstruct rte_eth_link link;\n> \n> -\tint i, valid_slave = 0, active_pos = -1;\n> +\tint i, valid_slave = 0;\n> +\tuint8_t active_pos;\n> \tuint8_t lsc_flag = 0;\n> \n> \tif (type != RTE_ETH_EVENT_INTR_LSC || param == NULL)\n> @@ -1005,16 +1026,12 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,\n> \t\treturn;\n> \n> \t/* Search for port in active port list */\n> -\tfor (i = 0; i < internals->active_slave_count; i++) {\n> -\t\tif (port_id == internals->active_slaves[i]) {\n> -\t\t\tactive_pos = i;\n> -\t\t\tbreak;\n> -\t\t}\n> -\t}\n> +\tactive_pos = find_slave_by_id(internals->active_slaves,\n> +\t\t\tinternals->active_slave_count, port_id);\n> \n> \trte_eth_link_get_nowait(port_id, &link);\n> \tif (link.link_status) {\n> -\t\tif (active_pos >= 0)\n> +\t\tif (active_pos < internals->active_slave_count)\n> \t\t\treturn;\n> \n> \t\t/* if no active slave ports then set this port to be primary port */\n> @@ -1028,21 +1045,19 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,\n> \t\t\tlink_properties_set(bonded_eth_dev,\n> \t\t\t\t\t&(slave_eth_dev->data->dev_link));\n> \t\t}\n> -\t\tinternals->active_slaves[internals->active_slave_count++] = port_id;\n> +\n> +\t\trte_eth_bond_activate_slave(bonded_eth_dev, port_id);\n> \n> \t\t/* If user has defined the primary port then default to using it */\n> \t\tif (internals->user_defined_primary_port &&\n> \t\t\t\tinternals->primary_port == port_id)\n> \t\t\tbond_ethdev_primary_set(internals, port_id);\n> \t} else {\n> -\t\tif (active_pos < 0)\n> +\t\tif (active_pos == internals->active_slave_count)\n> \t\t\treturn;\n> \n> \t\t/* Remove from active slave list */\n> -\t\tfor (i = active_pos; i < (internals->active_slave_count - 1); i++)\n> -\t\t\tinternals->active_slaves[i] = internals->active_slaves[i+1];\n> -\n> -\t\tinternals->active_slave_count--;\n> +\t\trte_eth_bond_deactive_slave(bonded_eth_dev, active_pos);\n> \n> \t\t/* No active slaves, change link status to down and reset other\n> \t\t * link properties */\n> diff --git a/lib/librte_pmd_bond/rte_eth_bond_private.h b/lib/librte_pmd_bond/rte_eth_bond_private.h\n> index 60f1e8d..6742a4c 100644\n> --- a/lib/librte_pmd_bond/rte_eth_bond_private.h\n> +++ b/lib/librte_pmd_bond/rte_eth_bond_private.h\n> @@ -115,11 +115,11 @@ struct bond_dev_private {\n> \tuint16_t nb_rx_queues;\t\t\t/**< Total number of rx queues */\n> \tuint16_t nb_tx_queues;\t\t\t/**< Total number of tx queues*/\n> \n> -\tuint8_t slave_count;\t\t\t/**< Number of active slaves */\n> -\tuint8_t active_slave_count;\t\t/**< Number of slaves */\n> +\tuint8_t slave_count;\t\t\t/**< Number of slaves */\n> +\tuint8_t active_slave_count;\t\t/**< Number of active slaves */\n> \n> -\tuint8_t active_slaves[RTE_MAX_ETHPORTS];\t/**< Active slave list */\n> \tuint8_t slaves[RTE_MAX_ETHPORTS];\t\t\t/**< Slave list */\n> +\tuint8_t active_slaves[RTE_MAX_ETHPORTS];\t/**< Active slave list */\n> \n> \t/** Persisted configuration of slaves */\n> \tstruct slave_conf presisted_slaves_conf[RTE_MAX_ETHPORTS];\n> @@ -130,6 +130,19 @@ extern struct eth_dev_ops default_dev_ops;\n> int\n> valid_bonded_ethdev(struct rte_eth_dev *eth_dev);\n> \n> +static inline uint8_t\n> +find_slave_by_id(uint8_t *slaves, uint8_t slaves_count,\n> +\tuint8_t slave_id ) {\n> +\n> +\tuint8_t pos;\n> +\tfor (pos = 0; pos < slaves_count; pos++) {\n> +\t\tif (slave_id == slaves[pos])\n> +\t\t\tbreak;\n> +\t}\n> +\n> +\treturn pos;\n> +}\n> +\n> int\n> valid_port_id(uint8_t port_id);\n> \n> @@ -140,6 +153,14 @@ int\n> valid_slave_port_id(uint8_t port_id);\n> \n> void\n> +rte_eth_bond_deactive_slave(struct rte_eth_dev *eth_dev,\n> +\tuint8_t slave_pos );\n> +\n> +void\n> +rte_eth_bond_activate_slave(struct rte_eth_dev *eth_dev,\n> +\tuint8_t port_id );\n> +\n> +void\n> link_properties_set(struct rte_eth_dev *bonded_eth_dev,\n> \t\tstruct rte_eth_link *slave_dev_link);\n> void\n> @@ -153,6 +174,9 @@ int\n> mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr);\n> \n> int\n> +mac_address_get(struct rte_eth_dev *eth_dev, struct ether_addr *dst_mac_addr);\n> +\n> +int\n> mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev);\n> \n> uint8_t\n> -- \n> 1.7.9.5\n> \n>", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 9D96DB3C0;\n\tWed, 17 Sep 2014 16:45:35 +0200 (CEST)", "from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])\n\tby dpdk.org (Postfix) with ESMTP id DD094B3BF\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 16:45:33 +0200 (CEST)", "from nat-pool-rdu-u.redhat.com ([66.187.233.203] helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)\n\t(envelope-from <nhorman@tuxdriver.com>)\n\tid 1XUGZY-0000yf-79; Wed, 17 Sep 2014 10:51:14 -0400" ], "Date": "Wed, 17 Sep 2014 10:51:02 -0400", "From": "Neil Horman <nhorman@tuxdriver.com>", "To": "Pawel Wodkowski <pawelx.wodkowski@intel.com>", "Message-ID": "<20140917145102.GC4213@localhost.localdomain>", "References": "<1410963713-13837-1-git-send-email-pawelx.wodkowski@intel.com>\n\t<1410963713-13837-2-git-send-email-pawelx.wodkowski@intel.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<1410963713-13837-2-git-send-email-pawelx.wodkowski@intel.com>", "User-Agent": "Mutt/1.5.23 (2014-03-12)", "X-Spam-Score": "-2.9 (--)", "X-Spam-Status": "No", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH 1/2] bond: extract common code to separate\n\tfunctions", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null } ]