Netdev Archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
@ 2014-04-25 11:09 Lorenzo Colitti
  2014-04-25 11:09 ` [PATCH net-next v5 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Lorenzo Colitti @ 2014-04-25 11:09 UTC (permalink / raw
  To: netdev; +Cc: yoshfuji, hannes, davem, eric.dumazet, Lorenzo Colitti

rawv6_sendmsg and udpv6_sendmsg have ~100 lines of almost
identical code. Move this into a new ipv6_datagram_send_common
helper function.

Tested: black-box tested using user-mode Linux.

- Basic UDP sends using sendto work.
- Mark routing and oif routing using SO_BINDTODEVICE work.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 include/net/ipv6.h  |   7 +++
 net/ipv6/datagram.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/ipv6/raw.c      | 105 +++-------------------------------------
 net/ipv6/udp.c      | 125 +++++-------------------------------------------
 4 files changed, 161 insertions(+), 210 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index d640925..f1a247a 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -785,6 +785,13 @@ int compat_ipv6_getsockopt(struct sock *sk, int level, int optname,
 int ip6_datagram_connect(struct sock *sk, struct sockaddr *addr, int addr_len);
 int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *addr,
 				 int addr_len);
+int ip6_datagram_send_common(struct sock *sk, struct msghdr *msg,
+			     struct sockaddr_in6 *sin6, int addr_len,
+			     struct flowi6 *fl6, struct dst_entry **dstp,
+			     struct ipv6_txoptions **optp,
+			     struct ipv6_txoptions *opt_space,
+			     int *hlimit, int *tclass, int *dontfrag,
+			     int *connected);
 
 int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len,
 		    int *addr_len);
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index c3bf2d2..92ed36b 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -915,6 +915,140 @@ exit_f:
 }
 EXPORT_SYMBOL_GPL(ip6_datagram_send_ctl);
 
+int ip6_datagram_send_common(struct sock *sk, struct msghdr *msg,
+			     struct sockaddr_in6 *sin6, int addr_len,
+			     struct flowi6 *fl6, struct dst_entry **dstp,
+			     struct ipv6_txoptions **optp,
+			     struct ipv6_txoptions *opt_space,
+			     int *hlimit, int *tclass, int *dontfrag,
+			     int *connected)
+{
+	struct ipv6_txoptions *opt = NULL;
+	struct ip6_flowlabel *flowlabel = NULL;
+	struct in6_addr *final_p, final;
+	struct ipv6_pinfo *np = inet6_sk(sk);
+	struct in6_addr *daddr;
+	struct dst_entry *dst;
+	int err;
+
+	if (sin6) {
+		daddr = &sin6->sin6_addr;
+
+		if (np->sndflow) {
+			fl6->flowlabel = sin6->sin6_flowinfo&IPV6_FLOWINFO_MASK;
+			if (fl6->flowlabel&IPV6_FLOWLABEL_MASK) {
+				flowlabel = fl6_sock_lookup(sk, fl6->flowlabel);
+				if (flowlabel == NULL)
+					return -EINVAL;
+			}
+		}
+
+		/* Otherwise it will be difficult to maintain
+		 * sk->sk_dst_cache.
+		 */
+		if (sk->sk_state == TCP_ESTABLISHED &&
+		    ipv6_addr_equal(daddr, &sk->sk_v6_daddr))
+			daddr = &sk->sk_v6_daddr;
+
+		if (addr_len >= sizeof(struct sockaddr_in6) &&
+		    sin6->sin6_scope_id &&
+		    __ipv6_addr_needs_scope_id(__ipv6_addr_type(daddr)))
+			fl6->flowi6_oif = sin6->sin6_scope_id;
+
+		*connected = 0;
+	} else {
+		if (sk->sk_state != TCP_ESTABLISHED)
+			return -EDESTADDRREQ;
+
+		daddr = &sk->sk_v6_daddr;
+		fl6->flowlabel = np->flow_label;
+		*connected = 1;
+	}
+
+	if (!fl6->flowi6_oif)
+		fl6->flowi6_oif = sk->sk_bound_dev_if;
+
+	if (!fl6->flowi6_oif)
+		fl6->flowi6_oif = np->sticky_pktinfo.ipi6_ifindex;
+
+	fl6->flowi6_mark = sk->sk_mark;
+
+	*hlimit = *tclass = *dontfrag = -1;
+
+	if (msg->msg_controllen) {
+		opt = opt_space;
+		memset(opt, 0, sizeof(*opt));
+		opt->tot_len = sizeof(*opt);
+
+		err = ip6_datagram_send_ctl(sock_net(sk), sk, msg, fl6, opt,
+					    hlimit, tclass, dontfrag);
+		if (err < 0) {
+			fl6_sock_release(flowlabel);
+			return err;
+		}
+		if ((fl6->flowlabel&IPV6_FLOWLABEL_MASK) && !flowlabel) {
+			flowlabel = fl6_sock_lookup(sk, fl6->flowlabel);
+			if (flowlabel == NULL)
+				return -EINVAL;
+		}
+		if (!(opt->opt_nflen|opt->opt_flen))
+			opt = NULL;
+		*connected = 0;
+	}
+	if (opt == NULL)
+		opt = np->opt;
+	if (flowlabel)
+		opt = fl6_merge_options(opt_space, flowlabel, opt);
+	opt = ipv6_fixup_options(opt_space, opt);
+
+	fl6_sock_release(flowlabel);
+
+	if (!ipv6_addr_any(daddr))
+		fl6->daddr = *daddr;
+	else
+		fl6->daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
+
+	if (ipv6_addr_any(&fl6->saddr) && !ipv6_addr_any(&np->saddr))
+		fl6->saddr = np->saddr;
+
+	final_p = fl6_update_dst(fl6, opt, &final);
+	if (final_p)
+		*connected = 0;
+
+	if (!fl6->flowi6_oif && ipv6_addr_is_multicast(&fl6->daddr)) {
+		fl6->flowi6_oif = np->mcast_oif;
+		*connected = 0;
+	} else if (!fl6->flowi6_oif)
+		fl6->flowi6_oif = np->ucast_oif;
+
+	security_sk_classify_flow(sk, flowi6_to_flowi(fl6));
+
+	dst = ip6_sk_dst_lookup_flow(sk, fl6, final_p);
+	if (IS_ERR(dst))
+		return PTR_ERR(dst);
+
+	if (*hlimit < 0) {
+		if (ipv6_addr_is_multicast(&fl6->daddr))
+			*hlimit = np->mcast_hops;
+		else
+			*hlimit = np->hop_limit;
+		if (*hlimit < 0)
+			*hlimit = ip6_dst_hoplimit(dst);
+	}
+
+	if (*tclass < 0)
+		*tclass = np->tclass;
+
+	if (*dontfrag < 0)
+		*dontfrag = np->dontfrag;
+
+	*optp = opt;
+	*dstp = dst;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ip6_datagram_send_common);
+
 void ip6_dgram_sock_seq_show(struct seq_file *seq, struct sock *sp,
 			     __u16 srcp, __u16 destp, int bucket)
 {
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 1f29996..a17a12e 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -739,20 +739,16 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 {
 	struct ipv6_txoptions opt_space;
 	DECLARE_SOCKADDR(struct sockaddr_in6 *, sin6, msg->msg_name);
-	struct in6_addr *daddr, *final_p, final;
 	struct inet_sock *inet = inet_sk(sk);
-	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct raw6_sock *rp = raw6_sk(sk);
-	struct ipv6_txoptions *opt = NULL;
-	struct ip6_flowlabel *flowlabel = NULL;
+	struct ipv6_txoptions *opt;
 	struct dst_entry *dst = NULL;
 	struct flowi6 fl6;
 	int addr_len = msg->msg_namelen;
-	int hlimit = -1;
-	int tclass = -1;
-	int dontfrag = -1;
+	int hlimit, tclass, dontfrag;
 	u16 proto;
 	int err;
+	int connected;
 
 	/* Rough check on arithmetic overflow,
 	   better check is made in ip6_append_data().
@@ -769,8 +765,6 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 	 */
 	memset(&fl6, 0, sizeof(fl6));
 
-	fl6.flowi6_mark = sk->sk_mark;
-
 	if (sin6) {
 		if (addr_len < SIN6_LEN_RFC2133)
 			return -EINVAL;
@@ -788,105 +782,21 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 
 		if (proto > 255)
 			return -EINVAL;
-
-		daddr = &sin6->sin6_addr;
-		if (np->sndflow) {
-			fl6.flowlabel = sin6->sin6_flowinfo&IPV6_FLOWINFO_MASK;
-			if (fl6.flowlabel&IPV6_FLOWLABEL_MASK) {
-				flowlabel = fl6_sock_lookup(sk, fl6.flowlabel);
-				if (flowlabel == NULL)
-					return -EINVAL;
-			}
-		}
-
-		/*
-		 * Otherwise it will be difficult to maintain
-		 * sk->sk_dst_cache.
-		 */
-		if (sk->sk_state == TCP_ESTABLISHED &&
-		    ipv6_addr_equal(daddr, &sk->sk_v6_daddr))
-			daddr = &sk->sk_v6_daddr;
-
-		if (addr_len >= sizeof(struct sockaddr_in6) &&
-		    sin6->sin6_scope_id &&
-		    __ipv6_addr_needs_scope_id(__ipv6_addr_type(daddr)))
-			fl6.flowi6_oif = sin6->sin6_scope_id;
 	} else {
-		if (sk->sk_state != TCP_ESTABLISHED)
-			return -EDESTADDRREQ;
-
 		proto = inet->inet_num;
-		daddr = &sk->sk_v6_daddr;
-		fl6.flowlabel = np->flow_label;
 	}
 
-	if (fl6.flowi6_oif == 0)
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
-
-	if (msg->msg_controllen) {
-		opt = &opt_space;
-		memset(opt, 0, sizeof(struct ipv6_txoptions));
-		opt->tot_len = sizeof(struct ipv6_txoptions);
-
-		err = ip6_datagram_send_ctl(sock_net(sk), sk, msg, &fl6, opt,
-					    &hlimit, &tclass, &dontfrag);
-		if (err < 0) {
-			fl6_sock_release(flowlabel);
-			return err;
-		}
-		if ((fl6.flowlabel&IPV6_FLOWLABEL_MASK) && !flowlabel) {
-			flowlabel = fl6_sock_lookup(sk, fl6.flowlabel);
-			if (flowlabel == NULL)
-				return -EINVAL;
-		}
-		if (!(opt->opt_nflen|opt->opt_flen))
-			opt = NULL;
-	}
-	if (opt == NULL)
-		opt = np->opt;
-	if (flowlabel)
-		opt = fl6_merge_options(&opt_space, flowlabel, opt);
-	opt = ipv6_fixup_options(&opt_space, opt);
-
 	fl6.flowi6_proto = proto;
 	err = rawv6_probe_proto_opt(&fl6, msg);
 	if (err)
 		goto out;
 
-	if (!ipv6_addr_any(daddr))
-		fl6.daddr = *daddr;
-	else
-		fl6.daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
-	if (ipv6_addr_any(&fl6.saddr) && !ipv6_addr_any(&np->saddr))
-		fl6.saddr = np->saddr;
-
-	final_p = fl6_update_dst(&fl6, opt, &final);
-
-	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
-		fl6.flowi6_oif = np->mcast_oif;
-	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
-	security_sk_classify_flow(sk, flowi6_to_flowi(&fl6));
-
-	dst = ip6_dst_lookup_flow(sk, &fl6, final_p);
-	if (IS_ERR(dst)) {
-		err = PTR_ERR(dst);
+	err = ip6_datagram_send_common(sk, msg, sin6, addr_len, &fl6, &dst,
+				       &opt, &opt_space, &hlimit, &tclass,
+				       &dontfrag, &connected);
+	if (err)
 		goto out;
-	}
-	if (hlimit < 0) {
-		if (ipv6_addr_is_multicast(&fl6.daddr))
-			hlimit = np->mcast_hops;
-		else
-			hlimit = np->hop_limit;
-		if (hlimit < 0)
-			hlimit = ip6_dst_hoplimit(dst);
-	}
-
-	if (tclass < 0)
-		tclass = np->tclass;
 
-	if (dontfrag < 0)
-		dontfrag = np->dontfrag;
 
 	if (msg->msg_flags&MSG_CONFIRM)
 		goto do_confirm;
@@ -909,7 +819,6 @@ back_from_confirm:
 done:
 	dst_release(dst);
 out:
-	fl6_sock_release(flowlabel);
 	return err<0?err:len;
 do_confirm:
 	dst_confirm(dst);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 1e586d9..8d427b1 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1044,19 +1044,16 @@ int udpv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 	struct inet_sock *inet = inet_sk(sk);
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	DECLARE_SOCKADDR(struct sockaddr_in6 *, sin6, msg->msg_name);
-	struct in6_addr *daddr, *final_p, final;
-	struct ipv6_txoptions *opt = NULL;
-	struct ip6_flowlabel *flowlabel = NULL;
+	struct in6_addr *daddr;
+	struct ipv6_txoptions *opt;
 	struct flowi6 fl6;
-	struct dst_entry *dst;
+	struct dst_entry *dst = NULL;
 	int addr_len = msg->msg_namelen;
 	int ulen = len;
-	int hlimit = -1;
-	int tclass = -1;
-	int dontfrag = -1;
+	int hlimit, tclass, dontfrag;
 	int corkreq = up->corkflag || msg->msg_flags&MSG_MORE;
 	int err;
-	int connected = 0;
+	int connected;
 	int is_udplite = IS_UDPLITE(sk);
 	int (*getfrag)(void *, char *, int, int, int, struct sk_buff *);
 
@@ -1123,7 +1120,9 @@ do_udp_sendmsg:
 				release_sock(sk);
 				return -EAFNOSUPPORT;
 			}
-			dst = NULL;
+			hlimit = -1;
+			tclass = -1;
+			dontfrag = np->dontfrag;
 			goto do_append_data;
 		}
 		release_sock(sk);
@@ -1131,118 +1130,23 @@ do_udp_sendmsg:
 	ulen += sizeof(struct udphdr);
 
 	memset(&fl6, 0, sizeof(fl6));
+	fl6.flowi6_proto = sk->sk_protocol;
 
 	if (sin6) {
 		if (sin6->sin6_port == 0)
 			return -EINVAL;
 
 		fl6.fl6_dport = sin6->sin6_port;
-		daddr = &sin6->sin6_addr;
-
-		if (np->sndflow) {
-			fl6.flowlabel = sin6->sin6_flowinfo&IPV6_FLOWINFO_MASK;
-			if (fl6.flowlabel&IPV6_FLOWLABEL_MASK) {
-				flowlabel = fl6_sock_lookup(sk, fl6.flowlabel);
-				if (flowlabel == NULL)
-					return -EINVAL;
-			}
-		}
-
-		/*
-		 * Otherwise it will be difficult to maintain
-		 * sk->sk_dst_cache.
-		 */
-		if (sk->sk_state == TCP_ESTABLISHED &&
-		    ipv6_addr_equal(daddr, &sk->sk_v6_daddr))
-			daddr = &sk->sk_v6_daddr;
-
-		if (addr_len >= sizeof(struct sockaddr_in6) &&
-		    sin6->sin6_scope_id &&
-		    __ipv6_addr_needs_scope_id(__ipv6_addr_type(daddr)))
-			fl6.flowi6_oif = sin6->sin6_scope_id;
 	} else {
-		if (sk->sk_state != TCP_ESTABLISHED)
-			return -EDESTADDRREQ;
-
 		fl6.fl6_dport = inet->inet_dport;
-		daddr = &sk->sk_v6_daddr;
-		fl6.flowlabel = np->flow_label;
-		connected = 1;
-	}
-
-	if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
-
-	if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->sticky_pktinfo.ipi6_ifindex;
-
-	fl6.flowi6_mark = sk->sk_mark;
-
-	if (msg->msg_controllen) {
-		opt = &opt_space;
-		memset(opt, 0, sizeof(struct ipv6_txoptions));
-		opt->tot_len = sizeof(*opt);
-
-		err = ip6_datagram_send_ctl(sock_net(sk), sk, msg, &fl6, opt,
-					    &hlimit, &tclass, &dontfrag);
-		if (err < 0) {
-			fl6_sock_release(flowlabel);
-			return err;
-		}
-		if ((fl6.flowlabel&IPV6_FLOWLABEL_MASK) && !flowlabel) {
-			flowlabel = fl6_sock_lookup(sk, fl6.flowlabel);
-			if (flowlabel == NULL)
-				return -EINVAL;
-		}
-		if (!(opt->opt_nflen|opt->opt_flen))
-			opt = NULL;
-		connected = 0;
 	}
-	if (opt == NULL)
-		opt = np->opt;
-	if (flowlabel)
-		opt = fl6_merge_options(&opt_space, flowlabel, opt);
-	opt = ipv6_fixup_options(&opt_space, opt);
-
-	fl6.flowi6_proto = sk->sk_protocol;
-	if (!ipv6_addr_any(daddr))
-		fl6.daddr = *daddr;
-	else
-		fl6.daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
-	if (ipv6_addr_any(&fl6.saddr) && !ipv6_addr_any(&np->saddr))
-		fl6.saddr = np->saddr;
 	fl6.fl6_sport = inet->inet_sport;
 
-	final_p = fl6_update_dst(&fl6, opt, &final);
-	if (final_p)
-		connected = 0;
-
-	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr)) {
-		fl6.flowi6_oif = np->mcast_oif;
-		connected = 0;
-	} else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
-
-	security_sk_classify_flow(sk, flowi6_to_flowi(&fl6));
-
-	dst = ip6_sk_dst_lookup_flow(sk, &fl6, final_p);
-	if (IS_ERR(dst)) {
-		err = PTR_ERR(dst);
-		dst = NULL;
+	err = ip6_datagram_send_common(sk, msg, sin6, addr_len, &fl6, &dst,
+				       &opt, &opt_space, &hlimit, &tclass,
+				       &dontfrag, &connected);
+	if (err)
 		goto out;
-	}
-
-	if (hlimit < 0) {
-		if (ipv6_addr_is_multicast(&fl6.daddr))
-			hlimit = np->mcast_hops;
-		else
-			hlimit = np->hop_limit;
-		if (hlimit < 0)
-			hlimit = ip6_dst_hoplimit(dst);
-	}
-
-	if (tclass < 0)
-		tclass = np->tclass;
 
 	if (msg->msg_flags&MSG_CONFIRM)
 		goto do_confirm;
@@ -1262,8 +1166,6 @@ back_from_confirm:
 	up->pending = AF_INET6;
 
 do_append_data:
-	if (dontfrag < 0)
-		dontfrag = np->dontfrag;
 	up->len += ulen;
 	getfrag  =  is_udplite ?  udplite_getfrag : ip_generic_getfrag;
 	err = ip6_append_data(sk, getfrag, msg->msg_iov, ulen,
@@ -1298,7 +1200,6 @@ do_append_data:
 	release_sock(sk);
 out:
 	dst_release(dst);
-	fl6_sock_release(flowlabel);
 	if (!err)
 		return len;
 	/*
-- 
1.9.1.423.g4596e3a

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next v5 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6.
  2014-04-25 11:09 [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
@ 2014-04-25 11:09 ` Lorenzo Colitti
  2014-04-25 11:09 ` [PATCH net-next v5 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
  2014-04-25 11:24 ` [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code David Laight
  2 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Colitti @ 2014-04-25 11:09 UTC (permalink / raw
  To: netdev; +Cc: yoshfuji, hannes, davem, eric.dumazet, Lorenzo Colitti

This code was also virtually identical with the UDP and raw
socket sendmsg code.

Tested: compiles with CONFIG_IPV6={m,y} and CONFIG_L2TP_IP={m,y}.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 net/l2tp/l2tp_ip6.c | 112 ++++------------------------------------------------
 1 file changed, 8 insertions(+), 104 deletions(-)

diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 7704ea9..790db43 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -485,19 +485,15 @@ static int l2tp_ip6_sendmsg(struct kiocb *iocb, struct sock *sk,
 {
 	struct ipv6_txoptions opt_space;
 	DECLARE_SOCKADDR(struct sockaddr_l2tpip6 *, lsa, msg->msg_name);
-	struct in6_addr *daddr, *final_p, final;
-	struct ipv6_pinfo *np = inet6_sk(sk);
-	struct ipv6_txoptions *opt = NULL;
-	struct ip6_flowlabel *flowlabel = NULL;
+	struct ipv6_txoptions *opt;
 	struct dst_entry *dst = NULL;
 	struct flowi6 fl6;
 	int addr_len = msg->msg_namelen;
-	int hlimit = -1;
-	int tclass = -1;
-	int dontfrag = -1;
+	int hlimit, tclass, dontfrag;
 	int transhdrlen = 4; /* zero session-id */
 	int ulen = len + transhdrlen;
 	int err;
+	int connected;
 
 	/* Rough check on arithmetic overflow,
 	   better check is made in ip6_append_data().
@@ -514,7 +510,7 @@ static int l2tp_ip6_sendmsg(struct kiocb *iocb, struct sock *sk,
 	 */
 	memset(&fl6, 0, sizeof(fl6));
 
-	fl6.flowi6_mark = sk->sk_mark;
+	fl6.flowi6_proto = sk->sk_protocol;
 
 	if (lsa) {
 		if (addr_len < SIN6_LEN_RFC2133)
@@ -522,103 +518,13 @@ static int l2tp_ip6_sendmsg(struct kiocb *iocb, struct sock *sk,
 
 		if (lsa->l2tp_family && lsa->l2tp_family != AF_INET6)
 			return -EAFNOSUPPORT;
-
-		daddr = &lsa->l2tp_addr;
-		if (np->sndflow) {
-			fl6.flowlabel = lsa->l2tp_flowinfo & IPV6_FLOWINFO_MASK;
-			if (fl6.flowlabel&IPV6_FLOWLABEL_MASK) {
-				flowlabel = fl6_sock_lookup(sk, fl6.flowlabel);
-				if (flowlabel == NULL)
-					return -EINVAL;
-			}
-		}
-
-		/*
-		 * Otherwise it will be difficult to maintain
-		 * sk->sk_dst_cache.
-		 */
-		if (sk->sk_state == TCP_ESTABLISHED &&
-		    ipv6_addr_equal(daddr, &sk->sk_v6_daddr))
-			daddr = &sk->sk_v6_daddr;
-
-		if (addr_len >= sizeof(struct sockaddr_in6) &&
-		    lsa->l2tp_scope_id &&
-		    ipv6_addr_type(daddr) & IPV6_ADDR_LINKLOCAL)
-			fl6.flowi6_oif = lsa->l2tp_scope_id;
-	} else {
-		if (sk->sk_state != TCP_ESTABLISHED)
-			return -EDESTADDRREQ;
-
-		daddr = &sk->sk_v6_daddr;
-		fl6.flowlabel = np->flow_label;
-	}
-
-	if (fl6.flowi6_oif == 0)
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
-
-	if (msg->msg_controllen) {
-		opt = &opt_space;
-		memset(opt, 0, sizeof(struct ipv6_txoptions));
-		opt->tot_len = sizeof(struct ipv6_txoptions);
-
-		err = ip6_datagram_send_ctl(sock_net(sk), sk, msg, &fl6, opt,
-					    &hlimit, &tclass, &dontfrag);
-		if (err < 0) {
-			fl6_sock_release(flowlabel);
-			return err;
-		}
-		if ((fl6.flowlabel & IPV6_FLOWLABEL_MASK) && !flowlabel) {
-			flowlabel = fl6_sock_lookup(sk, fl6.flowlabel);
-			if (flowlabel == NULL)
-				return -EINVAL;
-		}
-		if (!(opt->opt_nflen|opt->opt_flen))
-			opt = NULL;
 	}
 
-	if (opt == NULL)
-		opt = np->opt;
-	if (flowlabel)
-		opt = fl6_merge_options(&opt_space, flowlabel, opt);
-	opt = ipv6_fixup_options(&opt_space, opt);
-
-	fl6.flowi6_proto = sk->sk_protocol;
-	if (!ipv6_addr_any(daddr))
-		fl6.daddr = *daddr;
-	else
-		fl6.daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
-	if (ipv6_addr_any(&fl6.saddr) && !ipv6_addr_any(&np->saddr))
-		fl6.saddr = np->saddr;
-
-	final_p = fl6_update_dst(&fl6, opt, &final);
-
-	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
-		fl6.flowi6_oif = np->mcast_oif;
-	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
-
-	security_sk_classify_flow(sk, flowi6_to_flowi(&fl6));
-
-	dst = ip6_dst_lookup_flow(sk, &fl6, final_p);
-	if (IS_ERR(dst)) {
-		err = PTR_ERR(dst);
+	err = ip6_datagram_send_common(sk, msg, (struct sockaddr_in6 *) lsa,
+				       addr_len, &fl6, &dst, &opt, &opt_space,
+				       &hlimit, &tclass, &dontfrag, &connected);
+	if (err)
 		goto out;
-	}
-
-	if (hlimit < 0) {
-		if (ipv6_addr_is_multicast(&fl6.daddr))
-			hlimit = np->mcast_hops;
-		else
-			hlimit = np->hop_limit;
-		if (hlimit < 0)
-			hlimit = ip6_dst_hoplimit(dst);
-	}
-
-	if (tclass < 0)
-		tclass = np->tclass;
-
-	if (dontfrag < 0)
-		dontfrag = np->dontfrag;
 
 	if (msg->msg_flags & MSG_CONFIRM)
 		goto do_confirm;
@@ -637,8 +543,6 @@ back_from_confirm:
 done:
 	dst_release(dst);
 out:
-	fl6_sock_release(flowlabel);
-
 	return err < 0 ? err : len;
 
 do_confirm:
-- 
1.9.1.423.g4596e3a

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next v5 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
  2014-04-25 11:09 [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
  2014-04-25 11:09 ` [PATCH net-next v5 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
@ 2014-04-25 11:09 ` Lorenzo Colitti
  2014-04-25 11:24 ` [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code David Laight
  2 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Colitti @ 2014-04-25 11:09 UTC (permalink / raw
  To: netdev; +Cc: yoshfuji, hannes, davem, eric.dumazet, Lorenzo Colitti

This replaces the ad-hoc code used by ping6_sendmsg with the
implementation now used by UDP, raw and L2TP sockets. This also
adds the ability to set options via ancillary data, proper
flowlabel validation, etc. etc.

Tested: Black-box tested using user-mode Linux.

- IPv6 pings using both connect()/send() and sendto() still work.
- Fragmented IPv6 pings still work.
- Specifying a flowlabel still works.
- Attempting to send a flowlabel that is not first set via
  IPV6_FLOWLABEL_MGR now correctly returns EINVAL.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 net/ipv6/ping.c | 95 +++++++++++++++++++--------------------------------------
 1 file changed, 31 insertions(+), 64 deletions(-)

diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
index bda7429..96730c6 100644
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -81,16 +81,17 @@ static int dummy_ipv6_chk_addr(struct net *net, const struct in6_addr *addr,
 int ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		    size_t len)
 {
+	DECLARE_SOCKADDR(struct sockaddr_in6 *, sin6, msg->msg_name);
 	struct inet_sock *inet = inet_sk(sk);
-	struct ipv6_pinfo *np = inet6_sk(sk);
+	struct ipv6_txoptions *opt, opt_space;
 	struct icmp6hdr user_icmph;
-	int addr_type;
+	int addr_len = msg->msg_namelen;
 	struct in6_addr *daddr;
-	int iif = 0;
 	struct flowi6 fl6;
 	int err;
-	int hlimit;
-	struct dst_entry *dst;
+	int hlimit, tclass, dontfrag;
+	int connected;
+	struct dst_entry *dst = NULL;
 	struct rt6_info *rt;
 	struct pingfakehdr pfh;
 
@@ -101,63 +102,38 @@ int ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	if (err)
 		return err;
 
-	if (msg->msg_name) {
-		DECLARE_SOCKADDR(struct sockaddr_in6 *, u, msg->msg_name);
-		if (msg->msg_namelen < sizeof(struct sockaddr_in6) ||
-		    u->sin6_family != AF_INET6) {
+	if (sin6) {
+		if (addr_len < sizeof(struct sockaddr_in6))
 			return -EINVAL;
-		}
-		if (sk->sk_bound_dev_if &&
-		    sk->sk_bound_dev_if != u->sin6_scope_id) {
-			return -EINVAL;
-		}
-		daddr = &(u->sin6_addr);
-		iif = u->sin6_scope_id;
+
+		if (sin6->sin6_family != AF_INET6)
+			return -EAFNOSUPPORT;
+
+		daddr = &sin6->sin6_addr;
 	} else {
-		if (sk->sk_state != TCP_ESTABLISHED)
-			return -EDESTADDRREQ;
 		daddr = &sk->sk_v6_daddr;
 	}
 
-	if (!iif)
-		iif = sk->sk_bound_dev_if;
-
-	addr_type = ipv6_addr_type(daddr);
-	if (__ipv6_addr_needs_scope_id(addr_type) && !iif)
-		return -EINVAL;
-	if (addr_type & IPV6_ADDR_MAPPED)
+	if (ipv6_addr_v4mapped(daddr))
 		return -EINVAL;
 
-	/* TODO: use ip6_datagram_send_ctl to get options from cmsg */
-
 	memset(&fl6, 0, sizeof(fl6));
-
 	fl6.flowi6_proto = IPPROTO_ICMPV6;
-	fl6.saddr = np->saddr;
-	fl6.daddr = *daddr;
-	fl6.flowi6_mark = sk->sk_mark;
 	fl6.fl6_icmp_type = user_icmph.icmp6_type;
 	fl6.fl6_icmp_code = user_icmph.icmp6_code;
-	security_sk_classify_flow(sk, flowi6_to_flowi(&fl6));
 
-	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
-		fl6.flowi6_oif = np->mcast_oif;
-	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
-
-	dst = ip6_sk_dst_lookup_flow(sk, &fl6,  daddr);
-	if (IS_ERR(dst))
-		return PTR_ERR(dst);
-	rt = (struct rt6_info *) dst;
-
-	np = inet6_sk(sk);
-	if (!np)
-		return -EBADF;
+	err = ip6_datagram_send_common(sk, msg, sin6, addr_len, &fl6, &dst,
+				       &opt, &opt_space, &hlimit, &tclass,
+				       &dontfrag, &connected);
+	if (err)
+		goto out;
 
-	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
-		fl6.flowi6_oif = np->mcast_oif;
-	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
+	/* TODO: Move this check into ip6_datagram_sendmsg. */
+	if (__ipv6_addr_needs_scope_id(__ipv6_addr_type(daddr)) &&
+	    !fl6.flowi6_oif) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	pfh.icmph.type = user_icmph.icmp6_type;
 	pfh.icmph.code = user_icmph.icmp6_code;
@@ -168,18 +144,10 @@ int ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	pfh.wcheck = 0;
 	pfh.family = AF_INET6;
 
-	if (ipv6_addr_is_multicast(&fl6.daddr))
-		hlimit = np->mcast_hops;
-	else
-		hlimit = np->hop_limit;
-	if (hlimit < 0)
-		hlimit = ip6_dst_hoplimit(dst);
-
+	rt = (struct rt6_info *) dst;
 	lock_sock(sk);
-	err = ip6_append_data(sk, ping_getfrag, &pfh, len,
-			      0, hlimit,
-			      np->tclass, NULL, &fl6, rt,
-			      MSG_DONTWAIT, np->dontfrag);
+	err = ip6_append_data(sk, ping_getfrag, &pfh, len, 0, hlimit, tclass,
+			      opt, &fl6, rt, msg->msg_flags, dontfrag);
 
 	if (err) {
 		ICMP6_INC_STATS(sock_net(sk), rt->rt6i_idev,
@@ -192,10 +160,9 @@ int ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	}
 	release_sock(sk);
 
-	if (err)
-		return err;
-
-	return len;
+out:
+	dst_release(dst);
+	return err ? err : len;
 }
 
 #ifdef CONFIG_PROC_FS
-- 
1.9.1.423.g4596e3a

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
  2014-04-25 11:09 [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
  2014-04-25 11:09 ` [PATCH net-next v5 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
  2014-04-25 11:09 ` [PATCH net-next v5 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
@ 2014-04-25 11:24 ` David Laight
  2014-04-25 16:51   ` David Miller
  2 siblings, 1 reply; 10+ messages in thread
From: David Laight @ 2014-04-25 11:24 UTC (permalink / raw
  To: 'Lorenzo Colitti', netdev@vger.kernel.org
  Cc: yoshfuji@linux-ipv6.org, hannes@stressinduktion.org,
	davem@davemloft.net, eric.dumazet@gmail.com

From: Lorenzo Colitti
> rawv6_sendmsg and udpv6_sendmsg have ~100 lines of almost
> identical code. Move this into a new ipv6_datagram_send_common
> helper function.
> 
> Tested: black-box tested using user-mode Linux.
> 
> - Basic UDP sends using sendto work.
> - Mark routing and oif routing using SO_BINDTODEVICE work.
...
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index d640925..f1a247a 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -785,6 +785,13 @@ int compat_ipv6_getsockopt(struct sock *sk, int level, int optname,
>  int ip6_datagram_connect(struct sock *sk, struct sockaddr *addr, int addr_len);
>  int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *addr,
>  				 int addr_len);
> +int ip6_datagram_send_common(struct sock *sk, struct msghdr *msg,
> +			     struct sockaddr_in6 *sin6, int addr_len,
> +			     struct flowi6 *fl6, struct dst_entry **dstp,
> +			     struct ipv6_txoptions **optp,
> +			     struct ipv6_txoptions *opt_space,
> +			     int *hlimit, int *tclass, int *dontfrag,
> +			     int *connected);

That is a lot of parameters! and udp send is probably an important path.

You also repeatedly dereference some of them (eg hlimit).
I think that all of hlimit, tclass, dontfrag and connected could be shadowed
by locals and only written out on success (maybe into a structure?).
It might also be possible to reduce the number of arguments by using a sequence of calls.
(getting the args 'passed by register' helps performance.)

	David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
  2014-04-25 11:24 ` [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code David Laight
@ 2014-04-25 16:51   ` David Miller
  2014-04-25 17:19     ` Hannes Frederic Sowa
  2014-04-26  3:40     ` Lorenzo Colitti
  0 siblings, 2 replies; 10+ messages in thread
From: David Miller @ 2014-04-25 16:51 UTC (permalink / raw
  To: David.Laight; +Cc: lorenzo, netdev, yoshfuji, hannes, eric.dumazet

From: David Laight <David.Laight@ACULAB.COM>
Date: Fri, 25 Apr 2014 11:24:21 +0000

> From: Lorenzo Colitti
>> rawv6_sendmsg and udpv6_sendmsg have ~100 lines of almost
>> identical code. Move this into a new ipv6_datagram_send_common
>> helper function.
>> 
>> Tested: black-box tested using user-mode Linux.
>> 
>> - Basic UDP sends using sendto work.
>> - Mark routing and oif routing using SO_BINDTODEVICE work.
> ...
>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
>> index d640925..f1a247a 100644
>> --- a/include/net/ipv6.h
>> +++ b/include/net/ipv6.h
>> @@ -785,6 +785,13 @@ int compat_ipv6_getsockopt(struct sock *sk, int level, int optname,
>>  int ip6_datagram_connect(struct sock *sk, struct sockaddr *addr, int addr_len);
>>  int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *addr,
>>  				 int addr_len);
>> +int ip6_datagram_send_common(struct sock *sk, struct msghdr *msg,
>> +			     struct sockaddr_in6 *sin6, int addr_len,
>> +			     struct flowi6 *fl6, struct dst_entry **dstp,
>> +			     struct ipv6_txoptions **optp,
>> +			     struct ipv6_txoptions *opt_space,
>> +			     int *hlimit, int *tclass, int *dontfrag,
>> +			     int *connected);
> 
> That is a lot of parameters! and udp send is probably an important path.

The more I read this series the more I dislike it.

All of these pointers being passed in, were local variables in the
original code.

This consolidation makes the code much worse, not better.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
  2014-04-25 16:51   ` David Miller
@ 2014-04-25 17:19     ` Hannes Frederic Sowa
  2014-04-25 17:33       ` David Miller
  2014-04-26  3:40     ` Lorenzo Colitti
  1 sibling, 1 reply; 10+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-25 17:19 UTC (permalink / raw
  To: David Miller; +Cc: David.Laight, lorenzo, netdev, yoshfuji, eric.dumazet

On Fri, Apr 25, 2014 at 12:51:55PM -0400, David Miller wrote:
> From: David Laight <David.Laight@ACULAB.COM>
> Date: Fri, 25 Apr 2014 11:24:21 +0000
> 
> > From: Lorenzo Colitti
> >> rawv6_sendmsg and udpv6_sendmsg have ~100 lines of almost
> >> identical code. Move this into a new ipv6_datagram_send_common
> >> helper function.
> >> 
> >> Tested: black-box tested using user-mode Linux.
> >> 
> >> - Basic UDP sends using sendto work.
> >> - Mark routing and oif routing using SO_BINDTODEVICE work.
> > ...
> >> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> >> index d640925..f1a247a 100644
> >> --- a/include/net/ipv6.h
> >> +++ b/include/net/ipv6.h
> >> @@ -785,6 +785,13 @@ int compat_ipv6_getsockopt(struct sock *sk, int level, int optname,
> >>  int ip6_datagram_connect(struct sock *sk, struct sockaddr *addr, int addr_len);
> >>  int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *addr,
> >>  				 int addr_len);
> >> +int ip6_datagram_send_common(struct sock *sk, struct msghdr *msg,
> >> +			     struct sockaddr_in6 *sin6, int addr_len,
> >> +			     struct flowi6 *fl6, struct dst_entry **dstp,
> >> +			     struct ipv6_txoptions **optp,
> >> +			     struct ipv6_txoptions *opt_space,
> >> +			     int *hlimit, int *tclass, int *dontfrag,
> >> +			     int *connected);
> > 
> > That is a lot of parameters! and udp send is probably an important path.
> 
> The more I read this series the more I dislike it.
> 
> All of these pointers being passed in, were local variables in the
> original code.
> 
> This consolidation makes the code much worse, not better.

Maybe we can put all those hlimit, tclass, dontfrag vars into a struct
and just pass one pointer.

Also it could be possible to just do that by value if return slot optimization
in gcc optimizes that away.

Greetings,

  Hannes

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
  2014-04-25 17:19     ` Hannes Frederic Sowa
@ 2014-04-25 17:33       ` David Miller
  2014-04-25 21:26         ` Hannes Frederic Sowa
  0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2014-04-25 17:33 UTC (permalink / raw
  To: hannes; +Cc: David.Laight, lorenzo, netdev, yoshfuji, eric.dumazet

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 25 Apr 2014 19:19:09 +0200

> Maybe we can put all those hlimit, tclass, dontfrag vars into a struct
> and just pass one pointer.

It's still an unnecessary memory dereference.

The compiler can potentially optimize the accesses to those variables
into a local register with the existing code, with this consolidation
it no longer can.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
  2014-04-25 17:33       ` David Miller
@ 2014-04-25 21:26         ` Hannes Frederic Sowa
  2014-04-26  4:04           ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-25 21:26 UTC (permalink / raw
  To: David Miller; +Cc: David.Laight, lorenzo, netdev, yoshfuji, eric.dumazet

On Fri, Apr 25, 2014 at 01:33:03PM -0400, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Fri, 25 Apr 2014 19:19:09 +0200
> 
> > Maybe we can put all those hlimit, tclass, dontfrag vars into a struct
> > and just pass one pointer.
> 
> It's still an unnecessary memory dereference.

True, true.

> The compiler can potentially optimize the accesses to those variables
> into a local register with the existing code, with this consolidation
> it no longer can.

We could hack up the function in a local header file in net/ipv6 and
__always_inline it. If the resulting assembly looks good would this
be acceptable for you? Maybe we could switch to something like struct
ipv6_output_opts where we gather all those state variables common to
the code paths.

Thanks,

  Hannes

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
  2014-04-25 16:51   ` David Miller
  2014-04-25 17:19     ` Hannes Frederic Sowa
@ 2014-04-26  3:40     ` Lorenzo Colitti
  1 sibling, 0 replies; 10+ messages in thread
From: Lorenzo Colitti @ 2014-04-26  3:40 UTC (permalink / raw
  To: David Miller
  Cc: David.Laight, netdev@vger.kernel.org, YOSHIFUJI Hideaki,
	Hannes Frederic Sowa, Eric Dumazet

On Sat, Apr 26, 2014 at 1:51 AM, David Miller <davem@davemloft.net> wrote:
> This consolidation makes the code much worse, not better.

This is because the three codepaths differ at the beginning, have a
common middle, and then later on UDP differs because it supports
corking and udplite, and because it does a ip6_dst_store at the end
(which I'd argue l2tp and ping should do as well but...).

I thought reducing duplication was useful, so I sent a patch to factor
out the duplicated code in the middle. The problem is that since the
codepaths also differ at the end, it has to pass in a lot of
parameters in, and pointers to local variables that we need later on.

I see a few ways to proceed here.

1. Refactor all these codepaths into a ip6_datagram_sendmsg that
handles all the code all the way to sending the packet. That would
basically look like udpv6_sendmsg does today, but it would also
support non-UDP sockets. A few more if statements, but no more
duplicated code.
2. Rework raw and l2tp (which are almost identical), into one
function, make ping use that function, and leave UDP alone.
3. Duplicate the cmsg / flowlabel parsing code into ping6.
4. Do nothing and have ping6 not support these APIs.

Thoughts?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
  2014-04-25 21:26         ` Hannes Frederic Sowa
@ 2014-04-26  4:04           ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-04-26  4:04 UTC (permalink / raw
  To: hannes; +Cc: David.Laight, lorenzo, netdev, yoshfuji, eric.dumazet

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 25 Apr 2014 23:26:42 +0200

> On Fri, Apr 25, 2014 at 01:33:03PM -0400, David Miller wrote:
>> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Date: Fri, 25 Apr 2014 19:19:09 +0200
>> 
>> > Maybe we can put all those hlimit, tclass, dontfrag vars into a struct
>> > and just pass one pointer.
>> 
>> It's still an unnecessary memory dereference.
> 
> True, true.
> 
>> The compiler can potentially optimize the accesses to those variables
>> into a local register with the existing code, with this consolidation
>> it no longer can.
> 
> We could hack up the function in a local header file in net/ipv6 and
> __always_inline it. If the resulting assembly looks good would this
> be acceptable for you? Maybe we could switch to something like struct
> ipv6_output_opts where we gather all those state variables common to
> the code paths.

I think the consolidation attempts seen so far generally look like
crap because we haven't found the right abstraction yet.

Once a good fit is found I don't think there will be any reason for
me to consider the result ugly.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-04-26  4:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-25 11:09 [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-25 11:09 ` [PATCH net-next v5 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-25 11:09 ` [PATCH net-next v5 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
2014-04-25 11:24 ` [PATCH net-next v5 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code David Laight
2014-04-25 16:51   ` David Miller
2014-04-25 17:19     ` Hannes Frederic Sowa
2014-04-25 17:33       ` David Miller
2014-04-25 21:26         ` Hannes Frederic Sowa
2014-04-26  4:04           ` David Miller
2014-04-26  3:40     ` Lorenzo Colitti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).