From: Jiri Pirko <jiri@resnulli.us>
To: Maor Gottlieb <maorg@mellanox.com>
Cc: davem@davemloft.net, jgg@mellanox.com, dledford@redhat.com,
j.vosburgh@gmail.com, vfalico@gmail.com, andy@greyhouse.net,
kuba@kernel.org, leonro@mellanox.com, saeedm@mellanox.com,
jiri@mellanox.com, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, alexr@mellanox.com
Subject: Re: [PATCH V2 mlx5-next 02/10] bonding: Rename slave_arr to usable_slaves
Date: Mon, 20 Apr 2020 16:17:50 +0200 [thread overview]
Message-ID: <20200420141750.GL6581@nanopsycho.orion> (raw)
In-Reply-To: <20200420075426.31462-3-maorg@mellanox.com>
Mon, Apr 20, 2020 at 09:54:18AM CEST, maorg@mellanox.com wrote:
>This patch renames slave_arr to usable_slaves, since we will
>have two arrays, one for the usable slaves and the other to all
>slaves. In addition, exports the bond_skip_slave logic to function.
I the patch description, you should tell the codebase what to do. You
should not talk about "this patch".
>
>Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
>---
> drivers/net/bonding/bond_alb.c | 4 +-
> drivers/net/bonding/bond_main.c | 85 +++++++++++++++++----------------
> include/net/bonding.h | 2 +-
> 3 files changed, 48 insertions(+), 43 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index c81698550e5a..7bb49b049dcc 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -1360,7 +1360,7 @@ netdev_tx_t bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> struct bond_up_slave *slaves;
> unsigned int count;
>
>- slaves = rcu_dereference(bond->slave_arr);
>+ slaves = rcu_dereference(bond->usable_slaves);
> count = slaves ? READ_ONCE(slaves->count) : 0;
> if (likely(count))
> tx_slave = slaves->arr[hash_index %
>@@ -1494,7 +1494,7 @@ netdev_tx_t bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> struct bond_up_slave *slaves;
> unsigned int count;
>
>- slaves = rcu_dereference(bond->slave_arr);
>+ slaves = rcu_dereference(bond->usable_slaves);
> count = slaves ? READ_ONCE(slaves->count) : 0;
> if (likely(count))
> tx_slave = slaves->arr[bond_xmit_hash(bond, skb) %
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 2e70e43c5df5..2cb41d480ae2 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -4087,6 +4087,29 @@ static void bond_slave_arr_handler(struct work_struct *work)
> bond_slave_arr_work_rearm(bond, 1);
> }
>
>+static void bond_skip_slave(struct bond_up_slave *slaves,
>+ struct slave *skipslave)
>+{
>+ int idx;
>+
>+ /* Rare situation where caller has asked to skip a specific
>+ * slave but allocation failed (most likely!). BTW this is
>+ * only possible when the call is initiated from
>+ * __bond_release_one(). In this situation; overwrite the
>+ * skipslave entry in the array with the last entry from the
>+ * array to avoid a situation where the xmit path may choose
>+ * this to-be-skipped slave to send a packet out.
>+ */
>+ for (idx = 0; slaves && idx < slaves->count; idx++) {
>+ if (skipslave == slaves->arr[idx]) {
>+ slaves->arr[idx] =
>+ slaves->arr[slaves->count - 1];
>+ slaves->count--;
>+ break;
>+ }
>+ }
Do this move in a separate patch. Is not related to the rename.
>+}
>+
> /* Build the usable slaves array in control path for modes that use xmit-hash
> * to determine the slave interface -
> * (a) BOND_MODE_8023AD
>@@ -4097,9 +4120,9 @@ static void bond_slave_arr_handler(struct work_struct *work)
> */
> int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
> {
>+ struct bond_up_slave *usable_slaves, *old_usable_slaves;
> struct slave *slave;
> struct list_head *iter;
>- struct bond_up_slave *new_arr, *old_arr;
> int agg_id = 0;
> int ret = 0;
>
>@@ -4107,11 +4130,10 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
> WARN_ON(lockdep_is_held(&bond->mode_lock));
> #endif
>
>- new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
>- GFP_KERNEL);
>- if (!new_arr) {
>+ usable_slaves = kzalloc(struct_size(usable_slaves, arr,
>+ bond->slave_cnt), GFP_KERNEL);
>+ if (!usable_slaves) {
> ret = -ENOMEM;
>- pr_err("Failed to build slave-array.\n");
> goto out;
> }
> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>@@ -4119,14 +4141,14 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
>
> if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
> pr_debug("bond_3ad_get_active_agg_info failed\n");
>- kfree_rcu(new_arr, rcu);
>+ kfree_rcu(usable_slaves, rcu);
> /* No active aggragator means it's not safe to use
> * the previous array.
> */
>- old_arr = rtnl_dereference(bond->slave_arr);
>- if (old_arr) {
>- RCU_INIT_POINTER(bond->slave_arr, NULL);
>- kfree_rcu(old_arr, rcu);
>+ old_usable_slaves = rtnl_dereference(bond->usable_slaves);
>+ if (old_usable_slaves) {
>+ RCU_INIT_POINTER(bond->usable_slaves, NULL);
>+ kfree_rcu(old_usable_slaves, rcu);
> }
> goto out;
> }
>@@ -4146,37 +4168,20 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
> continue;
>
> slave_dbg(bond->dev, slave->dev, "Adding slave to tx hash array[%d]\n",
>- new_arr->count);
>+ usable_slaves->count);
>
>- new_arr->arr[new_arr->count++] = slave;
>+ usable_slaves->arr[usable_slaves->count++] = slave;
> }
>
>- old_arr = rtnl_dereference(bond->slave_arr);
>- rcu_assign_pointer(bond->slave_arr, new_arr);
>- if (old_arr)
>- kfree_rcu(old_arr, rcu);
>+ old_usable_slaves = rtnl_dereference(bond->usable_slaves);
>+ rcu_assign_pointer(bond->usable_slaves, usable_slaves);
>+ if (old_usable_slaves)
>+ kfree_rcu(old_usable_slaves, rcu);
> out:
>- if (ret != 0 && skipslave) {
>- int idx;
>-
>- /* Rare situation where caller has asked to skip a specific
>- * slave but allocation failed (most likely!). BTW this is
>- * only possible when the call is initiated from
>- * __bond_release_one(). In this situation; overwrite the
>- * skipslave entry in the array with the last entry from the
>- * array to avoid a situation where the xmit path may choose
>- * this to-be-skipped slave to send a packet out.
>- */
>- old_arr = rtnl_dereference(bond->slave_arr);
>- for (idx = 0; old_arr != NULL && idx < old_arr->count; idx++) {
>- if (skipslave == old_arr->arr[idx]) {
>- old_arr->arr[idx] =
>- old_arr->arr[old_arr->count-1];
>- old_arr->count--;
>- break;
>- }
>- }
>- }
>+ if (ret != 0 && skipslave)
>+ bond_skip_slave(rtnl_dereference(bond->usable_slaves),
>+ skipslave);
>+
> return ret;
> }
>
>@@ -4192,7 +4197,7 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
> struct bond_up_slave *slaves;
> unsigned int count;
>
>- slaves = rcu_dereference(bond->slave_arr);
>+ slaves = rcu_dereference(bond->usable_slaves);
> count = slaves ? READ_ONCE(slaves->count) : 0;
> if (likely(count)) {
> slave = slaves->arr[bond_xmit_hash(bond, skb) % count];
>@@ -4483,9 +4488,9 @@ static void bond_uninit(struct net_device *bond_dev)
> __bond_release_one(bond_dev, slave->dev, true, true);
> netdev_info(bond_dev, "Released all slaves\n");
>
>- arr = rtnl_dereference(bond->slave_arr);
>+ arr = rtnl_dereference(bond->usable_slaves);
> if (arr) {
>- RCU_INIT_POINTER(bond->slave_arr, NULL);
>+ RCU_INIT_POINTER(bond->usable_slaves, NULL);
> kfree_rcu(arr, rcu);
> }
>
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index dc2ce31a1f52..33bdb6d5182d 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -200,7 +200,7 @@ struct bonding {
> struct slave __rcu *curr_active_slave;
> struct slave __rcu *current_arp_slave;
> struct slave __rcu *primary_slave;
>- struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
>+ struct bond_up_slave __rcu *usable_slaves; /* Array of usable slaves */
> bool force_primary;
> s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
> int (*recv_probe)(const struct sk_buff *, struct bonding *,
>--
>2.17.2
>
next prev parent reply other threads:[~2020-04-20 14:17 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-20 7:54 [PATCH V2 mlx5-next 00/10] Add support to get xmit slave Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 01/10] net/core: Introduce master_xmit_slave_get Maor Gottlieb
2020-04-20 14:01 ` Jiri Pirko
2020-04-20 17:29 ` David Ahern
2020-04-20 17:41 ` Jiri Pirko
2020-04-20 17:43 ` David Ahern
2020-04-20 17:53 ` Jiri Pirko
2020-04-20 17:54 ` Jiri Pirko
2020-04-20 17:56 ` David Ahern
2020-04-20 18:01 ` Jiri Pirko
2020-04-20 18:04 ` David Ahern
2020-04-20 18:48 ` Jiri Pirko
2020-04-20 18:56 ` Maor Gottlieb
2020-04-20 19:02 ` David Ahern
2020-04-21 5:37 ` Jiri Pirko
2020-04-21 5:43 ` Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 02/10] bonding: Rename slave_arr to usable_slaves Maor Gottlieb
2020-04-20 14:17 ` Jiri Pirko [this message]
2020-04-20 7:54 ` [PATCH V2 mlx5-next 03/10] bonding: Add helpers to get xmit slave Maor Gottlieb
2020-04-20 14:27 ` Jiri Pirko
2020-04-20 18:46 ` Jay Vosburgh
2020-04-20 18:59 ` Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 04/10] bonding: Implement ndo_xmit_slave_get Maor Gottlieb
2020-04-20 15:04 ` Jiri Pirko
2020-04-20 15:36 ` David Ahern
2020-04-20 7:54 ` [PATCH V2 mlx5-next 05/10] RDMA/core: Add LAG functionality Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 06/10] RDMA/core: Get xmit slave for LAG Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 07/10] net/mlx5: Change lag mutex lock to spin lock Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 08/10] net/mlx5: Add support to get lag physical port Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 09/10] RDMA/mlx5: Refactor affinity related code Maor Gottlieb
2020-04-20 7:54 ` [PATCH V2 mlx5-next 10/10] RDMA/mlx5: Set lag tx affinity according to slave Maor Gottlieb
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200420141750.GL6581@nanopsycho.orion \
--to=jiri@resnulli.us \
--cc=alexr@mellanox.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=dledford@redhat.com \
--cc=j.vosburgh@gmail.com \
--cc=jgg@mellanox.com \
--cc=jiri@mellanox.com \
--cc=kuba@kernel.org \
--cc=leonro@mellanox.com \
--cc=linux-rdma@vger.kernel.org \
--cc=maorg@mellanox.com \
--cc=netdev@vger.kernel.org \
--cc=saeedm@mellanox.com \
--cc=vfalico@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).