netdev - Re: [PATCH next] bonding: fix wq initialization for links created via netlink

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] [day] [month] [year] [list]

Date:   Thu, 20 Apr 2017 11:44:28 -0700
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Joe Stringer <joe@....org>
Cc:     Mahesh Bandewar <mahesh@...dewar.net>,
        Jay Vosburgh <j.vosburgh@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        Veaceslav Falico <vfalico@...il.com>,
        Nikolay Aleksandrov <nikolay@...hat.com>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH next] bonding: fix wq initialization for links created via netlink

On Thu, Apr 20, 2017 at 11:06 AM, Joe Stringer <joe@....org> wrote:
> On 19 April 2017 at 17:30, Mahesh Bandewar <mahesh@...dewar.net> wrote:
>> From: Mahesh Bandewar <maheshb@...gle.com>
>>
[...]
>>
>> Fixes: 4493b81bea ("bonding: initialize work-queues during creation of bond")
>> Reported-by: Joe Stringer <joe@....org>
>> Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
>> ---
>
> Thanks Mahesh, this fixes the issue I was observing.
>
> Tested-by: Joe Stringer <joe@....org>
>
Thanks for the confirmation Joe.

>>  drivers/net/bonding/bond_main.c    | 2 +-
>>  drivers/net/bonding/bond_netlink.c | 5 +++++
>>  include/net/bonding.h              | 1 +
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 6bd3b50faf48..e549bf6f5cac 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -3243,7 +3243,7 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
>>
>>  /*-------------------------- Device entry points ----------------------------*/
>>
>> -static void bond_work_init_all(struct bonding *bond)
>> +void bond_work_init_all(struct bonding *bond)
>>  {
>>         INIT_DELAYED_WORK(&bond->mcast_work,
>>                           bond_resend_igmp_join_requests_delayed);
>> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>> index b8df0f5e8c25..31b9f37c7dbd 100644
>> --- a/drivers/net/bonding/bond_netlink.c
>> +++ b/drivers/net/bonding/bond_netlink.c
>> @@ -449,6 +449,11 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev,
>>         err = register_netdevice(bond_dev);
>>
>>         netif_carrier_off(bond_dev);
>> +       if (err >= 0) {
>
> register_netdevice() returns 0 on success according to its
> documentation, does it need to handle positive err values?
>
Yes, that's not needed and could cause problems. Will update the patch.

>> +               struct bonding *bond = netdev_priv(bond_dev);
>> +
>> +               bond_work_init_all(bond);
>> +       }
>>
>>         return err;
>>  }
>> diff --git a/include/net/bonding.h b/include/net/bonding.h
>> index 04a21e8048be..b00508d22e0a 100644
>> --- a/include/net/bonding.h
>> +++ b/include/net/bonding.h
>> @@ -614,6 +614,7 @@ struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
>>                                               int level);
>>  int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
>>  void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay);
>> +void bond_work_init_all(struct bonding *bond);
>>
>>  #ifdef CONFIG_PROC_FS
>>  void bond_create_proc_entry(struct bonding *bond);
>> --
>> 2.12.2.816.g2cccc81164-goog
>>