lag MTU setting fix - fix for #18695

[matiAlfaro]

What I did

Fix for #18695

How I did it

When creating PortChannel, don't set MTU
MTU is set according to first port inserted to PortChannel.
When last port is removed, set PortChannel MTU back to None

How to verify it

Tested with new test added to UT

Previous command output (if the output of a command-line utility has changed)

Before this fix, you were not able to add to PortChannel a port with MTU !- 9100 (hard coded)

New command output (if the output of a command-line utility has changed)

PortChannel MTU will be determined according to first port inserted into PortChannel

[mbze430]

sorry, I am not sure how to check it. which version do I download from the pipeline?

[matiAlfaro]

@mbze430 - pipeline doesn't build image for this.
You can either build yourself or wait until this is integrated.

[matiAlfaro]

Hi @prsunny @qiluo-msft, could you please review,
Thanks.

[mbze430]

@matiAlfaro @prsunny @qiluo-msft
I still haven't seen this being merge to the master
https://dev.azure.com/mssonic/build/_build?definitionId=55
when would this be merged?

Once merged can you raise a PR for other builds? like 202311?

[prsunny]

Do we want to support setting MTU on portchannel?

[matiAlfaro]

I would advice against adding configuration to portchannel, this adds complexity, for example

• We'll need to "remember" and restore port configuration when port is removed from portchannel
• When applying configuration on a portchannel, somewhere we'll need to configure all ports
• There's a question of which ports, as there are several "states", the actives, the standby and the non-candidate
• There's also an issue with "show command" or "DB value", if we apply configuration on portchannel, what will we have in port-db (and show command)? it may be different than what is actually configured
  All of these can be addressed and solved, but I'm not sure it's worth the effort

[prsunny]

I would advice against adding configuration to portchannel, this adds complexity, for example

• We'll need to "remember" and restore port configuration when port is removed from portchannel
• When applying configuration on a portchannel, somewhere we'll need to configure all ports
• There's a question of which ports, as there are several "states", the actives, the standby and the non-candidate
• There's also an issue with "show command" or "DB value", if we apply configuration on portchannel, what will we have in port-db (and show command)? it may be different than what is actually configured
  All of these can be addressed and solved, but I'm not sure it's worth the effort

ok, i thought this PR is adding configuration to portchannel to set mtu

[mbze430]

Do we want to support setting MTU on portchannel?

I think this is crucial to have. Even a single link end to end have the ability to define MTU size, why shouldn't a portchannel be allowed to define a MTU size to optimize packet transmission. Specially for devices and applications that support different jumbo frame size.

[matiAlfaro]

Do we want to support setting MTU on portchannel?

I think this is crucial to have. Even a single link end to end have the ability to define MTU size, why shouldn't a portchannel be allowed to define a MTU size to optimize packet transmission. Specially for devices and applications that support different jumbo frame size.

I understand what you are saying, and it is worth looking into.
With the suggested current fix, could you get the functionality you need? if so, then maybe it would be worth to proceed with review (and eventually merge) of this fix. Adding MTU configuration on portchannel can be added later on.

[mbze430]

@matiAlfaro can you explain to me what exactly your solution is? My assumption right now is that the portchannel's MTU will be set to the first Eithernet port becomes the member of a portchannel?

ex: if Ethernet102 is set to MTU 5120 when creating "PortChannel001" and adding Ethernet102 to PortChannel001 it will automatically make PortChannel001 MTU 5120?

If that is the case, that is fine.... for now..

But if in the future PortChannel001 needs to be change to say 9216. It would be a BIG hassle to remove all the Ethernet ports in the PortChannel001 and re do everything, specially if there are like 3+ Ethernet ports.

Hope that make sense

[matiAlfaro]

@mbze430, yes - that's what I'm suggesting.
Trying to get from you, and others, feedback if this kind of solution is acceptable.
If not, and the only acceptable approach would be to allow configuration on portchannel, this will probably need to be handled as a feature, mainly meaning HLD and some additional tests (in PTF)
@prsunny, what do you think?