netdev - Re: [PATCH v2] net: ethernet: aquantia: set net

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:   Sat, 11 Mar 2017 00:19:55 +0300
From:   Pavel Belous <pavel.belous@...antia.com>
To:     David Arcari <darcari@...hat.com>,
        Lino Sanfilippo <LinoSanfilippo@....de>, netdev@...r.kernel.org
Subject: Re: [PATCH v2] net: ethernet: aquantia: set net_device mtu when mtu
 is changed



On 10.03.2017 17:47, David Arcari wrote:
> Hi,
>
> On 03/09/2017 05:43 PM, Lino Sanfilippo wrote:
>> Hi,
>>
>> On 09.03.2017 22:03, David Arcari wrote:
>>> When the aquantia device mtu is changed the net_device structure is not
>>> updated.  As a result the ip command does not properly reflect the mtu change.
>>>
>>> Commit 5513e16421cb incorrectly assumed that __dev_set_mtu() was making the
>>> assignment ndev->mtu = new_mtu;  This is not true in the case where the driver
>>> has a ndo_change_mtu routine.
>>>
>>> Fixes: 5513e16421cb ("net: ethernet: aquantia: Fixes for aq_ndev_change_mtu")
>>>
>>> v2: no longer close/open net-device after mtu change
>>>
>>> Cc: Pavel Belous <Pavel.Belous@...antia.com>
>>> Signed-off-by: David Arcari <darcari@...hat.com>
>>> ---
>>>  drivers/net/ethernet/aquantia/atlantic/aq_main.c | 10 ++--------
>>>  1 file changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>> index dad6362..bba5ebd 100644
>>> --- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>> @@ -96,15 +96,9 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
>>>  	struct aq_nic_s *aq_nic = netdev_priv(ndev);
>>>  	int err = aq_nic_set_mtu(aq_nic, new_mtu + ETH_HLEN);
>>>
>>> -	if (err < 0)
>>> -		goto err_exit;
>>> +	if (!err)
>>> +		ndev->mtu = new_mtu;
>>>
>>> -	if (netif_running(ndev)) {
>>> -		aq_ndev_close(ndev);
>>> -		aq_ndev_open(ndev);
>>> -	}
>>> -
>>> -err_exit:
>>
>> Removing the restart has nothing to do with the bug you want to fix here, has it?
>> I suggest to send a separate patch for this.
>>
>> Regards,
>> Lino
>>
>
> I'm fine with that.  Pavel does that work for you?
>
> It would mean that the original version of this patch should be applied and
> either you or I could send the follow-up patch.
>
> Best,
>
> -Dave
>

David,

Yes, I think for this it is better to make separate patch.
I can prepare a patch for "close/open netdev" myself.

Probably you need send the original version of this patch as "v3" (I'm 
no sure what it is possible to discard v2 and apply v1 instead.)

Thank you,
Pavel