portsorch ports init done flag should means buffer, autoneg, speed, m…

[jipanyang]

…tu, fec and other port level initial config done too.

Signed-off-by: Jipan Yang jipan.yang@alibaba-inc.com

What I did

Make sure physical port config is done before proceeding to other configurations which have dependency on physical port.

Why I did it
Other high level objects like lag, vlan, interface, acl, copp, pfc, mirror, qos, tunnel, route, and etc. check gPortsOrch->isInitDone() before proceeding their tasks.

The change in #515 altered the meaning of gPortsOrch->isInitDone(), and caused the possibility that physical port parameter setting may not be done though gPortsOrch->isInitDone() had become true, that introduced potential instability.

How I verified it

Details if related

[qiluo-msft]

You mentioned 'potential instability', do you have a vs unit test case to cover it? Actually it is hard to understand the instability, could you elaborate? #Closed

[jipanyang]

@qiluo-msft Currently most orches in orchagent wait for gPortsOrch->isInitDone() signal to start their processing.

Port level attributes like pfc_asym, fec mode, admin, mtu, speed, autoneg won't be applied until bufferorch finishes its job.

Before this change, at system initialization phase, all the orches like pfcorch, aclorch, copporch, fdborch, intfsorch, tunnorch, routorch, qosorch will race with bufferorch to run the task once they saw gPortsOrch->isInitDone() is ready.

Then pfc_asym, fec mode, admin, mtu, speed, autoneg may be set after pfcorch, intforch and others have done the first initialization.

Since those port level config is the very fundamental attributes for the whole system config, one example is the mtu, if intforch has used the default mtu to create router interface, the rif mtu has to be changed again to match the configured mtu, it is better to have the right mtu ready at the very beginning to avoid the flapping, especially during system initialization.

I'm not sure whether pfc_asym, fec, speed, autoneg and admin have similar impact on current or even future orch objects. But having a deterministic startup order helps to remove the unnecessary sai operation and avoid potential dependency problems.

#Resolved

[qiluo-msft]

isInitDone [](start = 16, length = 10)

The names of functions and member variables are confusing. Suggest:

bool PortsOrch::isPortReady()
{
    return m_initDone && m_pendingPortSet.empty();
}

/* Upon receiving PortInitDone, all the configured ports have been created*/
bool PortsOrch::isInitDone()
{
    return m_initDone;
}

[qiluo-msft]

Could you add a vs test?

[qiluo-msft]

As comments

[jipanyang]

@qiluo-msft It is kind of hard to design VS test for the swss orchagent objects initialization order. Maybe we could think more about it in KVM test setup.

[yxieca]

This change has merge conflict with 201811 branch. Cannot be cherry-picked directly.