icmp: Limit routing table updates to the FIB of the redirect
ClosedPublic
Actions

Authored by markj on Thu, Sep 10, 10:54 AM.

Details

Reviewers

melifaro
glebius
pouria
zlei

Group Reviewers

network

Commits

rG01a95394abfd: icmp: Limit routing table updates to the FIB of the redirect

Summary

When we receive an ICMP redirect, rib_add_redirect() is used to apply
the redirect to all FIBs. This has been the case since support for
multiple FIBs was added. However, it seems rather dubious: the new
gateway might not be routable from all FIBs, and the validation done for
v4 redirects in icmp_verify_redirect_gateway() is only applied to the
FIB from which the redirect originated.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

markj created this revision.Thu, Sep 10, 10:54 AM

Herald added subscribers: ae, imp. · View Herald TranscriptThu, Sep 10, 10:54 AM

markj requested review of this revision.Thu, Sep 10, 10:54 AM

Harbormaster completed remote builds in B76741: Diff 186366.Thu, Sep 10, 10:54 AM

pouria accepted this revision.Thu, Sep 10, 10:57 AM

This revision is now accepted and ready to land.Thu, Sep 10, 10:57 AM

Not sure but it might help with PR292319

In D59567#1367281, @pouria wrote:

Not sure but it might help with PR292319

I guess not, since the problem is still there with drop_redirect=1.

Indeed looks dubious. I wonder why Alexander did it this way. Could it be M_GETFIB(m) may give us a wrong number in certain conditions? I'll prod him with a private message to take a look at this review.

I have not tested this change yet, but it looks reasonable!

In D59567#1370458, @glebius wrote:

Indeed looks dubious. I wonder why Alexander did it this way. Could it be M_GETFIB(m) may give us a wrong number in certain conditions? I'll prod him with a private message to take a look at this review.

Note, this behaviour comes from commit 8b07e49a008c8 in 2008, which is what introduced support for multiple FIBs.

Then probably we don't need to block this review on Alexander if he doesn't reply in a reasonable time.

melifaro accepted this revision.Wed, Sep 16, 8:35 PM

Closed by commit rG01a95394abfd: icmp: Limit routing table updates to the FIB of the redirect (authored by markj). · Explain WhyThu, Sep 17, 1:18 PM

This revision was automatically updated to reflect the committed changes.

markj added a commit: rG01a95394abfd: icmp: Limit routing table updates to the FIB of the redirect.