| Message ID | 20130118081419.GC24987@secunet.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Steffen Klassert <steffen.klassert@secunet.com> Date: Fri, 18 Jan 2013 09:14:20 +0100 > The route lookup in ipv4_sk_update_pmtu() might return a route > different from the route we cached at the socket. This is because > standart routes are per cpu, so each cpu has it's own struct rtable. > This means that we do not invalidate the socket cached route if the > NET_RX_SOFTIRQ is not served by the same cpu that the sending socket > uses. As a result, the cached route reused until we disconnect. > > With this patch we invalidate the socket cached route if possible. > If the socket is owened by the user, we can't update the cached > route directly. A followup patch will implement socket release > callback functions for datagram sockets to handle this case. > > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> This looks fine. -- 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
Hello, On Fri, 18 Jan 2013, Steffen Klassert wrote: > The route lookup in ipv4_sk_update_pmtu() might return a route > different from the route we cached at the socket. This is because > standart routes are per cpu, so each cpu has it's own struct rtable. > This means that we do not invalidate the socket cached route if the > NET_RX_SOFTIRQ is not served by the same cpu that the sending socket > uses. As a result, the cached route reused until we disconnect. > > With this patch we invalidate the socket cached route if possible. > If the socket is owened by the user, we can't update the cached > route directly. A followup patch will implement socket release > callback functions for datagram sockets to handle this case. > > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> > --- > net/ipv4/route.c | 40 +++++++++++++++++++++++++++++++++++++++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/route.c b/net/ipv4/route.c > index 259cbee..e59541b 100644 > --- a/net/ipv4/route.c > +++ b/net/ipv4/route.c > @@ -965,7 +965,7 @@ void ipv4_update_pmtu(struct sk_buff *skb, struct net *net, u32 mtu, > } > EXPORT_SYMBOL_GPL(ipv4_update_pmtu); > > -void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) > +static void __ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) > { > const struct iphdr *iph = (const struct iphdr *) skb->data; > struct flowi4 fl4; > @@ -978,6 +978,44 @@ void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) > ip_rt_put(rt); > } > } > + > +void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) > +{ > + const struct iphdr *iph = (const struct iphdr *) skb->data; > + struct flowi4 fl4; > + struct rtable *rt = (struct rtable *) __sk_dst_get(sk); I guess __sk_dst_get should be called after bh_lock_sock ? > + struct dst_entry *dst; > + > + bh_lock_sock(sk); > + if (sock_owned_by_user(sk) || !rt) { > + __ipv4_sk_update_pmtu(skb, sk, mtu); > + goto out; > + } Regards -- Julian Anastasov <ja@ssi.bg> -- 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 Sat, Jan 19, 2013 at 02:54:04AM +0200, Julian Anastasov wrote: > > + > > +void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) > > +{ > > + const struct iphdr *iph = (const struct iphdr *) skb->data; > > + struct flowi4 fl4; > > + struct rtable *rt = (struct rtable *) __sk_dst_get(sk); > > I guess __sk_dst_get should be called after > bh_lock_sock ? Yes, absolutely. I'll fix it before I submit the patch. Thanks for your review! -- 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/ipv4/route.c b/net/ipv4/route.c index 259cbee..e59541b 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -965,7 +965,7 @@ void ipv4_update_pmtu(struct sk_buff *skb, struct net *net, u32 mtu, } EXPORT_SYMBOL_GPL(ipv4_update_pmtu); -void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) +static void __ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) { const struct iphdr *iph = (const struct iphdr *) skb->data; struct flowi4 fl4; @@ -978,6 +978,44 @@ void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) ip_rt_put(rt); } } + +void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) +{ + const struct iphdr *iph = (const struct iphdr *) skb->data; + struct flowi4 fl4; + struct rtable *rt = (struct rtable *) __sk_dst_get(sk); + struct dst_entry *dst; + + bh_lock_sock(sk); + if (sock_owned_by_user(sk) || !rt) { + __ipv4_sk_update_pmtu(skb, sk, mtu); + goto out; + } + + __build_flow_key(&fl4, sk, iph, 0, 0, 0, 0, 0); + + if (!__sk_dst_check(sk, 0)) { + rt = ip_route_output_flow(sock_net(sk), &fl4, sk); + if (IS_ERR(rt)) + goto out; + } + + __ip_rt_update_pmtu((struct rtable *) rt->dst.path, &fl4, mtu); + + dst = dst_check(&rt->dst, 0); + if (!dst) { + rt = ip_route_output_flow(sock_net(sk), &fl4, sk); + if (IS_ERR(rt)) + goto out; + + dst = &rt->dst; + } + + __sk_dst_set(sk, dst); + +out: + bh_unlock_sock(sk); +} EXPORT_SYMBOL_GPL(ipv4_sk_update_pmtu); void ipv4_redirect(struct sk_buff *skb, struct net *net,
The route lookup in ipv4_sk_update_pmtu() might return a route different from the route we cached at the socket. This is because standart routes are per cpu, so each cpu has it's own struct rtable. This means that we do not invalidate the socket cached route if the NET_RX_SOFTIRQ is not served by the same cpu that the sending socket uses. As a result, the cached route reused until we disconnect. With this patch we invalidate the socket cached route if possible. If the socket is owened by the user, we can't update the cached route directly. A followup patch will implement socket release callback functions for datagram sockets to handle this case. Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> --- net/ipv4/route.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)