[ghost]

• Implementing of configuration of mtu for host vlans.

Signed-off-by: Maksym Belei Maksym_Belei@jabil.com

What I did

Implemented possibility to set mtu for Host Vlans.

What exactly the fix does:
Update sequence is briefly described below:

1. Function updateHostVlanMtu reads mtu field from configuration of the vlan.
2. If it is absent, the function uses default mtu value(9100).
3. Then the function reads the list of members of the vlan from redis and iterates through them.
4. During iteration updateHostVlanMtu reads mtus of the members from config db and searches the lowest value in them.
5. Then checks whether the desired value of mtu is greater, than lowest value from members. If so, mtu value from members will be set to vlan, otherwise, desired value will be set.

The mtu updating process above will be performed in the next cases:

1. mtu field has set/changed in config table of the vlan(function doVlanTask);
2. a member of the vlan has added/removed(function doVlanMemberTask);
3. mtu of a member of the vlan has changed(function doVlanMemberUpdateTask).

Additional prerequirements:

1. Only members which have status ok in STATE_DB will be used for checking of mtu;
2. Update of mtu for the vlan will be performed only if the vlan has at least 1 member configured and state of the vlan in STATE_DB is ok.

Why I did it

To be able to set mtu to Host Vlan.

How I verified it

Details if related

[prsunny]

@maksymbelei95 , can we have some VS test to cover this? Seems its significant amount of changes to handle the mtu config.

[ghost]

@prsunny, sure, I can add new unit tests, related to mtu changes for vlan. Before that I just want to ask you to confirm that the idea of my changes is right to not to waste my time for unit tests developing in case if my understanding of the problem is wrong. If changes in the PR looks good to you, I will procced with developing of the related unit tests.

By the way, I have one unclear point regarding the expected behavior on mtu change of Host Vlan. Should vlanmgr update the related table in APPL_DB on change of MTU of the Host Vlan, like it makes here https://github.com/Azure/sonic-swss/blob/master/cfgmgr/vlanmgr.cpp#L387? Could you help me with this?

[prsunny]

@prsunny, sure, I can add new unit tests, related to mtu changes for vlan. Before that I just want to ask you to confirm that the idea of my changes is right to not to waste my time for unit tests developing in case if my understanding of the problem is wrong. If changes in the PR looks good to you, I will procced with developing of the related unit tests.

By the way, I have one unclear point regarding the expected behavior on mtu change of Host Vlan. Should vlanmgr update the related table in APPL_DB on change of MTU of the Host Vlan, like it makes here https://github.com/Azure/sonic-swss/blob/master/cfgmgr/vlanmgr.cpp#L387? Could you help me with this?

Sure @maksymbelei95. Let me first do the review on the approach, before developing unit-tests

[dgsudharsan]

@maksymbelei95 I believe the entire logic was under the assumption that a port can be part of only one VLAN. In general a port can be part of multiple VLANs and a VLAN can have multiple members. So this logic wouldn't work when a port or portchannel is part of multiple VLANs

[prsunny]

@maksymbelei95 , it looks like an over-complicated approach and as @dgsudharsan mentioned, this is not covering all scenarios. My suggestion is to keep it simple and just update the mtu for the Vlan interface based on user config irrespective of the individual ports mtu. Do you see an issue/gap with that?

[ghost]

@prsunny, @dgsudharsan, sorry for the late response.

So this logic wouldn't work when a port or portchannel is part of multiple VLANs
Thank you for review. It is my gap and I know how to fix it.

My suggestion is to keep it simple and just update the mtu for the Vlan interface based on user config irrespective of the individual ports mtu. Do you see an issue/gap with that?
I agree with you that the mechanism is pretty complex.
As I mentioned before and according to mentions in the code, MTU of the Vlan should be less or equal to MTU of its members. If it is ok to make the end user to be responsible to set correct value of MTU of the vlan, I will update the fix to simplify it, but with the next suggestion. I recommend to leave checking whether the VLAN has a member or not before setting the MTU, because only for Host Vlan which has at least one member the MTU can be successfully set. Otherwise, record of the Host Vlan will not be found in the system.