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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.