[ovs-dev] [PATCH v2] ofproto: Optimize internal device MTU update

[Ben Pfaff]

On Thu, Mar 22, 2012 at 11:34:34AM -0700, Pravin B Shelar wrote:
> Fixed according to comments from Ben.
> v1-v2:
>      - Do not allow larger mtu on internal device compared to
>        non internal devices.
> 
> --8<--------------------------cut here-------------------------->8--
> 
> Internal device mtu does not influence mtu of other internal devices.
> So skip MTU update to other devices when internal device mtu is changed.
> 
> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>

I think that the comment on update_mtu() deserves an update, because
it does not mention the significance of 'new_port'.

I'm not sure that the following logic in update_mtu() is correct:

    /* For non-internal port find new min mtu. */
    new_port->mtu = dev_mtu;
    p->min_mtu = find_min_mtu(p);
    if (dev_mtu > p->min_mtu) {
        return;
    }

Suppose, for example, that we have non-internal port p0 with MTU 1500
and p1 with MTU 5000, so that p->min_mtu is 1500.  Then p0's MTU goes
up to 9000.  dev_mtu is 9000 and p->min_mtu changes to 5000, so we
should adjust all the internal ports to MTU 5000, but because 9000 >
5000 we just return without adjusting anything.  (Is my logic
correct?)  Perhaps the test should be "if (old_min_mtu !=
new_min_mtu)"?

I don't see anything that reassesses MTU when a port is deleted.
Should it?  If the only low-MTU port gets deleted, then I would
naively expect OVS to raise all the internal device MTUs to the new
minimum MTU.

Thanks,

Ben.