* [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code
@ 2014-04-22 8:13 Lorenzo Colitti
2014-04-22 8:13 ` [PATCH net-next 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
` (5 more replies)
0 siblings, 6 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 8:13 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, hannes, davem, 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 | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++
net/ipv6/raw.c | 107 ++++--------------------------------------
net/ipv6/udp.c | 117 ++++------------------------------------------
4 files changed, 154 insertions(+), 208 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..a7ec6ee 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -915,6 +915,137 @@ 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;
+
+ *optp = NULL;
+ *dstp = NULL;
+ *hlimit = *tclass = *dontfrag = -1;
+
+ 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;
+ } 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;
+
+ 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);
+
+ 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)) {
+ fl6_sock_release(flowlabel);
+ 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;
+
+ *dstp = dst;
+ *optp = opt;
+
+ return 0;
+}
+
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..212fc95 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 dst_entry *dst = NULL;
+ struct ipv6_txoptions *opt;
+ struct dst_entry *dst;
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..777b423 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1044,16 +1044,13 @@ 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;
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;
@@ -1131,118 +1128,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 +1164,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 +1198,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] 29+ messages in thread
* [PATCH net-next 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6.
2014-04-22 8:13 [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
@ 2014-04-22 8:13 ` Lorenzo Colitti
2014-04-22 8:13 ` [PATCH net-next 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
` (4 subsequent siblings)
5 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 8:13 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, hannes, davem, Lorenzo Colitti
This code was also virtually identical with the UDP and raw
socket sendmsg code.
Tested: compiles with CONFIG_IPV6=Y and CONFIG_L2TP_IP=Y.
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
net/l2tp/l2tp_ip6.c | 114 +++++-----------------------------------------------
1 file changed, 9 insertions(+), 105 deletions(-)
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 7704ea9..9154f39 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 dst_entry *dst = NULL;
+ struct ipv6_txoptions *opt;
+ struct dst_entry *dst;
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] 29+ messages in thread
* [PATCH net-next 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-22 8:13 [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-22 8:13 ` [PATCH net-next 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
@ 2014-04-22 8:13 ` Lorenzo Colitti
2014-04-22 9:06 ` [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code YOSHIFUJI Hideaki
` (3 subsequent siblings)
5 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 8:13 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, hannes, davem, 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] 29+ messages in thread
* Re: [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code
2014-04-22 8:13 [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-22 8:13 ` [PATCH net-next 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-22 8:13 ` [PATCH net-next 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
@ 2014-04-22 9:06 ` YOSHIFUJI Hideaki
2014-04-22 9:38 ` [PATCH net-next v2 " Lorenzo Colitti
` (2 subsequent siblings)
5 siblings, 0 replies; 29+ messages in thread
From: YOSHIFUJI Hideaki @ 2014-04-22 9:06 UTC (permalink / raw)
To: Lorenzo Colitti, netdev; +Cc: hannes, davem, YOSHIFUJI Hideaki
Lorenzo Colitti wrote:
> 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 | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> net/ipv6/raw.c | 107 ++++--------------------------------------
> net/ipv6/udp.c | 117 ++++------------------------------------------
> 4 files changed, 154 insertions(+), 208 deletions(-)
>
:
> --- a/net/ipv6/datagram.c
> +++ b/net/ipv6/datagram.c
> @@ -915,6 +915,137 @@ 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) {
Put this opening brace at the beginning of the next line, please.
--yoshfuji
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH net-next v2 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code
2014-04-22 8:13 [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
` (2 preceding siblings ...)
2014-04-22 9:06 ` [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code YOSHIFUJI Hideaki
@ 2014-04-22 9:38 ` Lorenzo Colitti
2014-04-22 9:38 ` [PATCH net-next v2 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-22 9:38 ` [PATCH net-next v2 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 " Lorenzo Colitti
5 siblings, 2 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 9:38 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, hannes, davem, 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 | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++
net/ipv6/raw.c | 107 ++++--------------------------------------
net/ipv6/udp.c | 117 ++++------------------------------------------
4 files changed, 155 insertions(+), 208 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..9f48682 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -915,6 +915,138 @@ 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;
+
+ *optp = NULL;
+ *dstp = NULL;
+ *hlimit = *tclass = *dontfrag = -1;
+
+ 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;
+ } 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;
+
+ 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);
+
+ 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)) {
+ fl6_sock_release(flowlabel);
+ 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;
+
+ *dstp = dst;
+ *optp = opt;
+
+ return 0;
+}
+
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..212fc95 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 dst_entry *dst = NULL;
+ struct ipv6_txoptions *opt;
+ struct dst_entry *dst;
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..777b423 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1044,16 +1044,13 @@ 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;
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;
@@ -1131,118 +1128,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 +1164,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 +1198,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] 29+ messages in thread
* [PATCH net-next v2 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6.
2014-04-22 9:38 ` [PATCH net-next v2 " Lorenzo Colitti
@ 2014-04-22 9:38 ` Lorenzo Colitti
2014-04-22 14:23 ` Eric Dumazet
2014-04-22 9:38 ` [PATCH net-next v2 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
1 sibling, 1 reply; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 9:38 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, hannes, davem, Lorenzo Colitti
This code was also virtually identical with the UDP and raw
socket sendmsg code.
Tested: compiles with CONFIG_IPV6=Y and CONFIG_L2TP_IP=Y.
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
net/l2tp/l2tp_ip6.c | 114 +++++-----------------------------------------------
1 file changed, 9 insertions(+), 105 deletions(-)
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 7704ea9..9154f39 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 dst_entry *dst = NULL;
+ struct ipv6_txoptions *opt;
+ struct dst_entry *dst;
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] 29+ messages in thread
* [PATCH net-next v2 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-22 9:38 ` [PATCH net-next v2 " Lorenzo Colitti
2014-04-22 9:38 ` [PATCH net-next v2 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
@ 2014-04-22 9:38 ` Lorenzo Colitti
1 sibling, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 9:38 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, hannes, davem, 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] 29+ messages in thread
* Re: [PATCH net-next v2 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6.
2014-04-22 9:38 ` [PATCH net-next v2 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
@ 2014-04-22 14:23 ` Eric Dumazet
2014-04-22 15:11 ` Lorenzo Colitti
0 siblings, 1 reply; 29+ messages in thread
From: Eric Dumazet @ 2014-04-22 14:23 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, yoshfuji, hannes, davem
On Tue, 2014-04-22 at 18:38 +0900, Lorenzo Colitti wrote:
> This code was also virtually identical with the UDP and raw
> socket sendmsg code.
>
> Tested: compiles with CONFIG_IPV6=Y and CONFIG_L2TP_IP=Y.
>
> Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
> ---
> net/l2tp/l2tp_ip6.c | 114 +++++-----------------------------------------------
> 1 file changed, 9 insertions(+), 105 deletions(-)
Please try CONFIG_IPV6=m and/or CONFIG_L2TP_IP=m then ;)
You forgot to EXPORT_SYMBOL(ip6_datagram_send_common)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v2 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6.
2014-04-22 14:23 ` Eric Dumazet
@ 2014-04-22 15:11 ` Lorenzo Colitti
0 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 15:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, YOSHIFUJI Hideaki, Hannes Frederic Sowa, David Miller
On Tue, Apr 22, 2014 at 11:23 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Please try CONFIG_IPV6=m and/or CONFIG_L2TP_IP=m then ;)
:-). I seem to remember having made this mistake before. :-)
> You forgot to EXPORT_SYMBOL(ip6_datagram_send_common)
Since this is a new function, and the only callers are kernel
functions that previously were not exported, I made this
EXPORT_SYMBOL_GPL.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-22 8:13 [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
` (3 preceding siblings ...)
2014-04-22 9:38 ` [PATCH net-next v2 " Lorenzo Colitti
@ 2014-04-22 15:14 ` Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
` (3 more replies)
2014-04-23 6:37 ` [PATCH net-next v4 " Lorenzo Colitti
5 siblings, 4 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 15:14 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 | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++
net/ipv6/raw.c | 107 ++++--------------------------------------
net/ipv6/udp.c | 117 ++++-----------------------------------------
4 files changed, 156 insertions(+), 208 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..e6df861 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -915,6 +915,139 @@ 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;
+
+ *optp = NULL;
+ *dstp = NULL;
+ *hlimit = *tclass = *dontfrag = -1;
+
+ 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;
+ } 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;
+
+ 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);
+
+ 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)) {
+ fl6_sock_release(flowlabel);
+ 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;
+
+ *dstp = dst;
+ *optp = opt;
+
+ 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..212fc95 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 dst_entry *dst = NULL;
+ struct ipv6_txoptions *opt;
+ struct dst_entry *dst;
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..777b423 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1044,16 +1044,13 @@ 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;
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;
@@ -1131,118 +1128,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 +1164,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 +1198,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] 29+ messages in thread
* [PATCH net-next v3 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6.
2014-04-22 15:14 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
@ 2014-04-22 15:14 ` Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
` (2 subsequent siblings)
3 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 15:14 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 | 114 +++++-----------------------------------------------
1 file changed, 9 insertions(+), 105 deletions(-)
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 7704ea9..9154f39 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 dst_entry *dst = NULL;
+ struct ipv6_txoptions *opt;
+ struct dst_entry *dst;
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] 29+ messages in thread
* [PATCH net-next v3 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-22 15:14 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
@ 2014-04-22 15:14 ` Lorenzo Colitti
2014-04-22 15:48 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
2014-04-22 15:59 ` Eric Dumazet
3 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-22 15:14 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] 29+ messages in thread
* Re: [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-22 15:14 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
@ 2014-04-22 15:48 ` Hannes Frederic Sowa
2014-04-23 6:37 ` Lorenzo Colitti
2014-04-22 15:59 ` Eric Dumazet
3 siblings, 1 reply; 29+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-22 15:48 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, yoshfuji, davem, eric.dumazet
Hi!
On Wed, Apr 23, 2014 at 12:14:14AM +0900, Lorenzo Colitti wrote:
> 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 | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> net/ipv6/raw.c | 107 ++++--------------------------------------
> net/ipv6/udp.c | 117 ++++-----------------------------------------
> 4 files changed, 156 insertions(+), 208 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..e6df861 100644
> --- a/net/ipv6/datagram.c
> +++ b/net/ipv6/datagram.c
> @@ -915,6 +915,139 @@ 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;
> +
> + *optp = NULL;
> + *dstp = NULL;
> + *hlimit = *tclass = *dontfrag = -1;
Do we need those? If we return != 0 from ip6_datagram_send_common we
know the in/out arguments may or maybe got an update an as such cannot
use them.
Caller can initialize dstp = NULL so we can pass dstp to dst_release in the
error path in the caller function (dst_release checks for argument != NULL).
Does that make sense? It looks to me like it could hide errors, but just IMHO.
> +
> + 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;
> + } 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;
> +
> + 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);
> +
> + if (!ipv6_addr_any(daddr))
> + fl6->daddr = *daddr;
> + else
> + fl6->daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
Maybe a newline?
> + 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)) {
> + fl6_sock_release(flowlabel);
> + 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;
> +
> + *dstp = dst;
> + *optp = opt;
We would also need a fl6_sock_release here because we finished processing
it.
You could also release the flowlabel after fl6_merge_options and could remove
the fl6_sock_release after ip6_sk_dst_lookup_flow in the error handling.
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ip6_datagram_send_common);
The rest looks good to me and is a nice work!
Thanks,
Hannes
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-22 15:14 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
` (2 preceding siblings ...)
2014-04-22 15:48 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
@ 2014-04-22 15:59 ` Eric Dumazet
2014-04-23 6:38 ` Lorenzo Colitti
3 siblings, 1 reply; 29+ messages in thread
From: Eric Dumazet @ 2014-04-22 15:59 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, yoshfuji, hannes, davem
On Wed, 2014-04-23 at 00:14 +0900, Lorenzo Colitti wrote:
> + dst = ip6_sk_dst_lookup_flow(sk, fl6, final_p);
> + if (IS_ERR(dst)) {
> + fl6_sock_release(flowlabel);
> + 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;
> +
> + *dstp = dst;
> + *optp = opt;
> +
> + return 0;
> +}
Don't you leak a reference on struct ip6_flowlabel *flowlabel ?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-22 15:48 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
@ 2014-04-23 6:37 ` Lorenzo Colitti
0 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-23 6:37 UTC (permalink / raw)
To: Lorenzo Colitti, netdev, YOSHIFUJI Hideaki, David Miller, Eric Dumazet
On Wed, Apr 23, 2014 at 12:48 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>> + *optp = NULL;
>> + *dstp = NULL;
>> + *hlimit = *tclass = *dontfrag = -1;
>
> Do we need those? If we return != 0 from ip6_datagram_send_common we
> know the in/out arguments may or maybe got an update an as such cannot
> use them.
>
> Caller can initialize dstp = NULL so we can pass dstp to dst_release in the
> error path in the caller function (dst_release checks for argument != NULL).
Initalizing optp was not useful. As you say, the callers won't use it
if we return an error. I've removed it.
I've also removed the initialization of dstp. After thinking about it,
I agree it's bad for the callers to rely on this function to set dstp
to NULL on failure. It's more reasonable for the callers to assume
that the dst pointer they pass in won't be touched on failure.
As for hlimit, tclass and dontfrag, they (and connected), I think it's
useful to initialize them here because it simplifies the code. Since
we set them to -1 early on, we can later do things like "if (*hlimit <
0) { set the default hop limit }" and have things work regardless of
whether a hop limit was passed in via cmsg. I don't think it's
confusing: variables are all calculated and returned by this function,
and the callers should not be setting them since they have no
information on what to set them to. Also, they don't need to be freed.
I've moved them closer to where they are actually set.
>> + if (!ipv6_addr_any(daddr))
>> + fl6->daddr = *daddr;
>> + else
>> + fl6->daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
>
> Maybe a newline?
Added.
> We would also need a fl6_sock_release here because we finished processing
> it.
>
> You could also release the flowlabel after fl6_merge_options and could remove
> the fl6_sock_release after ip6_sk_dst_lookup_flow in the error handling.
Yes, that's better. Done.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-22 8:13 [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
` (4 preceding siblings ...)
2014-04-22 15:14 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
@ 2014-04-23 6:37 ` Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
` (2 more replies)
5 siblings, 3 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-23 6:37 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 | 121 ++++-------------------------------------------
4 files changed, 158 insertions(+), 209 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..e5c0821 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 *);
@@ -1131,118 +1128,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 +1164,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 +1198,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] 29+ messages in thread
* [PATCH net-next v4 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6.
2014-04-23 6:37 ` [PATCH net-next v4 " Lorenzo Colitti
@ 2014-04-23 6:37 ` Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
2014-04-24 15:00 ` [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
2 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-23 6:37 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] 29+ messages in thread
* [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-23 6:37 ` [PATCH net-next v4 " Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
@ 2014-04-23 6:37 ` Lorenzo Colitti
2014-04-23 11:11 ` Florent Fourcot
2014-04-24 15:00 ` [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
2 siblings, 1 reply; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-23 6:37 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] 29+ messages in thread
* Re: [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-22 15:59 ` Eric Dumazet
@ 2014-04-23 6:38 ` Lorenzo Colitti
0 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-23 6:38 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, YOSHIFUJI Hideaki, Hannes Frederic Sowa, David Miller
On Wed, Apr 23, 2014 at 12:59 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Don't you leak a reference on struct ip6_flowlabel *flowlabel ?
Yes. Should be fixed in patch series 4.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-23 6:37 ` [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
@ 2014-04-23 11:11 ` Florent Fourcot
2014-04-23 12:22 ` Lorenzo Colitti
0 siblings, 1 reply; 29+ messages in thread
From: Florent Fourcot @ 2014-04-23 11:11 UTC (permalink / raw)
To: Lorenzo Colitti, netdev; +Cc: yoshfuji, hannes, davem, eric.dumazet
Le 22/04/2014 17:14, Lorenzo Colitti a écrit :> +
> + if (sin6->sin6_family != AF_INET6)
> + return -EAFNOSUPPORT;
> +
It has before returned -EINVAL, it changes the return to the user space.
You made it consistent with other protocols, but perhaps should you add
a notice in the commit changelog?
> - if (sk->sk_bound_dev_if &&
> - sk->sk_bound_dev_if != u->sin6_scope_id) {
> - return -EINVAL;
> - }
What about this check now ?
Very good idea to simplify it, by the way.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-23 11:11 ` Florent Fourcot
@ 2014-04-23 12:22 ` Lorenzo Colitti
2014-04-24 15:06 ` Hannes Frederic Sowa
0 siblings, 1 reply; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-23 12:22 UTC (permalink / raw)
To: Florent Fourcot
Cc: netdev, YOSHIFUJI Hideaki, Hannes Frederic Sowa, David Miller,
Eric Dumazet
On Wed, Apr 23, 2014 at 8:11 PM, Florent Fourcot
<florent.fourcot@enst-bretagne.fr> wrote:
> Le 22/04/2014 17:14, Lorenzo Colitti a écrit :> +
>> + if (sin6->sin6_family != AF_INET6)
>> + return -EAFNOSUPPORT;
>> +
>
> It has before returned -EINVAL, it changes the return to the user space.
> You made it consistent with other protocols, but perhaps should you add
> a notice in the commit changelog?
Actually I'm not sure what the correct value is. When you setsockopt
IPV6_V6ONLY and then send to a mapped address, the error you get
depends on what you're trying to do - ip6_datagram_connect returns
EAFNOSUPPORT, but udpv6_sendmsg, dccp_v6_connect and tcp_v6_connect
return ENETUNREACH. I think EINVAL is wrong. EAFNOSUPPORT is probably
best because the code doesn't support dual-stack ping sockets, but it
could.
There are probably very few users of this code at the moment, since
the code was only released in 3.12, and support hasn't made it into
iputils yet. And even there, ping just probably prints the error
message and exits. So I don't think it's a big deal to change the
return code.
>> - if (sk->sk_bound_dev_if &&
>> - sk->sk_bound_dev_if != u->sin6_scope_id) {
>> - return -EINVAL;
>> - }
>
> What about this check now ?
I think that was incorrect. It would return EINVAL even if you did
something as simple as:
- Open an IPv6 ping socket.
- Bind it to eth0 with SO_BINDTODEVICE
- Send a ping to 2001:: without specifying a scope id.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-23 6:37 ` [PATCH net-next v4 " Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
@ 2014-04-24 15:00 ` Hannes Frederic Sowa
2014-04-24 15:02 ` Hannes Frederic Sowa
2 siblings, 1 reply; 29+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-24 15:00 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, yoshfuji, davem, eric.dumazet
On Wed, Apr 23, 2014 at 03:37:56PM +0900, Lorenzo Colitti wrote:
> if (msg->msg_flags&MSG_CONFIRM)
> goto do_confirm;
> @@ -1262,8 +1164,6 @@ back_from_confirm:
> up->pending = AF_INET6;
>
> do_append_data:
> - if (dontfrag < 0)
> - dontfrag = np->dontfrag;
I am afraid we could jump to do_append_data without having dontfrag
initialized. The jump happens before we call to ip6_datagram_send_common.
So the initialization of dontfrag to -1 needs to be added to the caller.
> up->len += ulen;
> getfrag = is_udplite ? udplite_getfrag : ip_generic_getfrag;
> err = ip6_append_data(sk, getfrag, msg->msg_iov, ulen,
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-24 15:00 ` [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
@ 2014-04-24 15:02 ` Hannes Frederic Sowa
2014-04-24 15:13 ` Lorenzo Colitti
0 siblings, 1 reply; 29+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-24 15:02 UTC (permalink / raw)
To: Lorenzo Colitti, netdev, yoshfuji, davem, eric.dumazet
On Thu, Apr 24, 2014 at 05:00:19PM +0200, Hannes Frederic Sowa wrote:
> On Wed, Apr 23, 2014 at 03:37:56PM +0900, Lorenzo Colitti wrote:
> > if (msg->msg_flags&MSG_CONFIRM)
> > goto do_confirm;
> > @@ -1262,8 +1164,6 @@ back_from_confirm:
> > up->pending = AF_INET6;
> >
> > do_append_data:
> > - if (dontfrag < 0)
> > - dontfrag = np->dontfrag;
>
> I am afraid we could jump to do_append_data without having dontfrag
> initialized. The jump happens before we call to ip6_datagram_send_common.
>
> So the initialization of dontfrag to -1 needs to be added to the caller.
Also see e36d3ff91130002 (udp6: respect IPV6_DONTFRAG sockopt in case there
are pending frames) which was a bug we had some time ago.
Bye,
Hannes
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-23 12:22 ` Lorenzo Colitti
@ 2014-04-24 15:06 ` Hannes Frederic Sowa
2014-04-24 15:35 ` Lorenzo Colitti
0 siblings, 1 reply; 29+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-24 15:06 UTC (permalink / raw)
To: Lorenzo Colitti
Cc: Florent Fourcot, netdev, YOSHIFUJI Hideaki, David Miller, Eric Dumazet
On Wed, Apr 23, 2014 at 09:22:23PM +0900, Lorenzo Colitti wrote:
> On Wed, Apr 23, 2014 at 8:11 PM, Florent Fourcot
> <florent.fourcot@enst-bretagne.fr> wrote:
> > Le 22/04/2014 17:14, Lorenzo Colitti a écrit :> +
> >> + if (sin6->sin6_family != AF_INET6)
> >> + return -EAFNOSUPPORT;
> >> +
> >
> > It has before returned -EINVAL, it changes the return to the user space.
> > You made it consistent with other protocols, but perhaps should you add
> > a notice in the commit changelog?
>
> Actually I'm not sure what the correct value is. When you setsockopt
> IPV6_V6ONLY and then send to a mapped address, the error you get
> depends on what you're trying to do - ip6_datagram_connect returns
> EAFNOSUPPORT, but udpv6_sendmsg, dccp_v6_connect and tcp_v6_connect
> return ENETUNREACH. I think EINVAL is wrong. EAFNOSUPPORT is probably
> best because the code doesn't support dual-stack ping sockets, but it
> could.
>
> There are probably very few users of this code at the moment, since
> the code was only released in 3.12, and support hasn't made it into
> iputils yet. And even there, ping just probably prints the error
> message and exits. So I don't think it's a big deal to change the
> return code.
Sure, but we don't know about other applications. Wouldn't it be just easier
and leave this as is for now and finally let ipv6 ping sockets also handle
ipv4? I looked at it some time ago and it didn't look complicated.
> >> - if (sk->sk_bound_dev_if &&
> >> - sk->sk_bound_dev_if != u->sin6_scope_id) {
> >> - return -EINVAL;
> >> - }
> >
> > What about this check now ?
>
> I think that was incorrect. It would return EINVAL even if you did
> something as simple as:
>
> - Open an IPv6 ping socket.
> - Bind it to eth0 with SO_BINDTODEVICE
> - Send a ping to 2001:: without specifying a scope id.
Agree with that.
Bye,
Hannes
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-24 15:02 ` Hannes Frederic Sowa
@ 2014-04-24 15:13 ` Lorenzo Colitti
2014-04-24 15:43 ` Hannes Frederic Sowa
0 siblings, 1 reply; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-24 15:13 UTC (permalink / raw)
To: Lorenzo Colitti, netdev, YOSHIFUJI Hideaki, David Miller, Eric Dumazet
On Fri, Apr 25, 2014 at 12:02 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> > I am afraid we could jump to do_append_data without having dontfrag
> > initialized. The jump happens before we call to ip6_datagram_send_common.
> >
> > So the initialization of dontfrag to -1 needs to be added to the caller.
>
> Also see e36d3ff91130002 (udp6: respect IPV6_DONTFRAG sockopt in case there
> are pending frames) which was a bug we had some time ago.
Hmm. So I'm the second person to trip over that goto. It doesn't help
that the compiler didn't notice that it could have been used
uninitialized.
I wonder, is it better to just initialize dontfrag to np->dontfrag
instead of -1 in the caller? ip6_datagram_send_ctl seems to just
overwrite dontfrag with whatever comes from userspace, so nobody ever
checks that it's < 0.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-24 15:06 ` Hannes Frederic Sowa
@ 2014-04-24 15:35 ` Lorenzo Colitti
2014-04-24 16:06 ` Lorenzo Colitti
0 siblings, 1 reply; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-24 15:35 UTC (permalink / raw)
To: Lorenzo Colitti, Florent Fourcot, netdev, YOSHIFUJI Hideaki,
David Miller, Eric Dumazet
On Fri, Apr 25, 2014 at 12:06 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>> There are probably very few users of this code at the moment, since
>> the code was only released in 3.12, and support hasn't made it into
>> iputils yet. And even there, ping just probably prints the error
>> message and exits. So I don't think it's a big deal to change the
>> return code.
>
> Sure, but we don't know about other applications. Wouldn't it be just easier
> and leave this as is for now and finally let ipv6 ping sockets also handle
> ipv4? I looked at it some time ago and it didn't look complicated.
I tried this today, but it's trickier than it looks. If you just
naively call ping_v4_sendmsg, then things almost work, except the
protocol field in the IPv4 header is set to 58 (ICMPv6) instead of 1
(ICMP).
The reason is that we call ping_v4_push_pending_frames ->
ip_push_pending_frames -> ip_finish_skb -> __ip_make_skb , which ends
up doing:
iph->protocol = sk->sk_protocol;
Passing down the protocol all through that call chain seems overkill,
because in basically every case except ICMP on a dual-stack socket,
the protocol you want is in fact sk->sk_protocol.
I suppose that since ping_v4_push_pending_frames gets called with the
socket locked, we might be able to get away with setting
sk->sk_protocol just before ping_v4_push_pending_frames and restoring
it afterwards. But that feels hacky and I don't know what else it
could break.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-24 15:13 ` Lorenzo Colitti
@ 2014-04-24 15:43 ` Hannes Frederic Sowa
2014-04-25 11:09 ` Lorenzo Colitti
0 siblings, 1 reply; 29+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-24 15:43 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, YOSHIFUJI Hideaki, David Miller, Eric Dumazet
On Fri, Apr 25, 2014 at 12:13:57AM +0900, Lorenzo Colitti wrote:
> On Fri, Apr 25, 2014 at 12:02 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > > I am afraid we could jump to do_append_data without having dontfrag
> > > initialized. The jump happens before we call to ip6_datagram_send_common.
> > >
> > > So the initialization of dontfrag to -1 needs to be added to the caller.
> >
> > Also see e36d3ff91130002 (udp6: respect IPV6_DONTFRAG sockopt in case there
> > are pending frames) which was a bug we had some time ago.
>
> Hmm. So I'm the second person to trip over that goto. It doesn't help
> that the compiler didn't notice that it could have been used
> uninitialized.
>
> I wonder, is it better to just initialize dontfrag to np->dontfrag
> instead of -1 in the caller? ip6_datagram_send_ctl seems to just
> overwrite dontfrag with whatever comes from userspace, so nobody ever
> checks that it's < 0.
Yes, that is what I had in mind. So we must not reset dontfrag to -1 in
datagram_common_send and leave the check just after do_append_data as is.
Bye,
Hannes
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping.
2014-04-24 15:35 ` Lorenzo Colitti
@ 2014-04-24 16:06 ` Lorenzo Colitti
0 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-24 16:06 UTC (permalink / raw)
To: Lorenzo Colitti, Florent Fourcot, netdev, YOSHIFUJI Hideaki,
David Miller, Eric Dumazet
On Fri, Apr 25, 2014 at 12:35 AM, Lorenzo Colitti <lorenzo@google.com> wrote:
> I suppose that since ping_v4_push_pending_frames gets called with the
> socket locked, we might be able to get away with setting
> sk->sk_protocol just before ping_v4_push_pending_frames and restoring
> it afterwards. But that feels hacky and I don't know what else it
> could break.
FWIW, here's what it looks like: http://patchwork.ozlabs.org/patch/342428/
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code
2014-04-24 15:43 ` Hannes Frederic Sowa
@ 2014-04-25 11:09 ` Lorenzo Colitti
0 siblings, 0 replies; 29+ messages in thread
From: Lorenzo Colitti @ 2014-04-25 11:09 UTC (permalink / raw)
To: Lorenzo Colitti, netdev, YOSHIFUJI Hideaki, David Miller, Eric Dumazet
On Fri, Apr 25, 2014 at 12:43 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>> I wonder, is it better to just initialize dontfrag to np->dontfrag
>> instead of -1 in the caller? ip6_datagram_send_ctl seems to just
>> overwrite dontfrag with whatever comes from userspace, so nobody ever
>> checks that it's < 0.
>
> Yes, that is what I had in mind. So we must not reset dontfrag to -1 in
> datagram_common_send and leave the check just after do_append_data as is.
Thinking about it some more, it seems that that might be confusing as
well. Someone might trip up over it again - it's already happened
twice.
Instead, I've moved the initialization back into
ip6_datagram_send_common, so the variables are now obviously
uninitialized in the callers, and in udpv6_sendmsg I explicitly set
hlimit and tclass to -1, and dontfrag to np->dontfrag, before the goto
do_append data. That way, if the goto is taken, it's immediately
obvious that the variables are being set, and if it's not, it's clear
that they are uninitialized and being set in ip6_datagram_send_common.
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2014-04-25 11:10 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-22 8:13 [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-22 8:13 ` [PATCH net-next 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-22 8:13 ` [PATCH net-next 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
2014-04-22 9:06 ` [PATCH net-next 1/3] net: ipv6: unduplicate {raw,udp}v6_sendmsg code YOSHIFUJI Hideaki
2014-04-22 9:38 ` [PATCH net-next v2 " Lorenzo Colitti
2014-04-22 9:38 ` [PATCH net-next v2 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-22 14:23 ` Eric Dumazet
2014-04-22 15:11 ` Lorenzo Colitti
2014-04-22 9:38 ` [PATCH net-next v2 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-22 15:14 ` [PATCH net-next v3 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
2014-04-22 15:48 ` [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
2014-04-23 6:37 ` Lorenzo Colitti
2014-04-22 15:59 ` Eric Dumazet
2014-04-23 6:38 ` Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 " Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 2/3] net: ipv6: Use ip6_datagram_send_common in L2TP IPv6 Lorenzo Colitti
2014-04-23 6:37 ` [PATCH net-next v4 3/3] net: ipv6: Use ip6_datagram_send_common in ping Lorenzo Colitti
2014-04-23 11:11 ` Florent Fourcot
2014-04-23 12:22 ` Lorenzo Colitti
2014-04-24 15:06 ` Hannes Frederic Sowa
2014-04-24 15:35 ` Lorenzo Colitti
2014-04-24 16:06 ` Lorenzo Colitti
2014-04-24 15:00 ` [PATCH net-next v4 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code Hannes Frederic Sowa
2014-04-24 15:02 ` Hannes Frederic Sowa
2014-04-24 15:13 ` Lorenzo Colitti
2014-04-24 15:43 ` Hannes Frederic Sowa
2014-04-25 11:09 ` Lorenzo Colitti
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.