| Message ID | 1395642322-9404-2-git-send-email-wangyufen@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Mar 24, 2014 at 02:25:22PM +0800, Wangyufen wrote: > From: Wang Yufen <wangyufen@huawei.com> > > The issue raises when adding policy route, specify a particular > NIC as oif, the policy route did not take effect. The reason is > that fl6.oif is not set and route map failed. From the > tcp_v6_send_response function, if the binding address is linklocal, > fl6.oif is set, but not for global address. > > Signed-off-by: Wang Yufen <wangyufen@huawei.com> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> But this looks like a bug to me, so maybe this is something for net inclusion. Thanks, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/3/26 15:05, Hannes Frederic Sowa wrote: > On Mon, Mar 24, 2014 at 02:25:22PM +0800, Wangyufen wrote: >> From: Wang Yufen <wangyufen@huawei.com> >> >> The issue raises when adding policy route, specify a particular >> NIC as oif, the policy route did not take effect. The reason is >> that fl6.oif is not set and route map failed. From the >> tcp_v6_send_response function, if the binding address is linklocal, >> fl6.oif is set, but not for global address. >> >> Signed-off-by: Wang Yufen <wangyufen@huawei.com> > > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> > > But this looks like a bug to me, so maybe this is something for net > inclusion. I checked the commit log, I think, changes to ipv6 was incompleted in commit 4c67525849e0b7f4bd4fab leded to this issue . In tcp_v6_send_response, fl6.oif can't be directly set to iif for global address, but it should not be 0. > > Thanks, > > Hannes > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 27, 2014 at 10:00:34AM +0800, wangyufen wrote: > On 2014/3/26 15:05, Hannes Frederic Sowa wrote: > > On Mon, Mar 24, 2014 at 02:25:22PM +0800, Wangyufen wrote: > >> From: Wang Yufen <wangyufen@huawei.com> > >> > >> The issue raises when adding policy route, specify a particular > >> NIC as oif, the policy route did not take effect. The reason is > >> that fl6.oif is not set and route map failed. From the > >> tcp_v6_send_response function, if the binding address is linklocal, > >> fl6.oif is set, but not for global address. > >> > >> Signed-off-by: Wang Yufen <wangyufen@huawei.com> > > > > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> > > > > But this looks like a bug to me, so maybe this is something for net > > inclusion. > I checked the commit log, I think, changes to ipv6 was incompleted in > commit 4c67525849e0b7f4bd4fab leded to this issue . > In tcp_v6_send_response, fl6.oif can't be directly set to iif for global > address, but it should not be 0. Actually, I wonder if if (rt6_need_strict(&fl6.daddr) || !oif) fl6.flowi6_oif = inet6_iif(skb); else fl6.flowi6_oif = oif; would be ok, too, and would ensure that errors with no sockets would reach their target with higher probability in case of policy routes. In routing code we don't do strict lookup unless either we have the indication by the socket (if available) or destination address is multicast, linklocal or loopback. Otherwise we only favour flowi6_oif interfaces. Bye, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/3/27 13:32, Hannes Frederic Sowa wrote: > On Thu, Mar 27, 2014 at 10:00:34AM +0800, wangyufen wrote: >> On 2014/3/26 15:05, Hannes Frederic Sowa wrote: >>> On Mon, Mar 24, 2014 at 02:25:22PM +0800, Wangyufen wrote: >>>> From: Wang Yufen <wangyufen@huawei.com> >>>> >>>> The issue raises when adding policy route, specify a particular >>>> NIC as oif, the policy route did not take effect. The reason is >>>> that fl6.oif is not set and route map failed. From the >>>> tcp_v6_send_response function, if the binding address is linklocal, >>>> fl6.oif is set, but not for global address. >>>> >>>> Signed-off-by: Wang Yufen <wangyufen@huawei.com> >>> >>> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> >>> >>> But this looks like a bug to me, so maybe this is something for net >>> inclusion. >> I checked the commit log, I think, changes to ipv6 was incompleted in >> commit 4c67525849e0b7f4bd4fab leded to this issue . >> In tcp_v6_send_response, fl6.oif can't be directly set to iif for global >> address, but it should not be 0. > > Actually, I wonder if > > if (rt6_need_strict(&fl6.daddr) || !oif) > fl6.flowi6_oif = inet6_iif(skb); > else > fl6.flowi6_oif = oif; > > would be ok, too, and would ensure that errors with no sockets would > reach their target with higher probability in case of policy routes. > That would be better, I'll send v4 later > In routing code we don't do strict lookup unless either we have the > indication by the socket (if available) or destination address is > multicast, linklocal or loopback. Otherwise we only favour flowi6_oif > interfaces. > > Bye, > > Hannes > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index f6f38fe..066f36f 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -726,7 +726,7 @@ static const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops = { #endif static void tcp_v6_send_response(struct sk_buff *skb, u32 seq, u32 ack, u32 win, - u32 tsval, u32 tsecr, + u32 tsval, u32 tsecr, int oif, struct tcp_md5sig_key *key, int rst, u8 tclass, u32 label) { @@ -802,6 +802,8 @@ static void tcp_v6_send_response(struct sk_buff *skb, u32 seq, u32 ack, u32 win, fl6.flowi6_oif = inet6_iif(skb); fl6.fl6_dport = t1->dest; fl6.fl6_sport = t1->source; + if (fl6.flowi6_oif == 0) + fl6.flowi6_oif = oif; security_skb_classify_flow(skb, flowi6_to_flowi(&fl6)); /* Pass a socket to ip6_dst_lookup either it is for RST @@ -833,6 +835,7 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb) int genhash; struct sock *sk1 = NULL; #endif + int oif; if (th->rst) return; @@ -876,7 +879,8 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb) ack_seq = ntohl(th->seq) + th->syn + th->fin + skb->len - (th->doff << 2); - tcp_v6_send_response(skb, seq, ack_seq, 0, 0, 0, key, 1, 0, 0); + oif = sk ? sk->sk_bound_dev_if : 0; + tcp_v6_send_response(skb, seq, ack_seq, 0, 0, 0, oif, key, 1, 0, 0); #ifdef CONFIG_TCP_MD5SIG release_sk1: @@ -888,11 +892,11 @@ release_sk1: } static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, - u32 win, u32 tsval, u32 tsecr, + u32 win, u32 tsval, u32 tsecr, int oif, struct tcp_md5sig_key *key, u8 tclass, u32 label) { - tcp_v6_send_response(skb, seq, ack, win, tsval, tsecr, key, 0, tclass, + tcp_v6_send_response(skb, seq, ack, win, tsval, tsecr, oif, key, 0, tclass, label); } @@ -904,7 +908,7 @@ static void tcp_v6_timewait_ack(struct sock *sk, struct sk_buff *skb) tcp_v6_send_ack(skb, tcptw->tw_snd_nxt, tcptw->tw_rcv_nxt, tcptw->tw_rcv_wnd >> tw->tw_rcv_wscale, tcp_time_stamp + tcptw->tw_ts_offset, - tcptw->tw_ts_recent, tcp_twsk_md5_key(tcptw), + tcptw->tw_ts_recent, tw->tw_bound_dev_if, tcp_twsk_md5_key(tcptw), tw->tw_tclass, (tw->tw_flowlabel << 12)); inet_twsk_put(tw); @@ -914,7 +918,7 @@ static void tcp_v6_reqsk_send_ack(struct sock *sk, struct sk_buff *skb, struct request_sock *req) { tcp_v6_send_ack(skb, tcp_rsk(req)->snt_isn + 1, tcp_rsk(req)->rcv_isn + 1, - req->rcv_wnd, tcp_time_stamp, req->ts_recent, + req->rcv_wnd, tcp_time_stamp, req->ts_recent, sk->sk_bound_dev_if, tcp_v6_md5_do_lookup(sk, &ipv6_hdr(skb)->daddr), 0, 0); }