[MirrorOrch]: Mirror Session Retention across Warm Reboot #1054
Conversation
stcheng
force-pushed
the
mirror_retention
branch
from
4f3b36f to
c5b81a2
Compare
After warm reboot, it is expected that the monitor port of the mirror session is retained - no changing on the monitor port withint the ECMP group members and the LAG members. This is due to the general of the sairedis comparison logic and the minimalization of SAI function calls during reconciliation. Changes: 1. Add bake() and postBake() functions in MirrorOrch bake() function retrieves the state database information and get the VLAN + monitor port information. postBake() function leverages the information and recovers the active mirror sessions the same as before warm reboot. 2. state database format change Instead of storing the object ID of the monitor port, store the alias of the monitor port. Instead of storing true/false of VLAN header, store the VLAN ID. Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
stcheng
force-pushed
the
mirror_retention
branch
from
c5b81a2 to
29b4999
Compare
orchagent/mirrororch.cpp
Outdated
| session.nexthopInfo.nexthop = neighbor_entry.ip_address; | ||
| } | ||
| // If the port belongs to a LAG | ||
| if (port.m_lag_id) |
There was a problem hiding this comment.
a lag port can also belong to a vlan, in this case, you will set the nexthop twice, once in vlan, once in lag.
There was a problem hiding this comment.
currently, the whole MirrorOrch does not support LAG in VLAN scenario. In order to have it supported, a separate pull request will be desired.
| return Orch::bake(); | ||
| } | ||
|
|
||
| bool MirrorOrch::postBake() |
There was a problem hiding this comment.
the current algorithm has a flaw. for example, let's say previous orchagent has a bug which cacluates a wrong session results (wrong port), in this algorithm, the warm reboot will give the same wrong port, and if the route is not update later, the wrong port will not be corrected.
the correct algorithm will need to check if the recorded results is in the newly calculated results or not, if it is in then use it, if it is not, the warm reboot need to correct that error.
There was a problem hiding this comment.
I found current approach is difficult to achieve above results.
the best approach is the move mirror orch after route orch in the dotask loop.
Then in the mirror orch dotask loop, caculate the new mirror session results, check if the old results is contained in the new results, if yes, then use old results. It should also results simpler code since the postBake() introduce new logics which can be avoided.
There was a problem hiding this comment.
It will leads back to the original issue that we are facing - if there are more than 1 mirror sessions require destination update, comparison logic will not be able to handle such cases, and the processes will crash. If it is expected to fix the wrong session results during the warm reboot, such logic shall be introduced after the comparison logic.
stcheng
force-pushed
the
mirror_retention
branch
from
7befdb8 to
f9a15f1
Compare
|
@lguohan please check the update, thanks! |
stcheng
force-pushed
the
mirror_retention
branch
3 times, most recently
from
2abe9e3 to
672915e
Compare
|
I think we need vs test for this feature. can you later add vs test? |
|
sure; i'll send a separate pull request to enhance the test |
|
retest this please |
With this update, we could fix potential orchagent issues before the warm reboot when the monitor port was wrongly calculated. Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
stcheng
force-pushed
the
mirror_retention
branch
from
672915e to
edb334d
Compare
|
retest this please |
Changes: -- Display ipv4 address with left adjust of 25 width and with space before UDP. -- IPv6 address will be displayed as is. -- Add test data to appl_db.json and config_db.json -- add test file sflow_test.py -- use pass_db decorator for sflow_interface. -- since sflow needs ctx, create use Db() in function. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
After warm reboot, it is expected that the monitor port of
the mirror session is retained - no changing on the monitor
port withint the ECMP group members and the LAG members. This
is due to the general of the sairedis comparison logic and
the minimalization of SAI function calls during reconciliation.
Changes:
bake() function retrieves the state database information
and get the VLAN + monitor port information.
postBake() function leverages the information and recovers
the active mirror sessions the same as before warm reboot.
Instead of storing the object ID of the monitor port, store
the alias of the monitor port.
Instead of storing true/false of VLAN header, store the VLAN
ID.
Signed-off-by: Shu0T1an ChenG shuche@microsoft.com