[PATCH net-next 0/5] sctp: fully support for dscp and flowlabel per transport

[PATCH net-next 0/5] sctp: fully support for dscp and flowlabel per transport

[Xin Long]

Now dscp and flowlabel are set from sock when sending the packets,
but being multi-homing, sctp also supports for dscp and flowlabel
per transport, which is described in section 8.1.12 in RFC6458.

Xin Long (5):
  ipv4: add __ip_queue_xmit() that supports tos param
  sctp: add support for dscp and flowlabel per transport
  sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
  sctp: add support for setting flowlabel when adding a transport
  sctp: check for ipv6_pinfo legal sndflow with flowlabel in
    sctp_v6_get_dst

 include/linux/sctp.h       |   7 ++
 include/net/ip.h           |   2 +
 include/net/sctp/structs.h |   9 +++
 include/uapi/linux/sctp.h  |   4 ++
 net/ipv4/ip_output.c       |  13 +++-
 net/sctp/associola.c       |  15 +++++
 net/sctp/ipv6.c            |  20 +++++-
 net/sctp/protocol.c        |  16 +++--
 net/sctp/socket.c          | 157 +++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 234 insertions(+), 9 deletions(-)

-- 
2.1.0

[PATCH net-next 1/5] ipv4: add __ip_queue_xmit() that supports tos param

[Xin Long]

This patch introduces __ip_queue_xmit(), through which the callers
can pass tos param into it without having to set inet->tos. For
ipv6, ip6_xmit() already allows passing tclass parameter.

It's needed when some transport protocol doesn't use inet->tos,
like sctp's per transport dscp, which will be added in next patch.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/ip.h     |  2 ++
 net/ipv4/ip_output.c | 13 ++++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/net/ip.h b/include/net/ip.h
index 0d2281b..ca05b77 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -148,6 +148,8 @@ void ip_send_check(struct iphdr *ip);
 int __ip_local_out(struct net *net, struct sock *sk, struct sk_buff *skb);
 int ip_local_out(struct net *net, struct sock *sk, struct sk_buff *skb);
 
+int __ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
+		    __u8 tos);
 int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl);
 void ip_init(void);
 int ip_append_data(struct sock *sk, struct flowi4 *fl4,
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index b3308e9..107d37f 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -423,7 +423,8 @@ static void ip_copy_addrs(struct iphdr *iph, const struct flowi4 *fl4)
 }
 
 /* Note: skb->sk can be different from sk, in case of tunnels */
-int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
+int __ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
+		    __u8 tos)
 {
 	struct inet_sock *inet = inet_sk(sk);
 	struct net *net = sock_net(sk);
@@ -462,7 +463,7 @@ int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
 					   inet->inet_dport,
 					   inet->inet_sport,
 					   sk->sk_protocol,
-					   RT_CONN_FLAGS(sk),
+					   RT_CONN_FLAGS_TOS(sk, tos),
 					   sk->sk_bound_dev_if);
 		if (IS_ERR(rt))
 			goto no_route;
@@ -478,7 +479,7 @@ int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
 	skb_push(skb, sizeof(struct iphdr) + (inet_opt ? inet_opt->opt.optlen : 0));
 	skb_reset_network_header(skb);
 	iph = ip_hdr(skb);
-	*((__be16 *)iph) = htons((4 << 12) | (5 << 8) | (inet->tos & 0xff));
+	*((__be16 *)iph) = htons((4 << 12) | (5 << 8) | (tos & 0xff));
 	if (ip_dont_fragment(sk, &rt->dst) && !skb->ignore_df)
 		iph->frag_off = htons(IP_DF);
 	else
@@ -511,6 +512,12 @@ int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
 	kfree_skb(skb);
 	return -EHOSTUNREACH;
 }
+EXPORT_SYMBOL(__ip_queue_xmit);
+
+int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
+{
+	return __ip_queue_xmit(sk, skb, fl, inet_sk(sk)->tos);
+}
 EXPORT_SYMBOL(ip_queue_xmit);
 
 static void ip_copy_metadata(struct sk_buff *to, struct sk_buff *from)
-- 
2.1.0

[PATCH net-next 2/5] sctp: add support for dscp and flowlabel per transport

[Xin Long]

Like some other per transport params, flowlabel and dscp are added
in transport, asoc and sctp_sock. By default, transport sets its
value from asoc's, and asoc does it from sctp_sock. flowlabel
only works for ipv6 transport.

Other than that they need to be passed down in sctp_xmit, flow4/6
also needs to set them before looking up route in get_dst.

Note that it uses '& 0x100000' to check if flowlabel is set and
'& 0x1' (tos 1st bit is unused) to check if dscp is set by users,
so that they could be set to 0 by sockopt in next patch.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/linux/sctp.h       |  7 +++++++
 include/net/sctp/structs.h |  9 +++++++++
 net/sctp/associola.c       |  7 +++++++
 net/sctp/ipv6.c            | 11 +++++++++--
 net/sctp/protocol.c        | 16 ++++++++++++----
 5 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/include/linux/sctp.h b/include/linux/sctp.h
index b36c766..83d9434 100644
--- a/include/linux/sctp.h
+++ b/include/linux/sctp.h
@@ -801,4 +801,11 @@ struct sctp_strreset_resptsn {
 	__be32 receivers_next_tsn;
 };
 
+enum {
+	SCTP_DSCP_SET_MASK = 0x1,
+	SCTP_DSCP_VAL_MASK = 0xfc,
+	SCTP_FLOWLABEL_SET_MASK = 0x100000,
+	SCTP_FLOWLABEL_VAL_MASK = 0xfffff
+};
+
 #endif /* __LINUX_SCTP_H__ */
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 701a517..ab869e0 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -193,6 +193,9 @@ struct sctp_sock {
 	/* This is the max_retrans value for new associations. */
 	__u16 pathmaxrxt;
 
+	__u32 flowlabel;
+	__u8  dscp;
+
 	/* The initial Path MTU to use for new associations. */
 	__u32 pathmtu;
 
@@ -895,6 +898,9 @@ struct sctp_transport {
 	 */
 	__u16 pathmaxrxt;
 
+	__u32 flowlabel;
+	__u8  dscp;
+
 	/* This is the partially failed retrans value for the transport
 	 * and will be initialized from the assocs value.  This can be changed
 	 * using the SCTP_PEER_ADDR_THLDS socket option
@@ -1772,6 +1778,9 @@ struct sctp_association {
 	 */
 	__u16 pathmaxrxt;
 
+	__u32 flowlabel;
+	__u8  dscp;
+
 	/* Flag that path mtu update is pending */
 	__u8   pmtu_pending;
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 5d5a162..16ecfbc 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -115,6 +115,9 @@ static struct sctp_association *sctp_association_init(
 	/* Initialize path max retrans value. */
 	asoc->pathmaxrxt = sp->pathmaxrxt;
 
+	asoc->flowlabel = sp->flowlabel;
+	asoc->dscp = sp->dscp;
+
 	/* Initialize default path MTU. */
 	asoc->pathmtu = sp->pathmtu;
 
@@ -647,6 +650,10 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 	peer->sackdelay = asoc->sackdelay;
 	peer->sackfreq = asoc->sackfreq;
 
+	if (addr->sa.sa_family == AF_INET6)
+		peer->flowlabel = asoc->flowlabel;
+	peer->dscp = asoc->dscp;
+
 	/* Enable/disable heartbeat, SACK delay, and path MTU discovery
 	 * based on association setting.
 	 */
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 7339918..772513d 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -209,12 +209,17 @@ static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
 	struct sock *sk = skb->sk;
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct flowi6 *fl6 = &transport->fl.u.ip6;
+	__u8 tclass = np->tclass;
 	int res;
 
 	pr_debug("%s: skb:%p, len:%d, src:%pI6 dst:%pI6\n", __func__, skb,
 		 skb->len, &fl6->saddr, &fl6->daddr);
 
-	IP6_ECN_flow_xmit(sk, fl6->flowlabel);
+	if (transport->dscp & SCTP_DSCP_SET_MASK)
+		tclass = transport->dscp & SCTP_DSCP_VAL_MASK;
+
+	if (INET_ECN_is_capable(tclass))
+		IP6_ECN_flow_xmit(sk, fl6->flowlabel);
 
 	if (!(transport->param_flags & SPP_PMTUD_ENABLE))
 		skb->ignore_df = 1;
@@ -223,7 +228,7 @@ static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
 
 	rcu_read_lock();
 	res = ip6_xmit(sk, skb, fl6, sk->sk_mark, rcu_dereference(np->opt),
-		       np->tclass);
+		       tclass);
 	rcu_read_unlock();
 	return res;
 }
@@ -254,6 +259,8 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 		fl6->flowi6_oif = daddr->v6.sin6_scope_id;
 	else if (asoc)
 		fl6->flowi6_oif = asoc->base.sk->sk_bound_dev_if;
+	if (t->flowlabel & SCTP_FLOWLABEL_SET_MASK)
+		fl6->flowlabel = htonl(t->flowlabel & SCTP_FLOWLABEL_VAL_MASK);
 
 	pr_debug("%s: dst=%pI6 ", __func__, &fl6->daddr);
 
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 5dffbc4..d57fd30 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -426,13 +426,16 @@ static void sctp_v4_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 	struct dst_entry *dst = NULL;
 	union sctp_addr *daddr = &t->ipaddr;
 	union sctp_addr dst_saddr;
+	__u8 tos = inet_sk(sk)->tos;
 
+	if (t->dscp & SCTP_DSCP_SET_MASK)
+		tos = t->dscp & SCTP_DSCP_VAL_MASK;
 	memset(fl4, 0x0, sizeof(struct flowi4));
 	fl4->daddr  = daddr->v4.sin_addr.s_addr;
 	fl4->fl4_dport = daddr->v4.sin_port;
 	fl4->flowi4_proto = IPPROTO_SCTP;
 	if (asoc) {
-		fl4->flowi4_tos = RT_CONN_FLAGS(asoc->base.sk);
+		fl4->flowi4_tos = RT_CONN_FLAGS_TOS(asoc->base.sk, tos);
 		fl4->flowi4_oif = asoc->base.sk->sk_bound_dev_if;
 		fl4->fl4_sport = htons(asoc->base.bind_addr.port);
 	}
@@ -495,7 +498,7 @@ static void sctp_v4_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 		fl4->fl4_sport = laddr->a.v4.sin_port;
 		flowi4_update_output(fl4,
 				     asoc->base.sk->sk_bound_dev_if,
-				     RT_CONN_FLAGS(asoc->base.sk),
+				     RT_CONN_FLAGS_TOS(asoc->base.sk, tos),
 				     daddr->v4.sin_addr.s_addr,
 				     laddr->a.v4.sin_addr.s_addr);
 
@@ -971,16 +974,21 @@ static inline int sctp_v4_xmit(struct sk_buff *skb,
 			       struct sctp_transport *transport)
 {
 	struct inet_sock *inet = inet_sk(skb->sk);
+	__u8 dscp = inet->tos;
 
 	pr_debug("%s: skb:%p, len:%d, src:%pI4, dst:%pI4\n", __func__, skb,
-		 skb->len, &transport->fl.u.ip4.saddr, &transport->fl.u.ip4.daddr);
+		 skb->len, &transport->fl.u.ip4.saddr,
+		 &transport->fl.u.ip4.daddr);
+
+	if (transport->dscp & SCTP_DSCP_SET_MASK)
+		dscp = transport->dscp & SCTP_DSCP_VAL_MASK;
 
 	inet->pmtudisc = transport->param_flags & SPP_PMTUD_ENABLE ?
 			 IP_PMTUDISC_DO : IP_PMTUDISC_DONT;
 
 	SCTP_INC_STATS(sock_net(&inet->sk), SCTP_MIB_OUTSCTPPACKS);
 
-	return ip_queue_xmit(&inet->sk, skb, &transport->fl);
+	return __ip_queue_xmit(&inet->sk, skb, &transport->fl, dscp);
 }
 
 static struct sctp_af sctp_af_inet;
-- 
2.1.0

[PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[Xin Long]

spp_ipv6_flowlabel and spp_dscp are added in sctp_paddrparams in
this patch so that users could set sctp_sock/asoc/transport dscp
and flowlabel with spp_flags SPP_IPV6_FLOWLABEL or SPP_DSCP by
SCTP_PEER_ADDR_PARAMS , as described section 8.1.12 in RFC6458.

As said in last patch, it uses '| 0x100000' or '|0x1' to mark
flowlabel or dscp is set,  so that their values could be set
to 0.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h |   4 ++
 net/sctp/socket.c         | 152 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 156 insertions(+)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index c02986a..b479db5 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -763,6 +763,8 @@ enum  sctp_spp_flags {
 	SPP_SACKDELAY_DISABLE = 1<<6,	/*Disable SACK*/
 	SPP_SACKDELAY = SPP_SACKDELAY_ENABLE | SPP_SACKDELAY_DISABLE,
 	SPP_HB_TIME_IS_ZERO = 1<<7,	/* Set HB delay to 0 */
+	SPP_IPV6_FLOWLABEL = 1<<8,
+	SPP_DSCP = 1<<9,
 };
 
 struct sctp_paddrparams {
@@ -773,6 +775,8 @@ struct sctp_paddrparams {
 	__u32			spp_pathmtu;
 	__u32			spp_sackdelay;
 	__u32			spp_flags;
+	__u32			spp_ipv6_flowlabel;
+	__u8			spp_dscp;
 } __attribute__((packed, aligned(4)));
 
 /*
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index bf11f9c..857de62 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2393,6 +2393,8 @@ static int sctp_setsockopt_autoclose(struct sock *sk, char __user *optval,
  *     uint32_t                spp_pathmtu;
  *     uint32_t                spp_sackdelay;
  *     uint32_t                spp_flags;
+ *     uint32_t                spp_ipv6_flowlabel;
+ *     uint8_t                 spp_dscp;
  * };
  *
  *   spp_assoc_id    - (one-to-many style socket) This is filled in the
@@ -2472,6 +2474,45 @@ static int sctp_setsockopt_autoclose(struct sock *sk, char __user *optval,
  *                     also that this field is mutually exclusive to
  *                     SPP_SACKDELAY_ENABLE, setting both will have undefined
  *                     results.
+ *
+ *                     SPP_IPV6_FLOWLABEL:  Setting this flag enables the
+ *                     setting of the IPV6 flow label value.  The value is
+ *                     contained in the spp_ipv6_flowlabel field.
+ *                     Upon retrieval, this flag will be set to indicate that
+ *                     the spp_ipv6_flowlabel field has a valid value returned.
+ *                     If a specific destination address is set (in the
+ *                     spp_address field), then the value returned is that of
+ *                     the address.  If just an association is specified (and
+ *                     no address), then the association's default flow label
+ *                     is returned.  If neither an association nor a destination
+ *                     is specified, then the socket's default flow label is
+ *                     returned.  For non-IPv6 sockets, this flag will be left
+ *                     cleared.
+ *
+ *                     SPP_DSCP:  Setting this flag enables the setting of the
+ *                     Differentiated Services Code Point (DSCP) value
+ *                     associated with either the association or a specific
+ *                     address.  The value is obtained in the spp_dscp field.
+ *                     Upon retrieval, this flag will be set to indicate that
+ *                     the spp_dscp field has a valid value returned.  If a
+ *                     specific destination address is set when called (in the
+ *                     spp_address field), then that specific destination
+ *                     address's DSCP value is returned.  If just an association
+ *                     is specified, then the association's default DSCP is
+ *                     returned.  If neither an association nor a destination is
+ *                     specified, then the socket's default DSCP is returned.
+ *
+ *   spp_ipv6_flowlabel
+ *                   - This field is used in conjunction with the
+ *                     SPP_IPV6_FLOWLABEL flag and contains the IPv6 flow label.
+ *                     The 20 least significant bits are used for the flow
+ *                     label.  This setting has precedence over any IPv6-layer
+ *                     setting.
+ *
+ *   spp_dscp        - This field is used in conjunction with the SPP_DSCP flag
+ *                     and contains the DSCP.  The 6 most significant bits are
+ *                     used for the DSCP.  This setting has precedence over any
+ *                     IPv4- or IPv6- layer setting.
  */
 static int sctp_apply_peer_addr_params(struct sctp_paddrparams *params,
 				       struct sctp_transport   *trans,
@@ -2611,6 +2652,51 @@ static int sctp_apply_peer_addr_params(struct sctp_paddrparams *params,
 		}
 	}
 
+	if (params->spp_flags & SPP_IPV6_FLOWLABEL) {
+		if (trans && trans->ipaddr.sa.sa_family == AF_INET6) {
+			trans->flowlabel = params->spp_ipv6_flowlabel &
+					   SCTP_FLOWLABEL_VAL_MASK;
+			trans->flowlabel |= SCTP_FLOWLABEL_SET_MASK;
+		} else if (asoc) {
+			list_for_each_entry(trans,
+					    &asoc->peer.transport_addr_list,
+					    transports) {
+				if (trans->ipaddr.sa.sa_family != AF_INET6)
+					continue;
+				trans->flowlabel = params->spp_ipv6_flowlabel &
+						   SCTP_FLOWLABEL_VAL_MASK;
+				trans->flowlabel |= SCTP_FLOWLABEL_SET_MASK;
+			}
+			asoc->flowlabel = params->spp_ipv6_flowlabel &
+					  SCTP_FLOWLABEL_VAL_MASK;
+			asoc->flowlabel |= SCTP_FLOWLABEL_SET_MASK;
+		} else if (sctp_opt2sk(sp)->sk_family == AF_INET6) {
+			sp->flowlabel = params->spp_ipv6_flowlabel &
+					SCTP_FLOWLABEL_VAL_MASK;
+			sp->flowlabel |= SCTP_FLOWLABEL_SET_MASK;
+		}
+	}
+
+	if (params->spp_flags & SPP_DSCP) {
+		if (trans) {
+			trans->dscp = params->spp_dscp & SCTP_DSCP_VAL_MASK;
+			trans->dscp |= SCTP_DSCP_SET_MASK;
+		} else if (asoc) {
+			list_for_each_entry(trans,
+					    &asoc->peer.transport_addr_list,
+					    transports) {
+				trans->dscp = params->spp_dscp &
+					      SCTP_DSCP_VAL_MASK;
+				trans->dscp |= SCTP_DSCP_SET_MASK;
+			}
+			asoc->dscp = params->spp_dscp & SCTP_DSCP_VAL_MASK;
+			asoc->dscp |= SCTP_DSCP_SET_MASK;
+		} else {
+			sp->dscp = params->spp_dscp & SCTP_DSCP_VAL_MASK;
+			sp->dscp |= SCTP_DSCP_SET_MASK;
+		}
+	}
+
 	return 0;
 }
 
@@ -5453,6 +5539,45 @@ static int sctp_getsockopt_peeloff_flags(struct sock *sk, int len,
  *                     also that this field is mutually exclusive to
  *                     SPP_SACKDELAY_ENABLE, setting both will have undefined
  *                     results.
+ *
+ *                     SPP_IPV6_FLOWLABEL:  Setting this flag enables the
+ *                     setting of the IPV6 flow label value.  The value is
+ *                     contained in the spp_ipv6_flowlabel field.
+ *                     Upon retrieval, this flag will be set to indicate that
+ *                     the spp_ipv6_flowlabel field has a valid value returned.
+ *                     If a specific destination address is set (in the
+ *                     spp_address field), then the value returned is that of
+ *                     the address.  If just an association is specified (and
+ *                     no address), then the association's default flow label
+ *                     is returned.  If neither an association nor a destination
+ *                     is specified, then the socket's default flow label is
+ *                     returned.  For non-IPv6 sockets, this flag will be left
+ *                     cleared.
+ *
+ *                     SPP_DSCP:  Setting this flag enables the setting of the
+ *                     Differentiated Services Code Point (DSCP) value
+ *                     associated with either the association or a specific
+ *                     address.  The value is obtained in the spp_dscp field.
+ *                     Upon retrieval, this flag will be set to indicate that
+ *                     the spp_dscp field has a valid value returned.  If a
+ *                     specific destination address is set when called (in the
+ *                     spp_address field), then that specific destination
+ *                     address's DSCP value is returned.  If just an association
+ *                     is specified, then the association's default DSCP is
+ *                     returned.  If neither an association nor a destination is
+ *                     specified, then the socket's default DSCP is returned.
+ *
+ *   spp_ipv6_flowlabel
+ *                   - This field is used in conjunction with the
+ *                     SPP_IPV6_FLOWLABEL flag and contains the IPv6 flow label.
+ *                     The 20 least significant bits are used for the flow
+ *                     label.  This setting has precedence over any IPv6-layer
+ *                     setting.
+ *
+ *   spp_dscp        - This field is used in conjunction with the SPP_DSCP flag
+ *                     and contains the DSCP.  The 6 most significant bits are
+ *                     used for the DSCP.  This setting has precedence over any
+ *                     IPv4- or IPv6- layer setting.
  */
 static int sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
 					    char __user *optval, int __user *optlen)
@@ -5499,6 +5624,15 @@ static int sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
 
 		/*draft-11 doesn't say what to return in spp_flags*/
 		params.spp_flags      = trans->param_flags;
+		if (trans->flowlabel & SCTP_FLOWLABEL_SET_MASK) {
+			params.spp_ipv6_flowlabel = trans->flowlabel &
+						    SCTP_FLOWLABEL_VAL_MASK;
+			params.spp_flags |= SPP_IPV6_FLOWLABEL;
+		}
+		if (trans->dscp & SCTP_DSCP_SET_MASK) {
+			params.spp_dscp	= trans->dscp & SCTP_DSCP_VAL_MASK;
+			params.spp_flags |= SPP_DSCP;
+		}
 	} else if (asoc) {
 		/* Fetch association values. */
 		params.spp_hbinterval = jiffies_to_msecs(asoc->hbinterval);
@@ -5508,6 +5642,15 @@ static int sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
 
 		/*draft-11 doesn't say what to return in spp_flags*/
 		params.spp_flags      = asoc->param_flags;
+		if (asoc->flowlabel & SCTP_FLOWLABEL_SET_MASK) {
+			params.spp_ipv6_flowlabel = asoc->flowlabel &
+						    SCTP_FLOWLABEL_VAL_MASK;
+			params.spp_flags |= SPP_IPV6_FLOWLABEL;
+		}
+		if (asoc->dscp & SCTP_DSCP_SET_MASK) {
+			params.spp_dscp	= asoc->dscp & SCTP_DSCP_VAL_MASK;
+			params.spp_flags |= SPP_DSCP;
+		}
 	} else {
 		/* Fetch socket values. */
 		params.spp_hbinterval = sp->hbinterval;
@@ -5517,6 +5660,15 @@ static int sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
 
 		/*draft-11 doesn't say what to return in spp_flags*/
 		params.spp_flags      = sp->param_flags;
+		if (sp->flowlabel & SCTP_FLOWLABEL_SET_MASK) {
+			params.spp_ipv6_flowlabel = sp->flowlabel &
+						    SCTP_FLOWLABEL_VAL_MASK;
+			params.spp_flags |= SPP_IPV6_FLOWLABEL;
+		}
+		if (sp->dscp & SCTP_DSCP_SET_MASK) {
+			params.spp_dscp	= sp->dscp & SCTP_DSCP_VAL_MASK;
+			params.spp_flags |= SPP_DSCP;
+		}
 	}
 
 	if (copy_to_user(optval, &params, len))
-- 
2.1.0

[PATCH net-next 4/5] sctp: add support for setting flowlabel when adding a transport

[Xin Long]

Struct sockaddr_in6 has the member sin6_flowinfo that includes the
ipv6 flowlabel, it should also support for setting flowlabel when
adding a transport whose ipaddr is from userspace.

Note that addrinfo in sctp_sendmsg is using struct in6_addr for
the secondary addrs, which doesn't contain sin6_flowinfo, and
it needs to copy sin6_flowinfo from the primary addr.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/associola.c | 12 ++++++++++--
 net/sctp/socket.c    |  5 +++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 16ecfbc..297d9cf 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -650,8 +650,16 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 	peer->sackdelay = asoc->sackdelay;
 	peer->sackfreq = asoc->sackfreq;
 
-	if (addr->sa.sa_family == AF_INET6)
-		peer->flowlabel = asoc->flowlabel;
+	if (addr->sa.sa_family == AF_INET6) {
+		__be32 info = addr->v6.sin6_flowinfo;
+
+		if (info) {
+			peer->flowlabel = ntohl(info & IPV6_FLOWLABEL_MASK);
+			peer->flowlabel |= SCTP_FLOWLABEL_SET_MASK;
+		} else {
+			peer->flowlabel = asoc->flowlabel;
+		}
+	}
 	peer->dscp = asoc->dscp;
 
 	/* Enable/disable heartbeat, SACK delay, and path MTU discovery
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 857de62..1df5d07 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1697,6 +1697,7 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
 	struct sctp_association *asoc;
 	enum sctp_scope scope;
 	struct cmsghdr *cmsg;
+	__be32 flowinfo = 0;
 	struct sctp_af *af;
 	int err;
 
@@ -1781,6 +1782,9 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
 	if (!cmsgs->addrs_msg)
 		return 0;
 
+	if (daddr->sa.sa_family == AF_INET6)
+		flowinfo = daddr->v6.sin6_flowinfo;
+
 	/* sendv addr list parse */
 	for_each_cmsghdr(cmsg, cmsgs->addrs_msg) {
 		struct sctp_transport *transport;
@@ -1813,6 +1817,7 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
 			}
 
 			dlen = sizeof(struct in6_addr);
+			daddr->v6.sin6_flowinfo = flowinfo;
 			daddr->v6.sin6_family = AF_INET6;
 			daddr->v6.sin6_port = htons(asoc->peer.port);
 			memcpy(&daddr->v6.sin6_addr, CMSG_DATA(cmsg), dlen);
-- 
2.1.0

[PATCH net-next 5/5] sctp: check for ipv6_pinfo legal sndflow with flowlabel in sctp_v6_get_dst

[Xin Long]

The transport with illegal flowlabel should not be allowed to send
packets. Other transport protocols already denies this.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/ipv6.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 772513d..d83ddc4 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -262,6 +262,15 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 	if (t->flowlabel & SCTP_FLOWLABEL_SET_MASK)
 		fl6->flowlabel = htonl(t->flowlabel & SCTP_FLOWLABEL_VAL_MASK);
 
+	if (np->sndflow && (fl6->flowlabel & IPV6_FLOWLABEL_MASK)) {
+		struct ip6_flowlabel *flowlabel;
+
+		flowlabel = fl6_sock_lookup(sk, fl6->flowlabel);
+		if (!flowlabel)
+			goto out;
+		fl6_sock_release(flowlabel);
+	}
+
 	pr_debug("%s: dst=%pI6 ", __func__, &fl6->daddr);
 
 	if (asoc)
-- 
2.1.0

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[David Miller]

From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 25 Jun 2018 10:14:35 +0800

>  struct sctp_paddrparams {
> @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>  	__u32			spp_pathmtu;
>  	__u32			spp_sackdelay;
>  	__u32			spp_flags;
> +	__u32			spp_ipv6_flowlabel;
> +	__u8			spp_dscp;
>  } __attribute__((packed, aligned(4)));

I don't think you can change the size of this structure like this.

This check in sctp_setsockopt_peer_addr_params():

	if (optlen != sizeof(struct sctp_paddrparams))
		return -EINVAL;

is going to trigger in old kernels when executing programs
built against the new struct definition.

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[Neil Horman]

On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
> From: Xin Long <lucien.xin@gmail.com>
> Date: Mon, 25 Jun 2018 10:14:35 +0800
> 
> >  struct sctp_paddrparams {
> > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> >  	__u32			spp_pathmtu;
> >  	__u32			spp_sackdelay;
> >  	__u32			spp_flags;
> > +	__u32			spp_ipv6_flowlabel;
> > +	__u8			spp_dscp;
> >  } __attribute__((packed, aligned(4)));
> 
> I don't think you can change the size of this structure like this.
> 
> This check in sctp_setsockopt_peer_addr_params():
> 
> 	if (optlen != sizeof(struct sctp_paddrparams))
> 		return -EINVAL;
> 
> is going to trigger in old kernels when executing programs
> built against the new struct definition.
> 
I think thats also the reason its a packed aligned attribute, it can't be
changed, or older kernels won't be able to fill it out properly.
Neil

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[Marcelo Ricardo Leitner]

On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
> > From: Xin Long <lucien.xin@gmail.com>
> > Date: Mon, 25 Jun 2018 10:14:35 +0800
> > 
> > >  struct sctp_paddrparams {
> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> > >  	__u32			spp_pathmtu;
> > >  	__u32			spp_sackdelay;
> > >  	__u32			spp_flags;
> > > +	__u32			spp_ipv6_flowlabel;
> > > +	__u8			spp_dscp;
> > >  } __attribute__((packed, aligned(4)));
> > 
> > I don't think you can change the size of this structure like this.
> > 
> > This check in sctp_setsockopt_peer_addr_params():
> > 
> > 	if (optlen != sizeof(struct sctp_paddrparams))
> > 		return -EINVAL;
> > 
> > is going to trigger in old kernels when executing programs
> > built against the new struct definition.

That will happen, yes, but do we really care about being future-proof
here? I mean: if we also update such check(s) to support dealing with
smaller-than-supported structs, newer kernels will be able to run
programs built against the old struct, and the new one; while building
using newer headers and running on older kernel may fool the
application in other ways too (like enabling support for something
that is available on newer kernel and that is not present in the older
one).

> > 
> I think thats also the reason its a packed aligned attribute, it can't be
> changed, or older kernels won't be able to fill it out properly.
> Neil

It's more for supporting running 32-bits apps on 64-bit kernels
(according to 20c9c825b12fc).

  Marcelo

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[吉藤英明]

Hi,

2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>:
> On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>> > From: Xin Long <lucien.xin@gmail.com>
>> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>> >
>> > >  struct sctp_paddrparams {
>> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>> > >   __u32                   spp_pathmtu;
>> > >   __u32                   spp_sackdelay;
>> > >   __u32                   spp_flags;
>> > > + __u32                   spp_ipv6_flowlabel;
>> > > + __u8                    spp_dscp;
>> > >  } __attribute__((packed, aligned(4)));
>> >
>> > I don't think you can change the size of this structure like this.
>> >
>> > This check in sctp_setsockopt_peer_addr_params():
>> >
>> >     if (optlen != sizeof(struct sctp_paddrparams))
>> >             return -EINVAL;
>> >
>> > is going to trigger in old kernels when executing programs
>> > built against the new struct definition.
>
> That will happen, yes, but do we really care about being future-proof
> here? I mean: if we also update such check(s) to support dealing with
> smaller-than-supported structs, newer kernels will be able to run
> programs built against the old struct, and the new one; while building
> using newer headers and running on older kernel may fool the
> application in other ways too (like enabling support for something
> that is available on newer kernel and that is not present in the older
> one).

We should not break existing apps.
We still accept apps of pre-2.4 era without sin6_scope_id
(e.g., net/ipv6/af_inet6.c:inet6_bind()).

>
>> >
>> I think thats also the reason its a packed aligned attribute, it can't be
>> changed, or older kernels won't be able to fill it out properly.
>> Neil
>
> It's more for supporting running 32-bits apps on 64-bit kernels
> (according to 20c9c825b12fc).
>
>   Marcelo

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[Marcelo Ricardo Leitner]

Hi,

On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
> Hi,
> 
> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>:
> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
> >> > From: Xin Long <lucien.xin@gmail.com>
> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
> >> >
> >> > >  struct sctp_paddrparams {
> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> >> > >   __u32                   spp_pathmtu;
> >> > >   __u32                   spp_sackdelay;
> >> > >   __u32                   spp_flags;
> >> > > + __u32                   spp_ipv6_flowlabel;
> >> > > + __u8                    spp_dscp;
> >> > >  } __attribute__((packed, aligned(4)));
> >> >
> >> > I don't think you can change the size of this structure like this.
> >> >
> >> > This check in sctp_setsockopt_peer_addr_params():
> >> >
> >> >     if (optlen != sizeof(struct sctp_paddrparams))
> >> >             return -EINVAL;
> >> >
> >> > is going to trigger in old kernels when executing programs
> >> > built against the new struct definition.
> >
> > That will happen, yes, but do we really care about being future-proof
> > here? I mean: if we also update such check(s) to support dealing with
> > smaller-than-supported structs, newer kernels will be able to run
> > programs built against the old struct, and the new one; while building
> > using newer headers and running on older kernel may fool the
> > application in other ways too (like enabling support for something
> > that is available on newer kernel and that is not present in the older
> > one).
> 
> We should not break existing apps.
> We still accept apps of pre-2.4 era without sin6_scope_id
> (e.g., net/ipv6/af_inet6.c:inet6_bind()).

Yes. That's what I tried to say. That is supporting an old app built
with old kernel headers and running on a newer kernel, and not the
other way around (an app built with fresh headers and running on an
old kernel).

> 
> >
> >> >
> >> I think thats also the reason its a packed aligned attribute, it can't be
> >> changed, or older kernels won't be able to fill it out properly.
> >> Neil
> >
> > It's more for supporting running 32-bits apps on 64-bit kernels
> > (according to 20c9c825b12fc).
> >
> >   Marcelo

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[Xin Long]

On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> Hi,
>
> On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
>> Hi,
>>
>> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>:
>> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>> >> > From: Xin Long <lucien.xin@gmail.com>
>> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>> >> >
>> >> > >  struct sctp_paddrparams {
>> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>> >> > >   __u32                   spp_pathmtu;
>> >> > >   __u32                   spp_sackdelay;
>> >> > >   __u32                   spp_flags;
>> >> > > + __u32                   spp_ipv6_flowlabel;
>> >> > > + __u8                    spp_dscp;
>> >> > >  } __attribute__((packed, aligned(4)));
>> >> >
>> >> > I don't think you can change the size of this structure like this.
>> >> >
>> >> > This check in sctp_setsockopt_peer_addr_params():
>> >> >
>> >> >     if (optlen != sizeof(struct sctp_paddrparams))
>> >> >             return -EINVAL;
>> >> >
>> >> > is going to trigger in old kernels when executing programs
>> >> > built against the new struct definition.
>> >
>> > That will happen, yes, but do we really care about being future-proof
>> > here? I mean: if we also update such check(s) to support dealing with
>> > smaller-than-supported structs, newer kernels will be able to run
>> > programs built against the old struct, and the new one; while building
>> > using newer headers and running on older kernel may fool the
>> > application in other ways too (like enabling support for something
>> > that is available on newer kernel and that is not present in the older
>> > one).
>>
>> We should not break existing apps.
>> We still accept apps of pre-2.4 era without sin6_scope_id
>> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
>
> Yes. That's what I tried to say. That is supporting an old app built
> with old kernel headers and running on a newer kernel, and not the
> other way around (an app built with fresh headers and running on an
> old kernel).
To make it, I will update the check like:

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 1df5d07..c949d8c 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2715,13 +2715,18 @@ static int
sctp_setsockopt_peer_addr_params(struct sock *sk,
        struct sctp_sock        *sp = sctp_sk(sk);
        int error;
        int hb_change, pmtud_change, sackdelay_change;
+       int plen = sizeof(params);
+       int old_plen = plen - sizeof(u32) * 2;

-       if (optlen != sizeof(struct sctp_paddrparams))
+       if (optlen != plen && optlen != old_plen)
                return -EINVAL;

        if (copy_from_user(&params, optval, optlen))
                return -EFAULT;

+       if (optlen == old_plen)
+               params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);
+
        /* Validate flags and value parameters. */
        hb_change        = params.spp_flags & SPP_HB;
        pmtud_change     = params.spp_flags & SPP_PMTUD;
@@ -5591,10 +5596,13 @@ static int
sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
        struct sctp_transport   *trans = NULL;
        struct sctp_association *asoc = NULL;
        struct sctp_sock        *sp = sctp_sk(sk);
+       int plen = sizeof(params);
+       int old_plen = plen - sizeof(u32) * 2;

-       if (len < sizeof(struct sctp_paddrparams))
+       if (len < old_plen)
                return -EINVAL;
-       len = sizeof(struct sctp_paddrparams);
+
+       len = len >= plen ? plen : old_plen;
        if (copy_from_user(&params, optval, len))
                return -EFAULT;

does it look ok to you?

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[吉藤英明]

2018-06-26 13:33 GMT+09:00 Xin Long <lucien.xin@gmail.com>:
> On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
>> Hi,
>>
>> On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
>>> Hi,
>>>
>>> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>:
>>> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>>> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>>> >> > From: Xin Long <lucien.xin@gmail.com>
>>> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>>> >> >
>>> >> > >  struct sctp_paddrparams {
>>> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>>> >> > >   __u32                   spp_pathmtu;
>>> >> > >   __u32                   spp_sackdelay;
>>> >> > >   __u32                   spp_flags;
>>> >> > > + __u32                   spp_ipv6_flowlabel;
>>> >> > > + __u8                    spp_dscp;
>>> >> > >  } __attribute__((packed, aligned(4)));
>>> >> >
>>> >> > I don't think you can change the size of this structure like this.
>>> >> >
>>> >> > This check in sctp_setsockopt_peer_addr_params():
>>> >> >
>>> >> >     if (optlen != sizeof(struct sctp_paddrparams))
>>> >> >             return -EINVAL;
>>> >> >
>>> >> > is going to trigger in old kernels when executing programs
>>> >> > built against the new struct definition.
>>> >
>>> > That will happen, yes, but do we really care about being future-proof
>>> > here? I mean: if we also update such check(s) to support dealing with
>>> > smaller-than-supported structs, newer kernels will be able to run
>>> > programs built against the old struct, and the new one; while building
>>> > using newer headers and running on older kernel may fool the
>>> > application in other ways too (like enabling support for something
>>> > that is available on newer kernel and that is not present in the older
>>> > one).
>>>
>>> We should not break existing apps.
>>> We still accept apps of pre-2.4 era without sin6_scope_id
>>> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
>>
>> Yes. That's what I tried to say. That is supporting an old app built
>> with old kernel headers and running on a newer kernel, and not the
>> other way around (an app built with fresh headers and running on an
>> old kernel).
> To make it, I will update the check like:
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 1df5d07..c949d8c 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2715,13 +2715,18 @@ static int
> sctp_setsockopt_peer_addr_params(struct sock *sk,
>         struct sctp_sock        *sp = sctp_sk(sk);
>         int error;
>         int hb_change, pmtud_change, sackdelay_change;
> +       int plen = sizeof(params);
> +       int old_plen = plen - sizeof(u32) * 2;

if (optlen < offsetof(struct sctp_paddrparams, spp_ipv6_flowlabel))
maybe?

>
> -       if (optlen != sizeof(struct sctp_paddrparams))
> +       if (optlen != plen && optlen != old_plen)
>                 return -EINVAL;
>
>         if (copy_from_user(&params, optval, optlen))
>                 return -EFAULT;
>
> +       if (optlen == old_plen)
> +               params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);

I think we should return -EINVAL if size is not new one.

--yoshfuji

> +
>         /* Validate flags and value parameters. */
>         hb_change        = params.spp_flags & SPP_HB;
>         pmtud_change     = params.spp_flags & SPP_PMTUD;
> @@ -5591,10 +5596,13 @@ static int
> sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
>         struct sctp_transport   *trans = NULL;
>         struct sctp_association *asoc = NULL;
>         struct sctp_sock        *sp = sctp_sk(sk);
> +       int plen = sizeof(params);
> +       int old_plen = plen - sizeof(u32) * 2;
>
> -       if (len < sizeof(struct sctp_paddrparams))
> +       if (len < old_plen)
>                 return -EINVAL;
> -       len = sizeof(struct sctp_paddrparams);
> +
> +       len = len >= plen ? plen : old_plen;
>         if (copy_from_user(&params, optval, len))
>                 return -EFAULT;
>
> does it look ok to you?

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[Xin Long]

On Tue, Jun 26, 2018 at 8:02 PM, 吉藤英明
<hideaki.yoshifuji@miraclelinux.com> wrote:
> 2018-06-26 13:33 GMT+09:00 Xin Long <lucien.xin@gmail.com>:
>> On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>>> Hi,
>>>
>>> On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
>>>> Hi,
>>>>
>>>> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>:
>>>> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>>>> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>>>> >> > From: Xin Long <lucien.xin@gmail.com>
>>>> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>>>> >> >
>>>> >> > >  struct sctp_paddrparams {
>>>> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>>>> >> > >   __u32                   spp_pathmtu;
>>>> >> > >   __u32                   spp_sackdelay;
>>>> >> > >   __u32                   spp_flags;
>>>> >> > > + __u32                   spp_ipv6_flowlabel;
>>>> >> > > + __u8                    spp_dscp;
>>>> >> > >  } __attribute__((packed, aligned(4)));
>>>> >> >
>>>> >> > I don't think you can change the size of this structure like this.
>>>> >> >
>>>> >> > This check in sctp_setsockopt_peer_addr_params():
>>>> >> >
>>>> >> >     if (optlen != sizeof(struct sctp_paddrparams))
>>>> >> >             return -EINVAL;
>>>> >> >
>>>> >> > is going to trigger in old kernels when executing programs
>>>> >> > built against the new struct definition.
>>>> >
>>>> > That will happen, yes, but do we really care about being future-proof
>>>> > here? I mean: if we also update such check(s) to support dealing with
>>>> > smaller-than-supported structs, newer kernels will be able to run
>>>> > programs built against the old struct, and the new one; while building
>>>> > using newer headers and running on older kernel may fool the
>>>> > application in other ways too (like enabling support for something
>>>> > that is available on newer kernel and that is not present in the older
>>>> > one).
>>>>
>>>> We should not break existing apps.
>>>> We still accept apps of pre-2.4 era without sin6_scope_id
>>>> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
>>>
>>> Yes. That's what I tried to say. That is supporting an old app built
>>> with old kernel headers and running on a newer kernel, and not the
>>> other way around (an app built with fresh headers and running on an
>>> old kernel).
>> To make it, I will update the check like:
>>
>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>> index 1df5d07..c949d8c 100644
>> --- a/net/sctp/socket.c
>> +++ b/net/sctp/socket.c
>> @@ -2715,13 +2715,18 @@ static int
>> sctp_setsockopt_peer_addr_params(struct sock *sk,
>>         struct sctp_sock        *sp = sctp_sk(sk);
>>         int error;
>>         int hb_change, pmtud_change, sackdelay_change;
>> +       int plen = sizeof(params);
>> +       int old_plen = plen - sizeof(u32) * 2;
>
> if (optlen < offsetof(struct sctp_paddrparams, spp_ipv6_flowlabel))
> maybe?
Hi, yoshfuji,
offsetof() is better. thank you.

>
>>
>> -       if (optlen != sizeof(struct sctp_paddrparams))
>> +       if (optlen != plen && optlen != old_plen)
>>                 return -EINVAL;
>>
>>         if (copy_from_user(&params, optval, optlen))
>>                 return -EFAULT;
>>
>> +       if (optlen == old_plen)
>> +               params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);
>
> I think we should return -EINVAL if size is not new one.
Sorry, if we returned  -EINVAL when size is the old one,
how can we guarantee an old app built with old kernel
headers and running on a newer kernel works well?
or you meant?
if ((params.spp_flags & (SPP_DSCP | SPP_IPV6_FLOWLABEL)) &&
    optlen != plen)
        return EINVAL;

>
> --yoshfuji
>
>> +
>>         /* Validate flags and value parameters. */
>>         hb_change        = params.spp_flags & SPP_HB;
>>         pmtud_change     = params.spp_flags & SPP_PMTUD;
>> @@ -5591,10 +5596,13 @@ static int
>> sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
>>         struct sctp_transport   *trans = NULL;
>>         struct sctp_association *asoc = NULL;
>>         struct sctp_sock        *sp = sctp_sk(sk);
>> +       int plen = sizeof(params);
>> +       int old_plen = plen - sizeof(u32) * 2;
>>
>> -       if (len < sizeof(struct sctp_paddrparams))
>> +       if (len < old_plen)
>>                 return -EINVAL;
>> -       len = sizeof(struct sctp_paddrparams);
>> +
>> +       len = len >= plen ? plen : old_plen;
>>         if (copy_from_user(&params, optval, len))
>>                 return -EFAULT;
>>
>> does it look ok to you?

Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

[吉藤英明]

Hi,

2018-06-28 15:40 GMT+09:00 Xin Long <lucien.xin@gmail.com>:
> On Tue, Jun 26, 2018 at 8:02 PM, 吉藤英明
> <hideaki.yoshifuji@miraclelinux.com> wrote:
>> 2018-06-26 13:33 GMT+09:00 Xin Long <lucien.xin@gmail.com>:
>>> On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
>>> <marcelo.leitner@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
>>>>> Hi,
>>>>>
>>>>> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>:
>>>>> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>>>>> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>>>>> >> > From: Xin Long <lucien.xin@gmail.com>
>>>>> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>>>>> >> >
>>>>> >> > >  struct sctp_paddrparams {
>>>>> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>>>>> >> > >   __u32                   spp_pathmtu;
>>>>> >> > >   __u32                   spp_sackdelay;
>>>>> >> > >   __u32                   spp_flags;
>>>>> >> > > + __u32                   spp_ipv6_flowlabel;
>>>>> >> > > + __u8                    spp_dscp;
>>>>> >> > >  } __attribute__((packed, aligned(4)));
>>>>> >> >
>>>>> >> > I don't think you can change the size of this structure like this.
>>>>> >> >
>>>>> >> > This check in sctp_setsockopt_peer_addr_params():
>>>>> >> >
>>>>> >> >     if (optlen != sizeof(struct sctp_paddrparams))
>>>>> >> >             return -EINVAL;
>>>>> >> >
>>>>> >> > is going to trigger in old kernels when executing programs
>>>>> >> > built against the new struct definition.
>>>>> >
>>>>> > That will happen, yes, but do we really care about being future-proof
>>>>> > here? I mean: if we also update such check(s) to support dealing with
>>>>> > smaller-than-supported structs, newer kernels will be able to run
>>>>> > programs built against the old struct, and the new one; while building
>>>>> > using newer headers and running on older kernel may fool the
>>>>> > application in other ways too (like enabling support for something
>>>>> > that is available on newer kernel and that is not present in the older
>>>>> > one).
>>>>>
>>>>> We should not break existing apps.
>>>>> We still accept apps of pre-2.4 era without sin6_scope_id
>>>>> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
>>>>
>>>> Yes. That's what I tried to say. That is supporting an old app built
>>>> with old kernel headers and running on a newer kernel, and not the
>>>> other way around (an app built with fresh headers and running on an
>>>> old kernel).
>>> To make it, I will update the check like:
>>>
>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>> index 1df5d07..c949d8c 100644
>>> --- a/net/sctp/socket.c
>>> +++ b/net/sctp/socket.c
>>> @@ -2715,13 +2715,18 @@ static int
>>> sctp_setsockopt_peer_addr_params(struct sock *sk,
>>>         struct sctp_sock        *sp = sctp_sk(sk);
>>>         int error;
>>>         int hb_change, pmtud_change, sackdelay_change;
>>> +       int plen = sizeof(params);
>>> +       int old_plen = plen - sizeof(u32) * 2;
>>
>> if (optlen < offsetof(struct sctp_paddrparams, spp_ipv6_flowlabel))
>> maybe?
> Hi, yoshfuji,
> offsetof() is better. thank you.
>
>>
>>>
>>> -       if (optlen != sizeof(struct sctp_paddrparams))
>>> +       if (optlen != plen && optlen != old_plen)
>>>                 return -EINVAL;
>>>
>>>         if (copy_from_user(&params, optval, optlen))
>>>                 return -EFAULT;
>>>
>>> +       if (optlen == old_plen)
>>> +               params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);
>>
>> I think we should return -EINVAL if size is not new one.
> Sorry, if we returned  -EINVAL when size is the old one,
> how can we guarantee an old app built with old kernel
> headers and running on a newer kernel works well?
> or you meant?
> if ((params.spp_flags & (SPP_DSCP | SPP_IPV6_FLOWLABEL)) &&
>     optlen != plen)
>         return EINVAL;

Yes, I meant this (it should be -EINVAL though).


>
>>
>> --yoshfuji
>>
>>> +
>>>         /* Validate flags and value parameters. */
>>>         hb_change        = params.spp_flags & SPP_HB;
>>>         pmtud_change     = params.spp_flags & SPP_PMTUD;
>>> @@ -5591,10 +5596,13 @@ static int
>>> sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
>>>         struct sctp_transport   *trans = NULL;
>>>         struct sctp_association *asoc = NULL;
>>>         struct sctp_sock        *sp = sctp_sk(sk);
>>> +       int plen = sizeof(params);
>>> +       int old_plen = plen - sizeof(u32) * 2;
>>>
>>> -       if (len < sizeof(struct sctp_paddrparams))
>>> +       if (len < old_plen)
>>>                 return -EINVAL;
>>> -       len = sizeof(struct sctp_paddrparams);
>>> +
>>> +       len = len >= plen ? plen : old_plen;
>>>         if (copy_from_user(&params, optval, len))
>>>                 return -EFAULT;
>>>
>>> does it look ok to you?

Re: [PATCH net-next 1/5] ipv4: add __ip_queue_xmit() that supports tos param

[David Miller]

From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 25 Jun 2018 10:14:33 +0800

> +EXPORT_SYMBOL(__ip_queue_xmit);
> +
> +int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
> +{
> +	return __ip_queue_xmit(sk, skb, fl, inet_sk(sk)->tos);
> +}
>  EXPORT_SYMBOL(ip_queue_xmit);

Maybe better to only export __ip_queue_xmit() and make ip_queue_xmit() an
inline function in net/ip.h?

Re: [PATCH net-next 1/5] ipv4: add __ip_queue_xmit() that supports tos param

[Neil Horman]

On Mon, Jun 25, 2018 at 04:26:54PM +0900, David Miller wrote:
> From: Xin Long <lucien.xin@gmail.com>
> Date: Mon, 25 Jun 2018 10:14:33 +0800
> 
> > +EXPORT_SYMBOL(__ip_queue_xmit);
> > +
> > +int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
> > +{
> > +	return __ip_queue_xmit(sk, skb, fl, inet_sk(sk)->tos);
> > +}
> >  EXPORT_SYMBOL(ip_queue_xmit);
> 
> Maybe better to only export __ip_queue_xmit() and make ip_queue_xmit() an
> inline function in net/ip.h?
> 
I concur.  No need to export both here

Neil

Re: [PATCH net-next 1/5] ipv4: add __ip_queue_xmit() that supports tos param

[Xin Long]

On Mon, Jun 25, 2018 at 7:13 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Mon, Jun 25, 2018 at 04:26:54PM +0900, David Miller wrote:
>> From: Xin Long <lucien.xin@gmail.com>
>> Date: Mon, 25 Jun 2018 10:14:33 +0800
>>
>> > +EXPORT_SYMBOL(__ip_queue_xmit);
>> > +
>> > +int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
>> > +{
>> > +   return __ip_queue_xmit(sk, skb, fl, inet_sk(sk)->tos);
>> > +}
>> >  EXPORT_SYMBOL(ip_queue_xmit);
>>
>> Maybe better to only export __ip_queue_xmit() and make ip_queue_xmit() an
>> inline function in net/ip.h?
>>
> I concur.  No need to export both here
>
will do that.