netdev - Re: [PATCH net-next 09/12] ionic: change mtu without full queue rebuild

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]

Message-ID: <cbcedcf2-f199-1d10-4169-7605cb55794f@pensando.io>
Date:   Wed, 26 Aug 2020 17:04:49 -0700
From:   Shannon Nelson <snelson@...sando.io>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH net-next 09/12] ionic: change mtu without full queue
 rebuild

On 8/26/20 2:09 PM, Jakub Kicinski wrote:
> On Wed, 26 Aug 2020 09:42:11 -0700 Shannon Nelson wrote:
>> +	mutex_lock(&lif->queue_lock);
>> +	netif_device_detach(lif->netdev);
>> +	ionic_stop_queues(lif);
>> +	ionic_txrx_deinit(lif);
>>   
>> +	err = ionic_txrx_init(lif);
>> +	if (err)
>> +		goto err_out;
>> +
>> +	/* don't start the queues until we have link */
>> +	if (netif_carrier_ok(netdev)) {
>> +		err = ionic_start_queues(lif);
>> +		if (err)
>> +			goto err_out;
>> +	}
>> +
>> +err_out:
>> +	netif_device_attach(lif->netdev);
>> +	mutex_unlock(&lif->queue_lock);
> Looks a little racy, since the link state is changed before queue_lock
> is taken:
>
>                  if (!netif_carrier_ok(netdev)) { u32 link_speed;
>                          ionic_port_identify(lif->ionic);
>                          link_speed = le32_to_cpu(lif->info->status.link_speed);
>                          netdev_info(netdev, "Link up - %d Gbps\n",
>                                      link_speed / 1000);
>                          netif_carrier_on(netdev);
>                  }
>                                                                                  
>                  if (lif->netdev->flags & IFF_UP && netif_running(lif->netdev)) \
> {
>                          mutex_lock(&lif->queue_lock);
>                          ionic_start_queues(lif);
>                          mutex_unlock(&lif->queue_lock);
>                  }

Yeah, that would probably be better served to just call 
ionic_link_status_check_request() here and let it do the job.
sln