* [PATCH net-next 0/2] net/ipv6: Handle onlink flag with multipath routes @ 2018-03-13 15:40 David Ahern 2018-03-13 15:40 ` [PATCH net-next 1/2] " David Ahern 2018-03-13 15:40 ` [PATCH net-next 2/2] selftests: Add multipath tests for onlink flag David Ahern 0 siblings, 2 replies; 7+ messages in thread From: David Ahern @ 2018-03-13 15:40 UTC (permalink / raw) To: netdev; +Cc: David Ahern ONLINK flag is per nexthop. For multipath routes it is specified in rtnh_flags in struct rtnexthop versus rtm_flags for single path routes. Update ip6_route_multipath_add to look at rtnh_flags before the call to ip6_route_info_create. Add multipath test cases as well. David Ahern (2): net/ipv6: Handle onlink flag with multipath routes selftests: Add multipath tests for onlink flag net/ipv6/route.c | 1 + tools/testing/selftests/net/fib-onlink-tests.sh | 92 ++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 3 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 1/2] net/ipv6: Handle onlink flag with multipath routes 2018-03-13 15:40 [PATCH net-next 0/2] net/ipv6: Handle onlink flag with multipath routes David Ahern @ 2018-03-13 15:40 ` David Ahern 2018-03-16 15:40 ` David Miller 2018-03-13 15:40 ` [PATCH net-next 2/2] selftests: Add multipath tests for onlink flag David Ahern 1 sibling, 1 reply; 7+ messages in thread From: David Ahern @ 2018-03-13 15:40 UTC (permalink / raw) To: netdev; +Cc: David Ahern For multipath routes the ONLINK flag is specified per nexthop in rtnh_flags (as opposed to rtm_flags for unicast routes). Update ip6_route_multipath_add to set fc_flags based on rtnh_flags. Fixes: fc1e64e1092f ("net/ipv6: Add support for onlink flag") Signed-off-by: David Ahern <dsahern@gmail.com> --- net/ipv6/route.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/ipv6/route.c b/net/ipv6/route.c index f0ae58424c45..b223dffa8521 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -4166,6 +4166,7 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, r_cfg.fc_encap_type = nla_get_u16(nla); } + r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK); rt = ip6_route_info_create(&r_cfg, extack); if (IS_ERR(rt)) { err = PTR_ERR(rt); -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] net/ipv6: Handle onlink flag with multipath routes 2018-03-13 15:40 ` [PATCH net-next 1/2] " David Ahern @ 2018-03-16 15:40 ` David Miller 2018-03-16 15:45 ` David Ahern 0 siblings, 1 reply; 7+ messages in thread From: David Miller @ 2018-03-16 15:40 UTC (permalink / raw) To: dsahern; +Cc: netdev From: David Ahern <dsahern@gmail.com> Date: Tue, 13 Mar 2018 08:40:09 -0700 > For multipath routes the ONLINK flag is specified per nexthop in rtnh_flags > (as opposed to rtm_flags for unicast routes). Update ip6_route_multipath_add > to set fc_flags based on rtnh_flags. > > Fixes: fc1e64e1092f ("net/ipv6: Add support for onlink flag") > Signed-off-by: David Ahern <dsahern@gmail.com> > --- > net/ipv6/route.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index f0ae58424c45..b223dffa8521 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -4166,6 +4166,7 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, > r_cfg.fc_encap_type = nla_get_u16(nla); > } > > + r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK); > rt = ip6_route_info_create(&r_cfg, extack); > if (IS_ERR(rt)) { > err = PTR_ERR(rt); Hmmm, this actually "accumulates" the flag rather than sets it. Have you thought about what should happen if the cfg has RTNH_F_ONLINK set? I think you should either change this logic to a true 'set', or adjust your commit message to address this aspect of the new behavior. Thank you. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] net/ipv6: Handle onlink flag with multipath routes 2018-03-16 15:40 ` David Miller @ 2018-03-16 15:45 ` David Ahern 2018-03-16 16:38 ` David Miller 0 siblings, 1 reply; 7+ messages in thread From: David Ahern @ 2018-03-16 15:45 UTC (permalink / raw) To: David Miller; +Cc: netdev On 3/16/18 8:40 AM, David Miller wrote: > From: David Ahern <dsahern@gmail.com> > Date: Tue, 13 Mar 2018 08:40:09 -0700 > >> For multipath routes the ONLINK flag is specified per nexthop in rtnh_flags >> (as opposed to rtm_flags for unicast routes). Update ip6_route_multipath_add >> to set fc_flags based on rtnh_flags. >> >> Fixes: fc1e64e1092f ("net/ipv6: Add support for onlink flag") >> Signed-off-by: David Ahern <dsahern@gmail.com> >> --- >> net/ipv6/route.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >> index f0ae58424c45..b223dffa8521 100644 >> --- a/net/ipv6/route.c >> +++ b/net/ipv6/route.c >> @@ -4166,6 +4166,7 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, >> r_cfg.fc_encap_type = nla_get_u16(nla); >> } >> >> + r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK); >> rt = ip6_route_info_create(&r_cfg, extack); >> if (IS_ERR(rt)) { >> err = PTR_ERR(rt); > > Hmmm, this actually "accumulates" the flag rather than sets it. > > Have you thought about what should happen if the cfg has RTNH_F_ONLINK > set? yes, that's why the test script adds cases with the ONLINK flag set for both nexthops, then one with it on the first nexthop only, and one with the flag only on the second nexthop. If you look at the full loop 'cfg' is the variable with the user data, and r_cfg is the 'local loop version' so r_cfg.fc_flags gets reset for each nexthop: while (rtnh_ok(rtnh, remaining)) { memcpy(&r_cfg, cfg, sizeof(*cfg)); ... r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK); rt = ip6_route_info_create(&r_cfg, extack); > > I think you should either change this logic to a true 'set', or adjust > your commit message to address this aspect of the new behavior. I can update the commit message. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] net/ipv6: Handle onlink flag with multipath routes 2018-03-16 15:45 ` David Ahern @ 2018-03-16 16:38 ` David Miller 2018-03-16 18:18 ` David Ahern 0 siblings, 1 reply; 7+ messages in thread From: David Miller @ 2018-03-16 16:38 UTC (permalink / raw) To: dsahern; +Cc: netdev From: David Ahern <dsahern@gmail.com> Date: Fri, 16 Mar 2018 08:45:10 -0700 > On 3/16/18 8:40 AM, David Miller wrote: >> Hmmm, this actually "accumulates" the flag rather than sets it. >> >> Have you thought about what should happen if the cfg has RTNH_F_ONLINK >> set? > > yes, that's why the test script adds cases with the ONLINK flag set for > both nexthops, then one with it on the first nexthop only, and one with > the flag only on the second nexthop. > > If you look at the full loop 'cfg' is the variable with the user data, > and r_cfg is the 'local loop version' so r_cfg.fc_flags gets reset for > each nexthop: > > while (rtnh_ok(rtnh, remaining)) { > memcpy(&r_cfg, cfg, sizeof(*cfg)); > ... > r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK); > rt = ip6_route_info_create(&r_cfg, extack); Right. >> I think you should either change this logic to a true 'set', or adjust >> your commit message to address this aspect of the new behavior. > > I can update the commit message. Please do, thanks David. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] net/ipv6: Handle onlink flag with multipath routes 2018-03-16 16:38 ` David Miller @ 2018-03-16 18:18 ` David Ahern 0 siblings, 0 replies; 7+ messages in thread From: David Ahern @ 2018-03-16 18:18 UTC (permalink / raw) To: David Miller; +Cc: netdev On 3/16/18 9:38 AM, David Miller wrote: > From: David Ahern <dsahern@gmail.com> > Date: Fri, 16 Mar 2018 08:45:10 -0700 > >> On 3/16/18 8:40 AM, David Miller wrote: >>> Hmmm, this actually "accumulates" the flag rather than sets it. >>> >>> Have you thought about what should happen if the cfg has RTNH_F_ONLINK >>> set? >> >> yes, that's why the test script adds cases with the ONLINK flag set for >> both nexthops, then one with it on the first nexthop only, and one with >> the flag only on the second nexthop. >> >> If you look at the full loop 'cfg' is the variable with the user data, >> and r_cfg is the 'local loop version' so r_cfg.fc_flags gets reset for >> each nexthop: >> >> while (rtnh_ok(rtnh, remaining)) { >> memcpy(&r_cfg, cfg, sizeof(*cfg)); >> ... >> r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK); >> rt = ip6_route_info_create(&r_cfg, extack); > > Right. > >>> I think you should either change this logic to a true 'set', or adjust >>> your commit message to address this aspect of the new behavior. >> >> I can update the commit message. > > Please do, thanks David. > And it looks like patch 1 needs to be in 'net'; did not realize the ipv6 ONLINK support is already in 4.16. I will send the patch adding tests separately for net-next. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 2/2] selftests: Add multipath tests for onlink flag 2018-03-13 15:40 [PATCH net-next 0/2] net/ipv6: Handle onlink flag with multipath routes David Ahern 2018-03-13 15:40 ` [PATCH net-next 1/2] " David Ahern @ 2018-03-13 15:40 ` David Ahern 1 sibling, 0 replies; 7+ messages in thread From: David Ahern @ 2018-03-13 15:40 UTC (permalink / raw) To: netdev; +Cc: David Ahern Add multipath tests for onlink flag: one test with onlink added to both nexthops, then tests with onlink added to only 1 nexthop. Signed-off-by: David Ahern <dsahern@gmail.com> --- tools/testing/selftests/net/fib-onlink-tests.sh | 92 ++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/net/fib-onlink-tests.sh b/tools/testing/selftests/net/fib-onlink-tests.sh index 06b1d7cc12cc..fcf0b589e652 100755 --- a/tools/testing/selftests/net/fib-onlink-tests.sh +++ b/tools/testing/selftests/net/fib-onlink-tests.sh @@ -56,7 +56,8 @@ TEST_NET6[2]=2001:db8:102 # connected gateway CONGW[1]=169.254.1.254 -CONGW[2]=169.254.5.254 +CONGW[2]=169.254.3.254 +CONGW[3]=169.254.5.254 # recursive gateway RECGW4[1]=169.254.11.254 @@ -232,6 +233,23 @@ run_ip() log_test $? ${exp_rc} "${desc}" } +run_ip_mpath() +{ + local table="$1" + local prefix="$2" + local nh1="$3" + local nh2="$4" + local exp_rc="$5" + local desc="$6" + + # dev arg may be empty + [ -n "${dev}" ] && dev="dev ${dev}" + + run_cmd ip ro add table "${table}" "${prefix}"/32 \ + nexthop via ${nh1} nexthop via ${nh2} + log_test $? ${exp_rc} "${desc}" +} + valid_onlink_ipv4() { # - unicast connected, unicast recursive @@ -243,13 +261,37 @@ valid_onlink_ipv4() log_subsection "VRF ${VRF}" - run_ip ${VRF_TABLE} ${TEST_NET4[2]}.1 ${CONGW[2]} ${NETIFS[p5]} 0 "unicast connected" + run_ip ${VRF_TABLE} ${TEST_NET4[2]}.1 ${CONGW[3]} ${NETIFS[p5]} 0 "unicast connected" run_ip ${VRF_TABLE} ${TEST_NET4[2]}.2 ${RECGW4[2]} ${NETIFS[p5]} 0 "unicast recursive" log_subsection "VRF device, PBR table" - run_ip ${PBR_TABLE} ${TEST_NET4[2]}.3 ${CONGW[2]} ${NETIFS[p5]} 0 "unicast connected" + run_ip ${PBR_TABLE} ${TEST_NET4[2]}.3 ${CONGW[3]} ${NETIFS[p5]} 0 "unicast connected" run_ip ${PBR_TABLE} ${TEST_NET4[2]}.4 ${RECGW4[2]} ${NETIFS[p5]} 0 "unicast recursive" + + # multipath version + # + log_subsection "default VRF - main table - multipath" + + run_ip_mpath 254 ${TEST_NET4[1]}.5 \ + "${CONGW[1]} dev ${NETIFS[p1]} onlink" \ + "${CONGW[2]} dev ${NETIFS[p3]} onlink" \ + 0 "unicast connected - multipath" + + run_ip_mpath 254 ${TEST_NET4[1]}.6 \ + "${RECGW4[1]} dev ${NETIFS[p1]} onlink" \ + "${RECGW4[2]} dev ${NETIFS[p3]} onlink" \ + 0 "unicast recursive - multipath" + + run_ip_mpath 254 ${TEST_NET4[1]}.7 \ + "${CONGW[1]} dev ${NETIFS[p1]}" \ + "${CONGW[2]} dev ${NETIFS[p3]} onlink" \ + 0 "unicast connected - multipath onlink first only" + + run_ip_mpath 254 ${TEST_NET4[1]}.8 \ + "${CONGW[1]} dev ${NETIFS[p1]} onlink" \ + "${CONGW[2]} dev ${NETIFS[p3]}" \ + 0 "unicast connected - multipath onlink second only" } invalid_onlink_ipv4() @@ -289,6 +331,20 @@ run_ip6() log_test $? ${exp_rc} "${desc}" } +run_ip6_mpath() +{ + local table="$1" + local prefix="$2" + local nh1="$3" + local nh2="$4" + local exp_rc="$5" + local desc="$6" + + run_cmd ip -6 ro add table "${table}" "${prefix}"/128 \ + nexthop via ${nh1} nexthop via ${nh2} + log_test $? ${exp_rc} "${desc}" +} + valid_onlink_ipv6() { # - unicast connected, unicast recursive, v4-mapped @@ -310,6 +366,35 @@ valid_onlink_ipv6() run_ip6 ${PBR_TABLE} ${TEST_NET6[2]}::4 ${V6ADDRS[p5]/::*}::64 ${NETIFS[p5]} 0 "unicast connected" run_ip6 ${PBR_TABLE} ${TEST_NET6[2]}::5 ${RECGW6[2]} ${NETIFS[p5]} 0 "unicast recursive" run_ip6 ${PBR_TABLE} ${TEST_NET6[2]}::6 ::ffff:${TEST_NET4IN6[2]} ${NETIFS[p5]} 0 "v4-mapped" + + # multipath version + # + log_subsection "default VRF - main table - multipath" + + run_ip6_mpath 254 ${TEST_NET6[1]}::4 \ + "${V6ADDRS[p1]/::*}::64 dev ${NETIFS[p1]} onlink" \ + "${V6ADDRS[p3]/::*}::64 dev ${NETIFS[p3]} onlink" \ + 0 "unicast connected - multipath onlink" + + run_ip6_mpath 254 ${TEST_NET6[1]}::5 \ + "${RECGW6[1]} dev ${NETIFS[p1]} onlink" \ + "${RECGW6[2]} dev ${NETIFS[p3]} onlink" \ + 0 "unicast recursive - multipath onlink" + + run_ip6_mpath 254 ${TEST_NET6[1]}::6 \ + "::ffff:${TEST_NET4IN6[1]} dev ${NETIFS[p1]} onlink" \ + "::ffff:${TEST_NET4IN6[2]} dev ${NETIFS[p3]} onlink" \ + 0 "v4-mapped - multipath onlink" + + run_ip6_mpath 254 ${TEST_NET6[1]}::7 \ + "${V6ADDRS[p1]/::*}::64 dev ${NETIFS[p1]} onlink" \ + "${V6ADDRS[p3]/::*}::64 dev ${NETIFS[p3]}" \ + 0 "unicast connected - multipath onlink first only" + + run_ip6_mpath 254 ${TEST_NET6[1]}::8 \ + "${V6ADDRS[p1]/::*}::64 dev ${NETIFS[p1]}" \ + "${V6ADDRS[p3]/::*}::64 dev ${NETIFS[p3]} onlink" \ + 0 "unicast connected - multipath onlink second only" } invalid_onlink_ipv6() @@ -355,6 +440,7 @@ run_onlink_tests() log_section "IPv6 onlink" log_subsection "Valid onlink commands" valid_onlink_ipv6 + log_subsection "Invalid onlink commands" invalid_onlink_ipv6 } -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-16 18:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-13 15:40 [PATCH net-next 0/2] net/ipv6: Handle onlink flag with multipath routes David Ahern 2018-03-13 15:40 ` [PATCH net-next 1/2] " David Ahern 2018-03-16 15:40 ` David Miller 2018-03-16 15:45 ` David Ahern 2018-03-16 16:38 ` David Miller 2018-03-16 18:18 ` David Ahern 2018-03-13 15:40 ` [PATCH net-next 2/2] selftests: Add multipath tests for onlink flag David Ahern
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.