ipspace / netlab

Making virtual networking labs suck less
https://netlab.tools
Other
428 stars 63 forks source link

Default IP MTU #177

Closed jbemmel closed 2 years ago

jbemmel commented 2 years ago

See https://github.com/ipspace/netsim-tools/issues/171

I propose the following:

defaults:
 ip_mtu: 9000

(and then we update all device templates to apply this consistently)

Do we need separate ipv4/ipv6 values?

ipspace commented 2 years ago

You caught me on a break ;) I wanted to open this can of worms as well.

The pervasive default seems to be 1500 (or OSPF multi-vendor tests wouldn't run as clearly demonstrated in #171), the only exception I found so far was Cumulus container -- looks like Cumulus takes its values from Linux, and Linux thinks Ethernet VM interfaces have MTU 1500, while Linux bridges or vEth links have MTU 9000 (or whatever).

Unless there are extremely good reasons I'd stick with the supermajority default (1500). We obviously need configurable MTU though, so:

Any objections?

petercrocker commented 2 years ago

Ooh arrr there be dragons! Dangerous. Different vendors have different numbers meaning the same thing. Arista is 9214 while JunOS is 9216 while both mean exactly the same thing. You’d have to normalise them if you want consistency!

ipspace commented 2 years ago

Normalizing is not hard. Our mtu parameter would mean L3 MTU, and would have to be translated by the author of configuration templates into whatever imperial units used by the device.

petercrocker commented 2 years ago

Fair point, just be very clear and upfront in the docs what MTU means, and maybe give examples of the platforms that deviate (Arista = MTU -2, Cisco = MTU -16 iirc). Would need to look them all up again!

jbemmel commented 2 years ago

Some platforms have limitations in the amount of different MTU values that can be supported, while in theory it could be (most intuitively) modeled as a per-link attribute, in practice I think we mainly and most urgently need a system/topology wide default

Hence the proposal under 'defaults' - not sure where that would end up in topology-defaults.yml?

ipspace commented 2 years ago

Fair point, just be very clear and upfront in the docs what MTU means, and maybe give examples of the platforms that deviate (Arista = MTU -2, Cisco = MTU -16 iirc). Would need to look them all up again!

You must be a Junos guy 😉 Cisco always used mtu to mean L3 MTU on all media and added L2 headers as needed... apart from MPLS tags and a few other things 🥴

ipspace commented 2 years ago

Some platforms have limitations in the amount of different MTU values that can be supported, while in theory it could be (most intuitively) modeled as a per-link attribute, in practice I think we mainly and most urgently need a system/topology wide default

OK, and I'm proposing that to be 1500 as that's the assumed default for most devices.

Hence the proposal under 'defaults' - not sure where that would end up in topology-defaults.yml?

I could implement something like #140 for interfaces (add device data that is valid link attribute to interfaces), in which case (after mtu becomes a valid link/interface attribute) you'd just add mtu: 1500 to a device with different default and use mtu on all interfaces in device configuration template.

Makes sense?

petercrocker commented 2 years ago

You must be a Junos guy 😉

I’ve been acquired in to Juniper, acquired in to Cisco, and worked closely with the ALU (pre Nokia!) people, and last $job was Arista & Cumulus so I don’t know what I am any more…

I do like the ability to set a topology-wide MTU just so you know all things are jumbo frames. Aside from that, I don’t have an opinion.

jbemmel commented 2 years ago

ok, so topology-wide default IP mtu of 1500 with the ability to override on a per-node or per-link basis

ipspace commented 2 years ago

I do like the ability to set a topology-wide MTU just so you know all things are jumbo frames. Aside from that, I don’t have an opinion.

Good one. How about another ladder of inheritance when setting interface parameters:

  1. whatever is specified in node data on a link
  2. link parameter
  3. global defaults.interfaces.x parameter
  4. device parameter

I would still leave defaults.interfaces.mtu undefined and use mtu: 1500 on devices with other MTU defaults so we don't end with mtu parameters everywhere on interfaces (Ansible inventory) and configurations. Alternatively, we can decide that text files are cheap (YAML parsing not so much, but CPU cycles are) and go for most-explicit settings ;)

Thoughts on this last bit?

petercrocker commented 2 years ago

Cumulus defaults to 9216 (9000) so we’ll have to code that up if we set default to 1500. It’s not pretty how to do it (Linux setting, not cli) but i’ve got code around for it I can dig out.

ipspace commented 2 years ago

Cumulus defaults to 9216 (9000) so we’ll have to code that up if we set default to 1500. It’s not pretty how to do it (Linux setting, not cli) but i’ve got code around for it I can dig out.

MTU on VX is 1500:

vagrant@cumulus:mgmt:~$ ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master mgmt state UP mode DEFAULT group default qlen 1000
    link/ether 08:4f:a9:00:00:05 brd ff:ff:ff:ff:ff:ff
3: swp1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 52:54:00:cc:21:78 brd ff:ff:ff:ff:ff:ff
4: swp2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 52:54:00:c8:14:c3 brd ff:ff:ff:ff:ff:ff
5: swp3: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 52:54:00:c1:9d:fd brd ff:ff:ff:ff:ff:ff
6: mgmt: <NOARP,MASTER,UP,LOWER_UP> mtu 65536 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether b6:8b:eb:56:4b:a3 brd ff:ff:ff:ff:ff:ff

One less thing to worry about ;)

petercrocker commented 2 years ago

Interesting, I should probably know that! Maybe only on real hardware then. It changed in CL 4.1: https://docs.nvidia.com/networking-ethernet-software/cumulus-linux-41/Whats-New/

jbemmel commented 2 years ago

See https://docs.nvidia.com/networking-ethernet-software/cumulus-linux-50/api/index.html#/interface/updateInterfaceLink

https://github.com/ipspace/netsim-tools/blob/dev/netsim/ansible/templates/initial/cumulus_nvue.j2#L27

mtu: {{ mtu }}

should do it?

ipspace commented 2 years ago

MTU is implemented (documentation in links.md). The order of preference is:

Configuration templates have been modified for IOS, NXOS, EOS, Junos, FRR, Cumulus 4.

@jbemmel @petercrocker @ssasso: Could you please fix applicable config templates. The fix could be as easy as (EOS, NXOS):

{% if l.mtu is defined %}
 mtu {{ l.mtu }}
{% endif %}

Junos adds L2 header to MTU, so:

{% if l.mtu is defined %}
    mtu {{ l.mtu + 14 }};
{% endif %}

Finally, there's IOS, where CSR refuses to configure IP MTU over 1500 bytes and interface MTU below 1500 bytes, so:

{% if l.mtu is defined %}
{%   if min_mtu is defined and min_mtu > l.mtu %}
 ip mtu {{ l.mtu }}
{%   else %}
 mtu {{ l.mtu }}
{%   endif %}
{% endif %}

min_mtu is defined in defaults.devices.csr.node.min_mtu

jbemmel commented 2 years ago

Is global system mtu configured? If not, wondering if that shouldn't be

mtu {{ l.mtu | default( mtu ) }}

Otherwise, how does defaults.interfaces.mtu: 8192 get applied?

ipspace commented 2 years ago

Is global system mtu configured? If not, wondering if that shouldn't be

No. It is assumed that devices have a default of 1500, and that the device mtu is configured to be 1500 if that's not the case. Don't want to have tons of unnecessary mtu statements in interface configs.

Otherwise, how does defaults.interfaces.mtu: 8192 get applied?

https://github.com/ipspace/netsim-tools/commit/fbe0778bcf1873e8642c31e9992889f66f5515b4#diff-7131075444cef5d8012759be5faba987fb6391d829d414e4a2f74fd32f890f9cR145

jbemmel commented 2 years ago

That's my point - without setting the global system default you'll end up with "tons of unnecessary mtu statements"

For SR Linux, I'm thinking we'd need something like this:

/system/mtu
default-port-mtu: {{ defaults.interfaces.mtu + 14 }}
default-l2-mtu: {{ defaults.interfaces.mtu + 14 }}
default-ip-mtu: {{ defaults.interfaces.mtu }}

per link:
{% if l.mtu != defaults.interfaces.mtu %}
mtu {{ l.mtu + 14 }}
{% endif %}
ipspace commented 2 years ago

Hey, that would make a few things even easier. Thank you! How about

I could make that work tomorrow, it's getting late over here.

ipspace commented 2 years ago

Implemented. If a device supports system MTU:

All the necessary magic happens behind the scenes ;), including explicit interface MTU removal when it happens to be equal to system MTU.

jbemmel commented 2 years ago

I think we've beaten this particular animal well beyond death...closing

ssasso commented 2 years ago

(sorry I was away for some days... will try to add vyos / ros between today and tomorrow)

ipspace commented 2 years ago

(sorry I was away for some days... will try to add vyos / ros between today and tomorrow)

Hey, we're not running against any sort of deadline ;) Thanks a million for the PR!