[PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Arcari]

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")

Cc: Pavel Belous <Pavel.Belous@aquantia.com>
Signed-off-by: David Arcari <darcari@redhat.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index dad6362..d05fbfd 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -98,6 +98,7 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
 
 	if (err < 0)
 		goto err_exit;
+	ndev->mtu = new_mtu;
 
 	if (netif_running(ndev)) {
 		aq_ndev_close(ndev);
-- 
1.8.3.1

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[Pavel Belous]

On 09.03.2017 00:33, 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")
>
> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
> Signed-off-by: David Arcari <darcari@redhat.com>
> ---
>  drivers/net/ethernet/aquantia/atlantic/aq_main.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
> index dad6362..d05fbfd 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
> @@ -98,6 +98,7 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
>
>  	if (err < 0)
>  		goto err_exit;
> +	ndev->mtu = new_mtu;
>
>  	if (netif_running(ndev)) {
>  		aq_ndev_close(ndev);
>

Thank you, David.

I think we should also remove closing/opening net-device after mtu changed.

Regards,
Pavel

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Arcari]

On 03/09/2017 02:02 PM, Pavel Belous wrote:
> 
> 
> On 09.03.2017 00:33, 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")
>>
>> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
>> Signed-off-by: David Arcari <darcari@redhat.com>
>> ---
>>  drivers/net/ethernet/aquantia/atlantic/aq_main.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>> b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>> index dad6362..d05fbfd 100644
>> --- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>> @@ -98,6 +98,7 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int
>> new_mtu)
>>
>>      if (err < 0)
>>          goto err_exit;
>> +    ndev->mtu = new_mtu;
>>
>>      if (netif_running(ndev)) {
>>          aq_ndev_close(ndev);
>>
> 
> Thank you, David.
> 
> I think we should also remove closing/opening net-device after mtu changed.

Hi Pavel,

I'll go ahead and submit v2.

Thanks,

-Dave

> 
> Regards,
> Pavel

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Arcari]

On 03/09/2017 03:01 PM, David Arcari wrote:
> On 03/09/2017 02:02 PM, Pavel Belous wrote:
>>
>>
>> On 09.03.2017 00:33, 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")
>>>
>>> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
>>> Signed-off-by: David Arcari <darcari@redhat.com>
>>> ---
>>>  drivers/net/ethernet/aquantia/atlantic/aq_main.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>> b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>> index dad6362..d05fbfd 100644
>>> --- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>> @@ -98,6 +98,7 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int
>>> new_mtu)
>>>
>>>      if (err < 0)
>>>          goto err_exit;
>>> +    ndev->mtu = new_mtu;
>>>
>>>      if (netif_running(ndev)) {
>>>          aq_ndev_close(ndev);
>>>
>>
>> Thank you, David.
>>
>> I think we should also remove closing/opening net-device after mtu changed.
> 
> Hi Pavel,
> 
> I'll go ahead and submit v2.
> 
> Thanks,
> 
> -Dave

Hi Pavel,

Before I post v2, won't that mean that if the interface is up that the user will
have to manually toggle it for the new mtu to take effect?

Is that the desired behavior?

Thanks,

-Dave

> 
>>
>> Regards,
>> Pavel
> 

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[Pavel Belous]

On 09.03.2017 23:28, David Arcari wrote:
> On 03/09/2017 03:01 PM, David Arcari wrote:
>> On 03/09/2017 02:02 PM, Pavel Belous wrote:
>>>
>>>
>>> On 09.03.2017 00:33, 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")
>>>>
>>>> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
>>>> Signed-off-by: David Arcari <darcari@redhat.com>
>>>> ---
>>>>  drivers/net/ethernet/aquantia/atlantic/aq_main.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>>> b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>>> index dad6362..d05fbfd 100644
>>>> --- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>>> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
>>>> @@ -98,6 +98,7 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int
>>>> new_mtu)
>>>>
>>>>      if (err < 0)
>>>>          goto err_exit;
>>>> +    ndev->mtu = new_mtu;
>>>>
>>>>      if (netif_running(ndev)) {
>>>>          aq_ndev_close(ndev);
>>>>
>>>
>>> Thank you, David.
>>>
>>> I think we should also remove closing/opening net-device after mtu changed.
>>
>> Hi Pavel,
>>
>> I'll go ahead and submit v2.
>>
>> Thanks,
>>
>> -Dave
>
> Hi Pavel,
>
> Before I post v2, won't that mean that if the interface is up that the user will
> have to manually toggle it for the new mtu to take effect?
>
> Is that the desired behavior?
>
> Thanks,
>
> -Dave
>
>>
>>>
>>> Regards,
>>> Pavel
>>
>

No. The new mtu will take effect immediately.
Closing/opening is not needed at all.


Regards,
Pavel

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Miller]

From: David Arcari <darcari@redhat.com>
Date: Wed,  8 Mar 2017 16:33:21 -0500

> 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")
> 
> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
> Signed-off-by: David Arcari <darcari@redhat.com>

Applied, thanks.

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Arcari]

On 03/13/2017 02:09 AM, David Miller wrote:
> From: David Arcari <darcari@redhat.com>
> Date: Wed,  8 Mar 2017 16:33:21 -0500
> 
>> 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")
>>
>> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
>> Signed-off-by: David Arcari <darcari@redhat.com>
> 
> Applied, thanks.
> 

Hi David,

I see that my patch:

"net: ethernet: aquantia: call set_irq_affinity_hint before free_irq"

has been applied to net, but I don't see that this patch has been applied.

Is there any chance that you replied to the wrong patch?  Or am I missing something?

Thanks,

-Dave

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Miller]

From: David Arcari <darcari@redhat.com>
Date: Mon, 13 Mar 2017 11:50:50 -0400

> On 03/13/2017 02:09 AM, David Miller wrote:
>> From: David Arcari <darcari@redhat.com>
>> Date: Wed,  8 Mar 2017 16:33:21 -0500
>> 
>>> 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")
>>>
>>> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
>>> Signed-off-by: David Arcari <darcari@redhat.com>
>> 
>> Applied, thanks.
>> 
> 
> Hi David,
> 
> I see that my patch:
> 
> "net: ethernet: aquantia: call set_irq_affinity_hint before free_irq"
> 
> has been applied to net, but I don't see that this patch has been applied.

It is marked as "changes requested" in patchwork, because you were asked to
do the restart label removal in a separate patch.

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Arcari]

On 03/13/2017 03:56 PM, David Miller wrote:
> From: David Arcari <darcari@redhat.com>
> Date: Mon, 13 Mar 2017 11:50:50 -0400
> 
>> On 03/13/2017 02:09 AM, David Miller wrote:
>>> From: David Arcari <darcari@redhat.com>
>>> Date: Wed,  8 Mar 2017 16:33:21 -0500
>>>
>>>> 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")
>>>>
>>>> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
>>>> Signed-off-by: David Arcari <darcari@redhat.com>
>>>
>>> Applied, thanks.
>>>
>>
>> Hi David,
>>
>> I see that my patch:
>>
>> "net: ethernet: aquantia: call set_irq_affinity_hint before free_irq"
>>
>> has been applied to net, but I don't see that this patch has been applied.
> 
> It is marked as "changes requested" in patchwork, because you were asked to
> do the restart label removal in a separate patch.
> 


Sorry, I was not clear.  I was trying to ask if your "Applied, thanks" reply
(above) was meant for this email thread.  I believe it was meant for the
set_irq_affinity_hint patch.

Thanks,

-DA

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[Jarod Wilson]

On 2017-03-08 4:33 PM, 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")
>
> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
> Signed-off-by: David Arcari <darcari@redhat.com>

Looks like an appropriate and necessary fix to me.

Reviewed-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Miller]

From: David Arcari <darcari@redhat.com>
Date: Mon, 13 Mar 2017 19:07:16 -0400

> 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")
> 
> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
> Signed-off-by: David Arcari <darcari@redhat.com>

Applied.

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[Jarod Wilson]

On 2017-03-13 7:07 PM, 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")
>
> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
> Signed-off-by: David Arcari <darcari@redhat.com>

Would help if I replied to the proper version of the patch. To 
reiterate, patch looks appropriate and necessary.

Reviewed-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[Pavel Belous]

On 03/14/2017 02:07 AM, 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")
>
> Cc: Pavel Belous <Pavel.Belous@aquantia.com>
> Signed-off-by: David Arcari <darcari@redhat.com>
> ---
>  drivers/net/ethernet/aquantia/atlantic/aq_main.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
> index dad6362..d05fbfd 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
> @@ -98,6 +98,7 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
>
>  	if (err < 0)
>  		goto err_exit;
> +	ndev->mtu = new_mtu;
>
>  	if (netif_running(ndev)) {
>  		aq_ndev_close(ndev);
>

Tested-by: Pavel Belous <pavel.belous@aquantia.com>

[PATCH] net: ethernet: aquantia: set net_device mtu when mtu is changed

[David Arcari]

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")

Cc: Pavel Belous <Pavel.Belous@aquantia.com>
Signed-off-by: David Arcari <darcari@redhat.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index dad6362..d05fbfd 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -98,6 +98,7 @@ static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
 
 	if (err < 0)
 		goto err_exit;
+	ndev->mtu = new_mtu;
 
 	if (netif_running(ndev)) {
 		aq_ndev_close(ndev);
-- 
1.8.3.1