border: check errors before updating metadata #1988
Conversation
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sgmonroy)
go/border/rpkt/path.go, line 354 at r1 (raw file):
} // Check that the segment didn't change from a down-segment to an up-segment. if origConsDir && !*rp.consDirFlag {
This check can't be moved here, consDirFlag hasn't been changed yet.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sgmonroy)
go/border/rpkt/path.go, line 354 at r1 (raw file):
Previously, kormat (Stephen Shirley) wrote…
This check can't be moved here,
consDirFlaghasn't been changed yet.
I made a mistake when creating the patch, will fix in next version.
sgmonroy
force-pushed
the
br-fix-incpath
branch
from
5824c22
to
ac41b6e
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sgmonroy)
go/border/rpkt/path.go, line 363 at r2 (raw file):
rp.hopF = hopF rp.IncrementedPath = true if segChgd {
As discussed offline, i think this block should be dropped, and just have rp.consDirFlag = rp.infoF.ConsDir.
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @kormat)
go/border/rpkt/path.go, line 363 at r2 (raw file):
Previously, kormat (Stephen Shirley) wrote…
As discussed offline, i think this block should be dropped, and just have
rp.consDirFlag = rp.infoF.ConsDir.
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:complete! all files reviewed, all discussions resolved
Check for possible errors when incrementing the path before updating the packet metadata, which can result in an invalid state. The issue showed up when the border router processes a BadSegment packet. Because the metadata is updated, when we create the scmp reply and reverse the packet, we fail to increment the reversed path, not being able to send the error back. With this change, the packet metadata is only updated once we have verified that there are no errors when incrementing the path.
sgmonroy
force-pushed
the
br-fix-incpath
branch
from
1f33557
to
8a01529
Compare
Check for possible errors when incrementing the path before updating the
packet metadata, which can result in an invalid state.
The issue showed up when the border router processes a BadSegment
packet. Because the metadata is updated, when we create the scmp reply
and reverse the packet, we fail to increment the reversed path, not
being able to send the error back.
With this change, the packet metadata is only updated once we have
verified that there are no errors when incrementing the path.
This change is