Why vlan_dev can not follow real_dev mtu change from smaller to bigger

Why vlan_dev can not follow real_dev mtu change from smaller to bigger

[Ziyang Xuan (William)]

Hello,

Recently, I did some tests about mtu change for vlan device
and real_dev. I found that vlan_dev's mtu could not follow its
real_dev's mtu change from smaller to bigger.

For example:
Firstly, change real_dev's mtu from 1500 to 256, vlan_dev's mtu
follow change from 1500 to 256.
Secondly, change real_dev's mtu from 256 to 1500, but vlan_dev's
mtu is still 256.

I fond the code as following. But I could not understand the
limitations. Is there anyone can help me?

static int vlan_device_event(struct notifier_block *unused, unsigned long event,
			     void *ptr)
{
	...

	case NETDEV_CHANGEMTU:
		vlan_group_for_each_dev(grp, i, vlandev) {
			if (vlandev->mtu <= dev->mtu)
				continue;

			dev_set_mtu(vlandev, dev->mtu);
		}
		break;
	...
}

Thank you for your reply.

Re: Why vlan_dev can not follow real_dev mtu change from smaller to bigger

[Ziyang Xuan (William)]

> Hello,
> 
> Recently, I did some tests about mtu change for vlan device
> and real_dev. I found that vlan_dev's mtu could not follow its
> real_dev's mtu change from smaller to bigger.
> 
> For example:
> Firstly, change real_dev's mtu from 1500 to 256, vlan_dev's mtu
> follow change from 1500 to 256.
> Secondly, change real_dev's mtu from 256 to 1500, but vlan_dev's
> mtu is still 256.
> 
> I fond the code as following. But I could not understand the
> limitations. Is there anyone can help me?
> 
> static int vlan_device_event(struct notifier_block *unused, unsigned long event,
> 			     void *ptr)
> {
> 	...
> 
> 	case NETDEV_CHANGEMTU:
> 		vlan_group_for_each_dev(grp, i, vlandev) {
> 			if (vlandev->mtu <= dev->mtu)
> 				continue;
> 
> 			dev_set_mtu(vlandev, dev->mtu);
> 		}
> 		break;
> 	...
> }
> 
> Thank you for your reply.
> 
commit: 2e477c9bd2bb ("vlan: Propagate physical MTU changes") wanted
to ensure that all existing VLAN device MTUs do not exceed the new
underlying MTU. Make VLAN device MTUs equal to the new underlying MTU
follow this rule. So I think the following modification is reasonable
and can solve the above problem.

@@ -418,12 +418,8 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
                break;

        case NETDEV_CHANGEMTU:
-               vlan_group_for_each_dev(grp, i, vlandev) {
-                       if (vlandev->mtu <= dev->mtu)
-                               continue;
-
+               vlan_group_for_each_dev(grp, i, vlandev)
                        dev_set_mtu(vlandev, dev->mtu);
-               }
                break;

Are there any other different operations?

Thanks.

Re: Why vlan_dev can not follow real_dev mtu change from smaller to bigger

[David Ahern]

On 2/16/22 11:44 PM, Ziyang Xuan (William) wrote:
>> Hello,
>>
>> Recently, I did some tests about mtu change for vlan device
>> and real_dev. I found that vlan_dev's mtu could not follow its
>> real_dev's mtu change from smaller to bigger.
>>
>> For example:
>> Firstly, change real_dev's mtu from 1500 to 256, vlan_dev's mtu
>> follow change from 1500 to 256.
>> Secondly, change real_dev's mtu from 256 to 1500, but vlan_dev's
>> mtu is still 256.
>>
>> I fond the code as following. But I could not understand the
>> limitations. Is there anyone can help me?
>>
>> static int vlan_device_event(struct notifier_block *unused, unsigned long event,
>> 			     void *ptr)
>> {
>> 	...
>>
>> 	case NETDEV_CHANGEMTU:
>> 		vlan_group_for_each_dev(grp, i, vlandev) {
>> 			if (vlandev->mtu <= dev->mtu)
>> 				continue;
>>
>> 			dev_set_mtu(vlandev, dev->mtu);
>> 		}
>> 		break;
>> 	...
>> }
>>
>> Thank you for your reply.
>>
> commit: 2e477c9bd2bb ("vlan: Propagate physical MTU changes") wanted
> to ensure that all existing VLAN device MTUs do not exceed the new
> underlying MTU. Make VLAN device MTUs equal to the new underlying MTU
> follow this rule. So I think the following modification is reasonable
> and can solve the above problem.
> 
> @@ -418,12 +418,8 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
>                 break;
> 
>         case NETDEV_CHANGEMTU:
> -               vlan_group_for_each_dev(grp, i, vlandev) {
> -                       if (vlandev->mtu <= dev->mtu)
> -                               continue;
> -
> +               vlan_group_for_each_dev(grp, i, vlandev)
>                         dev_set_mtu(vlandev, dev->mtu);
> -               }
>                 break;
> 

vlans must work within the mtu of the underlying device, so shrinking
the mtu of the vlan device when the real device changes is appropriate.

vlan devices do not necessarily have to operate at the same mtu as the
real device, so when the real device mtu is increased it might not be
appropriate to raise the mtu of the vlan devices. Admin needs to manage
that.

Re: Why vlan_dev can not follow real_dev mtu change from smaller to bigger

[Ziyang Xuan (William)]

> On 2/16/22 11:44 PM, Ziyang Xuan (William) wrote:
>>> Hello,
>>>
>>> Recently, I did some tests about mtu change for vlan device
>>> and real_dev. I found that vlan_dev's mtu could not follow its
>>> real_dev's mtu change from smaller to bigger.
>>>
>>> For example:
>>> Firstly, change real_dev's mtu from 1500 to 256, vlan_dev's mtu
>>> follow change from 1500 to 256.
>>> Secondly, change real_dev's mtu from 256 to 1500, but vlan_dev's
>>> mtu is still 256.
>>>
>>> I fond the code as following. But I could not understand the
>>> limitations. Is there anyone can help me?
>>>
>>> static int vlan_device_event(struct notifier_block *unused, unsigned long event,
>>> 			     void *ptr)
>>> {
>>> 	...
>>>
>>> 	case NETDEV_CHANGEMTU:
>>> 		vlan_group_for_each_dev(grp, i, vlandev) {
>>> 			if (vlandev->mtu <= dev->mtu)
>>> 				continue;
>>>
>>> 			dev_set_mtu(vlandev, dev->mtu);
>>> 		}
>>> 		break;
>>> 	...
>>> }
>>>
>>> Thank you for your reply.
>>>
>> commit: 2e477c9bd2bb ("vlan: Propagate physical MTU changes") wanted
>> to ensure that all existing VLAN device MTUs do not exceed the new
>> underlying MTU. Make VLAN device MTUs equal to the new underlying MTU
>> follow this rule. So I think the following modification is reasonable
>> and can solve the above problem.
>>
>> @@ -418,12 +418,8 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
>>                 break;
>>
>>         case NETDEV_CHANGEMTU:
>> -               vlan_group_for_each_dev(grp, i, vlandev) {
>> -                       if (vlandev->mtu <= dev->mtu)
>> -                               continue;
>> -
>> +               vlan_group_for_each_dev(grp, i, vlandev)
>>                         dev_set_mtu(vlandev, dev->mtu);
>> -               }
>>                 break;
>>
> 
> vlans must work within the mtu of the underlying device, so shrinking
> the mtu of the vlan device when the real device changes is appropriate.
> 
> vlan devices do not necessarily have to operate at the same mtu as the
> real device, so when the real device mtu is increased it might not be
> appropriate to raise the mtu of the vlan devices. Admin needs to manage
> that.
> .

I have submitted a patch for the problem.

https://patchwork.kernel.org/project/netdevbpf/patch/20220221124644.1146105-1-william.xuanziyang@huawei.com/

Allow vlan device follow real device MTU change from smaller to bigger
when user has not configured vlan device MTU. That also ensure user
configuration has higher priority.

Welcome to review.