Fix downstream endpoint recovery - #175
Conversation
da53ab6 to
9304cb5
Compare
|
Thanks for the PR. One main thing to start:
This seems a bit backwards - I would have expected that we perform the Get Endpoint ID query to the endpoint first, to establish base connectivity. If that fails, then walk up the tree to find the top-most bridge with the connectivity issue. Is there a reason to interact with the bridge first? |
The way I was looking into this was, since everything will be routed via bridge if bridge itself is non responsive then probably downstream path might be broken too and we could be seeing timeouts with probe on downstream EID. |
Yes, I understand the rationale for checking connectivity to the bridge, but I think the ordering should be reversed - that we want to check for the smallest failure first - being the actual endpoint. If we have lost connectivity there, then we can walk up the bridge topology. Keep in mind there may be multiple bridges to walk! Also: a call to |
|
Ack, I'll update the logic to change the order. Also when you mention about multiple bridge walk, I do get the topological aspect and wanted to find out which bridge comms exactly broke, but cascaded bridge system support is not yet added, so may be we can delegate it for later? |
Yes, but you don't need to delegate much - at present, it will just appear as though the top-level bridge of a cascade just hosts all downstream devices (ie,. we do not have visibility on the intermediate bridges with the current code). So, once we have a concept of those intermediate bridges, as long as the recovery code has a "query the peer's bridge" path, we would just change that to iterate over the tree (ie, to the peer's bridge's bridge, etc) as suitable. However: with the intermediate bridges missing from the topology, we may see the following case: graph TD;
TMBO-->B1[Bridge 1];
B1--> B2[Bridge 2];
B2--> EP;
If bridge 2 is offline, the EP will be unreachable, but the only bridge that the TMBO is aware of is Bridge 1, which will appear to have full connectivity. We'll need to handle that case gracefully. |
|
Ack, I see two ways of doing this.
Although I'm not sure how could this be maintained since there is probably no direct way of asking bridge about its allocation results (Pool Start, Pool End) apart from what TBO knows about its nearest Bridge, but if this is somehow maintained then : We can recursively trace each bridges which might contain the endpoint eid E i.e (PSx < E < PEx) from TBOs nearest and check bridge's connectivity till we reach the last bridge nearest to Endpoint E. If intermediate bridge is point of failure then simply flag that Bridge EID as break point, may be?
|
|
Yes, for future features, we'll need more awareness of the downstream bridge layout and EID allocations, and I figure the recovery paths would just consume that. That would likely result in maintaining a tree data structure (say, maintainting We could look at either query hop or GRTE to populate that, I guess. |
I guess I understand the approach here, for GRTE capable bridges its direct approach of updating Also would it be okay if i introduce Query HOP logic in this PR as part of polling discovery? |
|
I don't think we want to introduce the full topology implementation in this change. |
f9ea49b to
eef2656
Compare
For downstream peers (whose EID falls within a bridge's pool range),
endpoint_recover must probe via right probe path
1. In case of probe failure, diagnose the bridge chain reaching
to endpoint and flag point of failure.
2. In case probe is successful, compare the UUID returned by the
downstream device against the stored UUID to detect device
exchange. A UUID mismatch points to replacement of old device
thus remove and republish new endpoint with same EID.
Introduce peer->gateway to track the next-hop bridge for each downstream
peer. For a single-level bridge this is set by peer_endpoint_poll() when
the downstream endpoint is discovered.
Signed-off-by: Faizan Ali <faizana@nvidia.com>
Add tests cases covering recovery of bridged (downstream) endpoints Signed-off-by: Faizan Ali <faizana@nvidia.com>
eef2656 to
1416ab6
Compare
|
I've updated the patch with rework, its open for review |
| } | ||
|
|
||
| static int query_get_peer_uuid(struct peer *peer) | ||
| static int query_get_peer_uuid(struct peer *peer, uint8_t uuid[16]) |
There was a problem hiding this comment.
Minor, but splitting this out to a prereq patch (in the same series, of course) may make the actual changes more clear. Up to you on whether that's worthwhile.
There was a problem hiding this comment.
I can add a new commit to capture this change separately.
| * If EID probe fails, check if the bridges are still responsive by walking up the bridge chain diagnosting point of exchange. | ||
| * In case we've got a response, verify UUID to detect device exchange at the same pool EID. | ||
| * for non matching UUID, remove old peer and publish the new peer with the same EID. | ||
| */ |
There was a problem hiding this comment.
Minor things:
- please keep the wrapping consistent with the rest of the file
diagnosting→diagnosting
not clear on what "point of exchange" is supposed to indicate though, perhaps "diagnosing loss of connectivity" ?
There was a problem hiding this comment.
Ack, will do the wrapping.
not clear on what "point of exchange" is supposed to indicate though, perhaps "diagnosing loss of connectivity" ?
yeah this sounds better, I'll update
2 participants
This change introduces mechanism to trigger recovery for endpoints which are behind the bridge. Current Recovery dbu-method doesn't work if triggered for downstream endpoint due to lack to right query path (addressed to downstream endpoint) with this PR we aim to merge with existing logic while handling downstream endpoint use case as well. Originally issue was being tracked via #149