[ovs-dev] [PATCH] ofproto: Do not override internal port MTU.

[Thadeu Lima de Souza Cascardo]

On Thu, Sep 01, 2016 at 05:17:44PM +0000, Daniele Di Proietto wrote:
> Let me try to sum up the problem further
> 
> 1) Behavior on master, before commit 47bf118665a3("ofproto: Always set MTU for new internal ports."):
> 
>     a) When an internal interface is added, or its MTU has changed we only change its MTU if it is bigger than the bridge minimum
>     b) When a non internal, non tunnel port is added, we reconfigure every internal interface to match exactly the bridge minimum, no matter what
>     c) If the bridge doesn't have physical ports (only tunnels and internals), we consider the minimum mtu to be invalid and we don't change the internal interfaces mtu
> 
> I felt that there was an inconsistency between 1a and 1b.  I thought 1c was not very clean, because the current configuration depends on the previous state (if there were physical ports at some point, we would lower the internal interfaces mtu, but we couldn't restore it when they were removed)
> 
> 2) Behavior on master after commit 47bf118665a3("ofproto: Always set MTU for new internal ports."):
> 
>     a) When an internal interface is added, or its MTU has changed we overwrite its MTU with the bridge minimum.
>     b) When a non internal, non tunnel port is added, we reconfigure every internal interface to match exactly the bridge minimum, no matter what
>     c) If the bridge doesn't have physical ports (only tunnels and internals), we consider the minimum mtu to be 1500.  We therefore force every internal interface to be 1500.
> 
> Behavior b is the same.  Behavior a is changed to match more closely behavior b.  Behavior c is changed completely.  Now the current configuration doesn't depend on the previous state.
> When I made the change I didn't think that there were valid use cases for allowing the user to configure internal interfaces MTU.  The test suite obviously proved me wrong.
> 
> ---------
> 
> Proposed solution:
> 
> A) This patch.
> 
>    Since OVS is not able the control the MTU in case of tunnelling or MPLS, stop doing it entirely.  The systems that relied on the old behavior need to be updated.
> 
> B) We could consider an hybrid solution that keeps backwards compatibility for tunnelling use cases (like our testsuite). 
> 
>    2a)
>    2b or 1b)
>    1c)
> 
> 
> 
> 
>    This would not solve the problem for MPLS.  MPLS uses physical devices, so the internal interface would be forced to match the physical interfaces and this is not OK for MPLS (or double vlans).  Also, this solution keeps behavior 1c, which makes the mtu assignment "stateful".
> 
> Other ideas?
> 
> Thanks,
> 
> Daniele


How about we let the interfaces to have a less than minimum MTU? Would that
solve the problem with tunnels/overlays?

Then, we partially revert the patch, like below. This has fixed at least the
VXLAN system test for me.

Cascardo.

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9d62b72..6383dc8 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2752,8 +2752,10 @@ update_mtu(struct ofproto *p, struct ofport *port)
         return;
     }
     if (ofport_is_internal(p, port)) {
-        if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
-            dev_mtu = p->min_mtu;
+        if (dev_mtu > p->min_mtu) {
+            if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
+                dev_mtu = p->min_mtu;
+            }
         }
         port->mtu = dev_mtu;
         return;