netdev - Re: [PATCH net-next v6 2/2] bonding: Simplify the xmit function for modes that use xmit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 1 Oct 2014 21:40:39 -0700
From:	Cong Wang <cwang@...pensource.com>
To:	Mahesh Bandewar <maheshb@...gle.com>
Cc:	Jay Vosburgh <j.vosburgh@...il.com>,
	Veaceslav Falico <vfalico@...hat.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	Maciej Zenczykowski <maze@...gle.com>
Subject: Re: [PATCH net-next v6 2/2] bonding: Simplify the xmit function for
 modes that use xmit_hash

On Wed, Oct 1, 2014 at 1:38 AM, Mahesh Bandewar <maheshb@...gle.com> wrote:
> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
> +{
> +       struct slave *slave;
> +       struct list_head *iter;
> +       struct bond_up_slave *new_arr, *old_arr;
> +       int slaves_in_agg;
> +       int agg_id = 0;
> +       int ret = 0;
> +
> +#ifdef CONFIG_LOCKDEP
> +       WARN_ON(lockdep_is_held(&bond->mode_lock));
> +#endif


I think you can use lockdep_is_held().

> +
> +       new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
> +                         GFP_KERNEL);
> +       if (!new_arr) {
> +               ret = -ENOMEM;
> +               pr_err("Failed to build slave-array.\n");
> +               goto out;
> +       }


No need to print an error message for OOM, it is already noisy. :)


> +       if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> +               struct ad_info ad_info;
> +
> +               if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
> +                       pr_debug("bond_3ad_get_active_agg_info failed\n");


I suspect how useful this debug info is since your patch is almost ready
to merge.

> +                       kfree_rcu(new_arr, rcu);
> +                       /* No active aggragator means its not safe to use

s/its/it's/

> +                        * 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);
> +                       }
> +                       goto out;
> +               }
> +               slaves_in_agg = ad_info.ports;
> +               agg_id = ad_info.aggregator_id;
> +       }



Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html