Hi Matt,

Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月21日周一 下午7:49写道：
>
> Hi Geliang,
>
> On 21/12/2020 11:16, Geliang Tang wrote:
> > Hi Matt,
> >
> > Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月19日周六 下午9:07写道：
> >>
> >> From: Geliang Tang <geliangtang(a)gmail.com>
> >>
> >> Here, we make sure we support IPv4-mapped in IPv6 addresses in different
> >> contexts:
>
> (...)
>
> >> +# no subflow IPv6 to v4 address
> >> +reset
> >> +ip netns exec $ns1 ./pm_nl_ctl limits 0 1
> >> +ip netns exec $ns2 ./pm_nl_ctl limits 0 1
> >> +ip netns exec $ns2 ./pm_nl_ctl add dead:beef:2::10.0.3.2 flags subflow
> >
> > dead:beef:2::10.0.3.2 is a invalid address, should we use dead:beef:3::2 here?
>
> I used "dead:beef:2::10.0.3.2" on purpose just to have a valid and
> usable IPv4 address at the end of the v6 one. If we try to use the IPv4
> address at the end of this v6 one, we will send the SYN+MPJ to the
> correct host (no routing issue). With the previous version of this
> patchset (v3), this was creating a subflow with 10.0.3.2!
>

Sorry, I didn't realize you did it on purpose, please keep it here.

> I should certainly add a new comment here.
> Maybe you would prefer an additional test with "dead:beef:3::2" as well?

Please add this additional test.

>
> >> +run_tests $ns1 $ns2 "10.0.1.1"
> >
> > We can drop the quotation marks here, only the IPv4-mapped address need
> > the quotation marks.
>
> Yes, I usually add quotes in Bash for each string but I see we didn't do
> that for the other tests, I will remove that!
>
> > And add "0 0 0 slow" behind it, since the test need to slow down when use
> > the IPv6 address.
>
> Do I need to do that here as well even if we don't create v6 subflows?

I think this can be consistent with other IPv6 tests, but it's up to you :)

Thanks.

-Geliang

>
> >> +chk_join_nr "no JOIN with different families v4-v6" 0 0 0
> >
> > This string is too long, it break the alignment of the output. Can we use
> > "diff" here instead of "different"?
>
> Good point, I didn't check that! I will short "different", good idea!
>
> >> +
> >> +# no subflow IPv4 to v6 address
> >> +reset
> >> +ip netns exec $ns1 ./pm_nl_ctl limits 0 1
> >> +ip netns exec $ns2 ./pm_nl_ctl limits 0 1
> >> +ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
> >> +run_tests $ns1 $ns2 dead:beef:1::1
> >
> > Add "0 0 0 slow" here.
>
> Same here, is it needed if we don't create any IPv6 subflow?
>
> >> +chk_join_nr "no JOIN with different families v6-v4" 0 0 0
> >
> > This string is too long too.
>
> Will do in v5!
>
> Thanks for the review!
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net