Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BMP's multipath support issue #2757

Open
sephiroth81 opened this issue Jan 12, 2024 · 0 comments
Open

BMP's multipath support issue #2757

sephiroth81 opened this issue Jan 12, 2024 · 0 comments

Comments

@sephiroth81
Copy link

@fujita Sorry for trouble you again. Recently I looked at the implementation of global rib for bmp and did some testing, and found that there are may be a issue?
Here is the relevant code for processing:
case *watchEventBestPath:
info := &table.PeerInfo{
Address: net.ParseIP("0.0.0.0").To4(),
AS: b.s.bgpConfig.Global.Config.As,
ID: net.ParseIP(b.s.bgpConfig.Global.Config.RouterId).To4(),
}
for _, p := range msg.PathList {
u := table.CreateUpdateMsgFromPaths([]*table.Path{p})[0]
if payload, err := u.Serialize(); err != nil {
return false
} else if err = write(bmpPeerRoute(bmp.BMP_PEER_TYPE_LOCAL_RIB, false, 0, true, info, p.GetTimestamp().Unix(), payload)); err != nil {
return false
}
}
the question is, if the TOPO is like this:
gobgpd-----router1
|
|_______router2
and gobgpd enable multipath, router1 and router2 send the same destination route to gobgpd. So gobgpd will add two equivalent routing to global rib. But in bmp case, the above logic actually only handles the case where there is only one routing. So bmp message it is not match the real situation.
We modified some code (mainly passing in oldpathlist for comparison with msg. MultiPathList and if multipah is enabled then go to the new logic), and the measured effect is that in the above scenario, the update message can be sent correctly.

I'm not quite sure what the reason is for ignoring multipath here. and If it weren't for the fact that dealing with multipath creates other problems, can I commit the relevant changes to override this scenario?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant