This revision was automatically updated to reflect the committed changes.
Closed by commit rS315458: Constrain IPv6 routes to single FIBs when
net.add_addr_allfibs=0 (authored by asomers).
CHANGED PRIOR TO COMMIT
https://reviews.freebsd.org/D9451?vs=26053&id=26359#toc
REPOSITORY
rS FreeBSD s
asomers accepted this revision.
asomers added inline comments.
This revision has a positive review.
INLINE COMMENTS
> jhujhiti_adjectivism.org wrote in nd6_nbr.c:265
> I think this is the only thing left to consider for this patch, but it seems
> to me that using the receiving interface's FIB is
jhujhiti_adjectivism.org marked 6 inline comments as done.
jhujhiti_adjectivism.org added inline comments.
INLINE COMMENTS
> asomers wrote in icmp6.c:2147
> No. According to the comment at the bottom of icmp6_error, it isn't, because
> icmp6_reflect can sometimes be called from the output path.
asomers added inline comments.
INLINE COMMENTS
> jhujhiti_adjectivism.org wrote in icmp6.c:2147
> @asomers, can you confirm that M_GETFIB(m) is always correctly set to the FIB
> of the receiving interface?
No. According to the comment at the bottom of icmp6_error, it isn't, because
icmp6_refl
jhujhiti_adjectivism.org added a comment.
Forgot to change the other tests - this ought to do it.
One annoying detail: while running these tests, I noticed intermittent
failures on the SLAAC test. I was reminded of a similar intermittent issue I
had while developing this: for reasons I c
jhujhiti_adjectivism.org updated this revision to Diff 26053.
jhujhiti_adjectivism.org marked 14 inline comments as done.
REPOSITORY
rS FreeBSD src repository
CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D9451?vs=26022&id=26053
REVISION DETAIL
https://reviews.freebsd.org/D9451
AF
asomers added a comment.
Almost done. I think the only thing left is to delete all of the related
atf_expect_fail statements from fibs_test.sh, not just one.
INLINE COMMENTS
> jhujhiti_adjectivism.org wrote in nd6.c:1353
> This seems like a good idea. Is this new code what you had in mind?
jhujhiti_adjectivism.org added inline comments.
INLINE COMMENTS
> asomers wrote in nd6.c:1295
> Yep, it's true. One way is to do it with static routes. Another way
> involves changing the interfaces's fib. For example, like this:
>
> ifconfig tap0 create
> ifconfig tap0 10.1.0.1/24 fib 2
jhujhiti_adjectivism.org updated this revision to Diff 26022.
jhujhiti_adjectivism.org marked an inline comment as done.
REPOSITORY
rS FreeBSD src repository
CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D9451?vs=25512&id=26022
REVISION DETAIL
https://reviews.freebsd.org/D9451
AFF
asomers added inline comments.
INLINE COMMENTS
> jhujhiti_adjectivism.org wrote in nd6.c:1295
> > It's totally valid for an interface to have multiple addresses assigned,
> > each of which is on a different fib.
>
> Is this true? I'm not aware of a way this could happen. Interface routes are
>
jhujhiti_adjectivism.org added inline comments.
INLINE COMMENTS
> asomers wrote in nd6.c:1295
> Remember, the interface fib only matters for forwarding packets. It's
> totally valid for an interface to have multiple addresses assigned, each of
> which is on a different fib. So, to correctly d
asomers added a comment.
This review is starting to look pretty good. But in addition to the few
things I mentioned inline, there's one other change that you need to make: you
get to clear the `atf_expect_fail` statements from
tests/sys/netinet/fibs_test.sh.
INLINE COMMENTS
> jhujhiti_adj
jhujhiti_adjectivism.org added a comment.
I've updated the patch to address these points aside from the open question
about determining neighborship.
I think the testcase for default router advertisement should look something
like this. I'm more familiar with epair interfaces than tap, b
jhujhiti_adjectivism.org updated the summary for this revision.
jhujhiti_adjectivism.org updated this revision to Diff 25512.
jhujhiti_adjectivism.org marked 13 inline comments as done.
REPOSITORY
rS FreeBSD src repository
CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D9451?vs=24782&i
asomers added a comment.
In https://reviews.freebsd.org/D9451#196364, @jhujhiti_adjectivism.org wrote:
> As I mentioned in the PR, this is my first attempt at kernel work, so I
very much appreciate the comments. I'll go ahead and update the review summary
at my next opportunity.
>
>
jhujhiti_adjectivism.org marked 6 inline comments as done.
jhujhiti_adjectivism.org added a comment.
As I mentioned in the PR, this is my first attempt at kernel work, so I very
much appreciate the comments. I'll go ahead and update the review summary at my
next opportunity.
The testcase
asomers requested changes to this revision.
asomers added a subscriber: bz.
asomers added a comment.
This revision now requires changes to proceed.
In addition to the issues I mentioned inline, could you please also update
the review summary to include the full commit message? Try to mention
asomers added a reviewer: bz.
REPOSITORY
rS FreeBSD src repository
REVISION DETAIL
https://reviews.freebsd.org/D9451
EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/
To: jhujhiti_adjectivism.org, #network, asomers, bz
Cc: bz, imp, ae, freebsd-net-list
___
jhujhiti_adjectivism.org updated this revision to Diff 24782.
jhujhiti_adjectivism.org added a comment.
Here's the same patch with full context
REPOSITORY
rS FreeBSD src repository
CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D9451?vs=24768&id=24782
REVISION DETAIL
https://rev
ae added a comment.
Can you resubmit the patch with more context? You can make it using such
arguments for svn diff "--diff-cmd=diff -x -U99" or similar arguments for
your VCS. It produces a much large patches, but it is possible to see more
context.
REPOSITORY
rS FreeBSD src reposito
asomers added a comment.
Awesome work jhujhiti. Unfortunately, I won't be able to test it until PR
216734 is fixed or I make myself another FreeBSD head machine. I'll try to do
that sometime next week.
REPOSITORY
rS FreeBSD src repository
REVISION DETAIL
https://reviews.freebsd.org/D9
jhujhiti_adjectivism.org created this revision.
jhujhiti_adjectivism.org added reviewers: asomers, network.
jhujhiti_adjectivism.org added a subscriber: freebsd-net-list.
jhujhiti_adjectivism.org set the repository for this revision to rS FreeBSD src
repository.
Herald added subscribers: ae, imp.
22 matches
Mail list logo
