[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZrNIrpIytVv8AV0-@Laptop-X1>
Date: Wed, 7 Aug 2024 18:13:02 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Tariq Toukan <tariqt@...dia.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Eric Dumazet <edumazet@...gle.com>, Jay Vosburgh <jv@...sburgh.net>,
Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org,
Saeed Mahameed <saeedm@...dia.com>, Gal Pressman <gal@...dia.com>,
Leon Romanovsky <leonro@...dia.com>,
Jianbo Liu <jianbol@...dia.com>, Cosmin Ratiu <cratiu@...dia.com>
Subject: Re: [PATCH net V3 2/3] bonding: extract the use of real_device into
local variable
On Mon, Aug 05, 2024 at 08:03:56AM +0300, Tariq Toukan wrote:
> From: Jianbo Liu <jianbol@...dia.com>
>
> Add a local variable for slave->dev, to prepare for the lock change in
> the next patch. There is no functionality change.
>
> Fixes: 9a5605505d9c ("bonding: Add struct bond_ipesc to manage SA")
> Signed-off-by: Jianbo Liu <jianbol@...dia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@...dia.com>
> Signed-off-by: Tariq Toukan <tariqt@...dia.com>
> ---
> drivers/net/bonding/bond_main.c | 58 +++++++++++++++++++--------------
> 1 file changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index eb5e43860670..e550b1c08fdb 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -427,6 +427,7 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs,
> struct netlink_ext_ack *extack)
> {
> struct net_device *bond_dev = xs->xso.dev;
> + struct net_device *real_dev;
> struct bond_ipsec *ipsec;
> struct bonding *bond;
> struct slave *slave;
> @@ -443,9 +444,10 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs,
> return -ENODEV;
> }
>
> - if (!slave->dev->xfrmdev_ops ||
> - !slave->dev->xfrmdev_ops->xdo_dev_state_add ||
> - netif_is_bond_master(slave->dev)) {
> + real_dev = slave->dev;
> + if (!real_dev->xfrmdev_ops ||
> + !real_dev->xfrmdev_ops->xdo_dev_state_add ||
> + netif_is_bond_master(real_dev)) {
> NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload");
> rcu_read_unlock();
> return -EINVAL;
> @@ -456,9 +458,9 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs,
> rcu_read_unlock();
> return -ENOMEM;
> }
> - xs->xso.real_dev = slave->dev;
>
> - err = slave->dev->xfrmdev_ops->xdo_dev_state_add(xs, extack);
> + xs->xso.real_dev = real_dev;
> + err = real_dev->xfrmdev_ops->xdo_dev_state_add(xs, extack);
> if (!err) {
> ipsec->xs = xs;
> INIT_LIST_HEAD(&ipsec->list);
> @@ -475,6 +477,7 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs,
> static void bond_ipsec_add_sa_all(struct bonding *bond)
> {
> struct net_device *bond_dev = bond->dev;
> + struct net_device *real_dev;
> struct bond_ipsec *ipsec;
> struct slave *slave;
>
> @@ -483,12 +486,13 @@ static void bond_ipsec_add_sa_all(struct bonding *bond)
> if (!slave)
> goto out;
>
> - if (!slave->dev->xfrmdev_ops ||
> - !slave->dev->xfrmdev_ops->xdo_dev_state_add ||
> - netif_is_bond_master(slave->dev)) {
> + real_dev = slave->dev;
> + if (!real_dev->xfrmdev_ops ||
> + !real_dev->xfrmdev_ops->xdo_dev_state_add ||
> + netif_is_bond_master(real_dev)) {
> spin_lock_bh(&bond->ipsec_lock);
> if (!list_empty(&bond->ipsec_list))
> - slave_warn(bond_dev, slave->dev,
> + slave_warn(bond_dev, real_dev,
> "%s: no slave xdo_dev_state_add\n",
> __func__);
> spin_unlock_bh(&bond->ipsec_lock);
> @@ -497,9 +501,9 @@ static void bond_ipsec_add_sa_all(struct bonding *bond)
>
> spin_lock_bh(&bond->ipsec_lock);
> list_for_each_entry(ipsec, &bond->ipsec_list, list) {
> - ipsec->xs->xso.real_dev = slave->dev;
> - if (slave->dev->xfrmdev_ops->xdo_dev_state_add(ipsec->xs, NULL)) {
> - slave_warn(bond_dev, slave->dev, "%s: failed to add SA\n", __func__);
> + ipsec->xs->xso.real_dev = real_dev;
> + if (real_dev->xfrmdev_ops->xdo_dev_state_add(ipsec->xs, NULL)) {
> + slave_warn(bond_dev, real_dev, "%s: failed to add SA\n", __func__);
> ipsec->xs->xso.real_dev = NULL;
> }
> }
> @@ -515,6 +519,7 @@ static void bond_ipsec_add_sa_all(struct bonding *bond)
> static void bond_ipsec_del_sa(struct xfrm_state *xs)
> {
> struct net_device *bond_dev = xs->xso.dev;
> + struct net_device *real_dev;
> struct bond_ipsec *ipsec;
> struct bonding *bond;
> struct slave *slave;
> @@ -532,16 +537,17 @@ static void bond_ipsec_del_sa(struct xfrm_state *xs)
> if (!xs->xso.real_dev)
> goto out;
>
> - WARN_ON(xs->xso.real_dev != slave->dev);
> + real_dev = slave->dev;
> + WARN_ON(xs->xso.real_dev != real_dev);
>
> - if (!slave->dev->xfrmdev_ops ||
> - !slave->dev->xfrmdev_ops->xdo_dev_state_delete ||
> - netif_is_bond_master(slave->dev)) {
> - slave_warn(bond_dev, slave->dev, "%s: no slave xdo_dev_state_delete\n", __func__);
> + if (!real_dev->xfrmdev_ops ||
> + !real_dev->xfrmdev_ops->xdo_dev_state_delete ||
> + netif_is_bond_master(real_dev)) {
> + slave_warn(bond_dev, real_dev, "%s: no slave xdo_dev_state_delete\n", __func__);
> goto out;
> }
>
> - slave->dev->xfrmdev_ops->xdo_dev_state_delete(xs);
> + real_dev->xfrmdev_ops->xdo_dev_state_delete(xs);
> out:
> spin_lock_bh(&bond->ipsec_lock);
> list_for_each_entry(ipsec, &bond->ipsec_list, list) {
> @@ -558,6 +564,7 @@ static void bond_ipsec_del_sa(struct xfrm_state *xs)
> static void bond_ipsec_del_sa_all(struct bonding *bond)
> {
> struct net_device *bond_dev = bond->dev;
> + struct net_device *real_dev;
> struct bond_ipsec *ipsec;
> struct slave *slave;
>
> @@ -568,21 +575,22 @@ static void bond_ipsec_del_sa_all(struct bonding *bond)
> return;
> }
>
> + real_dev = slave->dev;
> spin_lock_bh(&bond->ipsec_lock);
> list_for_each_entry(ipsec, &bond->ipsec_list, list) {
> if (!ipsec->xs->xso.real_dev)
> continue;
>
> - if (!slave->dev->xfrmdev_ops ||
> - !slave->dev->xfrmdev_ops->xdo_dev_state_delete ||
> - netif_is_bond_master(slave->dev)) {
> - slave_warn(bond_dev, slave->dev,
> + if (!real_dev->xfrmdev_ops ||
> + !real_dev->xfrmdev_ops->xdo_dev_state_delete ||
> + netif_is_bond_master(real_dev)) {
> + slave_warn(bond_dev, real_dev,
> "%s: no slave xdo_dev_state_delete\n",
> __func__);
> } else {
> - slave->dev->xfrmdev_ops->xdo_dev_state_delete(ipsec->xs);
> - if (slave->dev->xfrmdev_ops->xdo_dev_state_free)
> - slave->dev->xfrmdev_ops->xdo_dev_state_free(ipsec->xs);
> + real_dev->xfrmdev_ops->xdo_dev_state_delete(ipsec->xs);
> + if (real_dev->xfrmdev_ops->xdo_dev_state_free)
> + real_dev->xfrmdev_ops->xdo_dev_state_free(ipsec->xs);
> }
> ipsec->xs->xso.real_dev = NULL;
> }
> --
> 2.44.0
>
Reviewed-by: Hangbin Liu <liuhangbin@...il.com>
Powered by blists - more mailing lists