On Thu, Mar 22, 2012 at 4:17 PM, Ben Pfaff <b...@nicira.com> wrote:
> 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 <pshe...@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)"?
>
right, I will fix patch.
> 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.
>
ok, I will post separate patch for this.
> Thanks,
>
> Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
