[PATCHv2 net-next 0/6] sctp: support sctp_diag in kernel

[PATCHv2 net-next 0/6] sctp: support sctp_diag in kernel

[Xin Long]

This patchset will add sctp_diag module to implement diag interface on
sctp in kernel.

For a listening sctp endpoint, we will just dump it's ep info.
For a sctp connection, we will the assoc info and it's ep info.

The ss dump will looks like:

[iproute2]# ./misc/ss --sctp  -n -l
State      Recv-Q Send-Q   Local Address:Port       Peer Address:Port
LISTEN     0      128      172.16.254.254:8888      *:*
LISTEN     0      5        127.0.0.1:1234           *:*
LISTEN     0      5        127.0.0.1:1234           *:*
  - ESTAB  0      0        127.0.0.1%lo:1234        127.0.0.1:4321
LISTEN     0      128      172.16.254.254:8888      *:*
  - ESTAB  0      0        172.16.254.254%eth1:8888 172.16.253.253:8888
  - ESTAB  0      0        172.16.254.254%eth1:8888 172.16.1.1:8888
  - ESTAB  0      0        172.16.254.254%eth1:8888 172.16.1.2:8888
  - ESTAB  0      0        172.16.254.254%eth1:8888 172.16.2.1:8888
  - ESTAB  0      0        172.16.254.254%eth1:8888 172.16.2.2:8888
  - ESTAB  0      0        172.16.254.254%eth1:8888 172.16.3.1:8888
  - ESTAB  0      0        172.16.254.254%eth1:8888 172.16.3.2:8888
LISTEN     0      0        127.0.0.1:4321           *:*
  - ESTAB  0      0        127.0.0.1%lo:4321        127.0.0.1:1234

The entries with '- ESTAB' are the assocs, some of them may belong to
the same endpoint. So we will dump the parent endpoint first, like the
entry with 'LISTEN'. then dump the assocs. ep and assocs entries will
be dumped in right order so that ss can show them in tree format easily.

Besides, this patchset also simplifies sctp proc codes, cause it has
some similar codes with sctp diag in sctp transport traversal.

v1->v2:
  1. inet_diag_get_handler needs to return it as const.
  2. merge 5/7 into 2/7 of v1.

Xin Long (6):
  sctp: add sctp_info dump api for sctp_diag
  sctp: export some apis or variables for sctp_diag and reuse some for
    proc
  sctp: export some functions for sctp_diag in inet_diag
  sctp: add the sctp_diag.c file
  sctp: merge the seq_start/next/exits in remaddrs and assocs
  sctp: fix some rhashtable functions using in sctp proc/diag

 include/linux/sctp.h           |  65 +++++
 include/net/sctp/sctp.h        |  16 ++
 include/uapi/linux/inet_diag.h |   2 +
 net/ipv4/inet_diag.c           |   9 +-
 net/sctp/Kconfig               |   4 +
 net/sctp/Makefile              |   1 +
 net/sctp/proc.c                | 104 ++------
 net/sctp/sctp_diag.c           | 581 +++++++++++++++++++++++++++++++++++++++++
 net/sctp/socket.c              | 215 +++++++++++++++
 9 files changed, 911 insertions(+), 86 deletions(-)
 create mode 100644 net/sctp/sctp_diag.c

-- 
2.1.0

[PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Xin Long]

sctp_diag will dump some important details of sctp's assoc or ep, we use
sctp_info to describe them,  sctp_get_sctp_info to get them, and export
it to sctp_diag.ko.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/linux/sctp.h    | 65 +++++++++++++++++++++++++++++++++++++
 include/net/sctp/sctp.h |  3 ++
 net/sctp/socket.c       | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+)

diff --git a/include/linux/sctp.h b/include/linux/sctp.h
index a9414fd..a448ebc 100644
--- a/include/linux/sctp.h
+++ b/include/linux/sctp.h
@@ -705,4 +705,69 @@ typedef struct sctp_auth_chunk {
 	sctp_authhdr_t auth_hdr;
 } __packed sctp_auth_chunk_t;
 
+struct sctp_info {
+	__u32	sctpi_tag;
+	__u32	sctpi_state;
+	__u32	sctpi_rwnd;
+	__u16	sctpi_unackdata;
+	__u16	sctpi_penddata;
+	__u16	sctpi_instrms;
+	__u16	sctpi_outstrms;
+	__u32	sctpi_fragmentation_point;
+	__u32	sctpi_inqueue;
+	__u32	sctpi_outqueue;
+	__u32	sctpi_overall_error;
+	__u32	sctpi_max_burst;
+	__u32	sctpi_maxseg;
+	__u32	sctpi_peer_rwnd;
+	__u32	sctpi_peer_tag;
+	__u8	sctpi_peer_capable;
+	__u8	sctpi_peer_sack;
+
+	/* assoc status info */
+	__u64	sctpi_isacks;
+	__u64	sctpi_osacks;
+	__u64	sctpi_opackets;
+	__u64	sctpi_ipackets;
+	__u64	sctpi_rtxchunks;
+	__u64	sctpi_outofseqtsns;
+	__u64	sctpi_idupchunks;
+	__u64	sctpi_gapcnt;
+	__u64	sctpi_ouodchunks;
+	__u64	sctpi_iuodchunks;
+	__u64	sctpi_oodchunks;
+	__u64	sctpi_iodchunks;
+	__u64	sctpi_octrlchunks;
+	__u64	sctpi_ictrlchunks;
+
+	/* primary transport info */
+	struct sockaddr_storage	sctpi_p_address;
+	__s32	sctpi_p_state;
+	__u32	sctpi_p_cwnd;
+	__u32	sctpi_p_srtt;
+	__u32	sctpi_p_rto;
+	__u32	sctpi_p_hbinterval;
+	__u32	sctpi_p_pathmaxrxt;
+	__u32	sctpi_p_sackdelay;
+	__u32	sctpi_p_sackfreq;
+	__u32	sctpi_p_ssthresh;
+	__u32	sctpi_p_partial_bytes_acked;
+	__u32	sctpi_p_flight_size;
+	__u16	sctpi_p_error;
+
+	/* sctp sock info */
+	__u32	sctpi_s_autoclose;
+	__u32	sctpi_s_adaptation_ind;
+	__u32	sctpi_s_pd_point;
+	__u8	sctpi_s_nodelay;
+	__u8	sctpi_s_disable_fragments;
+	__u8	sctpi_s_v4mapped;
+	__u8	sctpi_s_frag_interleave;
+};
+
+struct sctp_infox {
+	struct sctp_info *sctpinfo;
+	struct sctp_association *asoc;
+};
+
 #endif /* __LINUX_SCTP_H__ */
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 65521cf..36e1eae 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -116,6 +116,9 @@ extern struct percpu_counter sctp_sockets_allocated;
 int sctp_asconf_mgmt(struct sctp_sock *, struct sctp_sockaddr_entry *);
 struct sk_buff *sctp_skb_recv_datagram(struct sock *, int, int, int *);
 
+int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
+		       struct sctp_info *info);
+
 /*
  * sctp/primitive.c
  */
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 878d28e..8f79f23 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4202,6 +4202,92 @@ static void sctp_shutdown(struct sock *sk, int how)
 	}
 }
 
+int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
+		       struct sctp_info *info)
+{
+	struct sctp_transport *prim;
+	struct list_head *pos, *temp;
+	int mask;
+
+	memset(info, 0, sizeof(*info));
+	if (!asoc) {
+		struct sctp_sock *sp = sctp_sk(sk);
+
+		info->sctpi_s_autoclose = sp->autoclose;
+		info->sctpi_s_adaptation_ind = sp->adaptation_ind;
+		info->sctpi_s_pd_point = sp->pd_point;
+		info->sctpi_s_nodelay = sp->nodelay;
+		info->sctpi_s_disable_fragments = sp->disable_fragments;
+		info->sctpi_s_v4mapped = sp->v4mapped;
+		info->sctpi_s_frag_interleave = sp->frag_interleave;
+
+		return 0;
+	}
+
+	info->sctpi_tag = asoc->c.my_vtag;
+	info->sctpi_state = asoc->state;
+	info->sctpi_rwnd = asoc->a_rwnd;
+	info->sctpi_unackdata = asoc->unack_data;
+	info->sctpi_penddata = sctp_tsnmap_pending(&asoc->peer.tsn_map);
+	info->sctpi_instrms = asoc->c.sinit_max_instreams;
+	info->sctpi_outstrms = asoc->c.sinit_num_ostreams;
+	list_for_each_safe(pos, temp, &asoc->base.inqueue.in_chunk_list)
+		info->sctpi_inqueue++;
+	list_for_each_safe(pos, temp, &asoc->outqueue.out_chunk_list)
+		info->sctpi_outqueue++;
+	info->sctpi_overall_error = asoc->overall_error_count;
+	info->sctpi_max_burst = asoc->max_burst;
+	info->sctpi_maxseg = asoc->frag_point;
+	info->sctpi_peer_rwnd = asoc->peer.rwnd;
+	info->sctpi_peer_tag = asoc->c.peer_vtag;
+
+	mask = asoc->peer.ecn_capable << 1;
+	mask = (mask | asoc->peer.ipv4_address) << 1;
+	mask = (mask | asoc->peer.ipv6_address) << 1;
+	mask = (mask | asoc->peer.hostname_address) << 1;
+	mask = (mask | asoc->peer.asconf_capable) << 1;
+	mask = (mask | asoc->peer.prsctp_capable) << 1;
+	mask = (mask | asoc->peer.auth_capable);
+	info->sctpi_peer_capable = mask;
+	mask = asoc->peer.sack_needed << 1;
+	mask = (mask | asoc->peer.sack_generation) << 1;
+	mask = (mask | asoc->peer.zero_window_announced);
+	info->sctpi_peer_sack = mask;
+
+	info->sctpi_isacks = asoc->stats.isacks;
+	info->sctpi_osacks = asoc->stats.osacks;
+	info->sctpi_opackets = asoc->stats.opackets;
+	info->sctpi_ipackets = asoc->stats.ipackets;
+	info->sctpi_rtxchunks = asoc->stats.rtxchunks;
+	info->sctpi_outofseqtsns = asoc->stats.outofseqtsns;
+	info->sctpi_idupchunks = asoc->stats.idupchunks;
+	info->sctpi_gapcnt = asoc->stats.gapcnt;
+	info->sctpi_ouodchunks = asoc->stats.ouodchunks;
+	info->sctpi_iuodchunks = asoc->stats.iuodchunks;
+	info->sctpi_oodchunks = asoc->stats.oodchunks;
+	info->sctpi_iodchunks = asoc->stats.iodchunks;
+	info->sctpi_octrlchunks = asoc->stats.octrlchunks;
+	info->sctpi_ictrlchunks = asoc->stats.ictrlchunks;
+
+	prim = asoc->peer.primary_path;
+	memcpy(&info->sctpi_p_address, &prim->ipaddr,
+	       sizeof(struct sockaddr_storage));
+	info->sctpi_p_state = prim->state;
+	info->sctpi_p_cwnd = prim->cwnd;
+	info->sctpi_p_srtt = prim->srtt;
+	info->sctpi_p_rto = jiffies_to_msecs(prim->rto);
+	info->sctpi_p_hbinterval = prim->hbinterval;
+	info->sctpi_p_pathmaxrxt = prim->pathmaxrxt;
+	info->sctpi_p_sackdelay = jiffies_to_msecs(prim->sackdelay);
+	info->sctpi_p_ssthresh = prim->ssthresh;
+	info->sctpi_p_partial_bytes_acked = prim->partial_bytes_acked;
+	info->sctpi_p_flight_size = prim->flight_size;
+	info->sctpi_p_error = prim->error_count;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sctp_get_sctp_info);
+
 /* 7.2.1 Association Status (SCTP_STATUS)
 
  * Applications can retrieve current status information about an
-- 
2.1.0

[PATCHv2 net-next 2/6] sctp: export some apis or variables for sctp_diag and reuse some for proc

[Xin Long]

For some main variables in sctp.ko, we couldn't export it to other modules,
so we have to define some api to access them.

It will include sctp transport and endpoint's traversal.

There are some transport traversal functions for sctp_diag, we can also
use it for sctp_proc. cause they have the similar situation to traversal
transport.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/sctp.h |  13 +++++
 net/sctp/proc.c         |  80 +++++++------------------------
 net/sctp/socket.c       | 124 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+), 62 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 36e1eae..c0c4deb 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -116,6 +116,19 @@ extern struct percpu_counter sctp_sockets_allocated;
 int sctp_asconf_mgmt(struct sctp_sock *, struct sctp_sockaddr_entry *);
 struct sk_buff *sctp_skb_recv_datagram(struct sock *, int, int, int *);
 
+int sctp_transport_walk_start(struct rhashtable_iter *iter);
+void sctp_transport_walk_stop(struct rhashtable_iter *iter);
+struct sctp_transport *sctp_transport_get_next(struct net *net,
+			struct rhashtable_iter *iter);
+struct sctp_transport *sctp_transport_get_idx(struct net *net,
+			struct rhashtable_iter *iter, int pos);
+int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
+				  struct net *net,
+				  const union sctp_addr *laddr,
+				  const union sctp_addr *paddr, void *p);
+int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
+			    struct net *net, int pos, void *p);
+int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), void *p);
 int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
 		       struct sctp_info *info);
 
diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index 5cfac8d..dd8492f 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -282,80 +282,31 @@ struct sctp_ht_iter {
 	struct rhashtable_iter hti;
 };
 
-static struct sctp_transport *sctp_transport_get_next(struct seq_file *seq)
-{
-	struct sctp_ht_iter *iter = seq->private;
-	struct sctp_transport *t;
-
-	t = rhashtable_walk_next(&iter->hti);
-	for (; t; t = rhashtable_walk_next(&iter->hti)) {
-		if (IS_ERR(t)) {
-			if (PTR_ERR(t) == -EAGAIN)
-				continue;
-			break;
-		}
-
-		if (net_eq(sock_net(t->asoc->base.sk), seq_file_net(seq)) &&
-		    t->asoc->peer.primary_path == t)
-			break;
-	}
-
-	return t;
-}
-
-static struct sctp_transport *sctp_transport_get_idx(struct seq_file *seq,
-						     loff_t pos)
-{
-	void *obj = SEQ_START_TOKEN;
-
-	while (pos && (obj = sctp_transport_get_next(seq)) && !IS_ERR(obj))
-		pos--;
-
-	return obj;
-}
-
-static int sctp_transport_walk_start(struct seq_file *seq)
-{
-	struct sctp_ht_iter *iter = seq->private;
-	int err;
-
-	err = rhashtable_walk_init(&sctp_transport_hashtable, &iter->hti);
-	if (err)
-		return err;
-
-	err = rhashtable_walk_start(&iter->hti);
-
-	return err == -EAGAIN ? 0 : err;
-}
-
-static void sctp_transport_walk_stop(struct seq_file *seq)
-{
-	struct sctp_ht_iter *iter = seq->private;
-
-	rhashtable_walk_stop(&iter->hti);
-	rhashtable_walk_exit(&iter->hti);
-}
-
 static void *sctp_assocs_seq_start(struct seq_file *seq, loff_t *pos)
 {
-	int err = sctp_transport_walk_start(seq);
+	struct sctp_ht_iter *iter = seq->private;
+	int err = sctp_transport_walk_start(&iter->hti);
 
 	if (err)
 		return ERR_PTR(err);
 
-	return sctp_transport_get_idx(seq, *pos);
+	return sctp_transport_get_idx(seq_file_net(seq), &iter->hti, *pos);
 }
 
 static void sctp_assocs_seq_stop(struct seq_file *seq, void *v)
 {
-	sctp_transport_walk_stop(seq);
+	struct sctp_ht_iter *iter = seq->private;
+
+	sctp_transport_walk_stop(&iter->hti);
 }
 
 static void *sctp_assocs_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
+	struct sctp_ht_iter *iter = seq->private;
+
 	++*pos;
 
-	return sctp_transport_get_next(seq);
+	return sctp_transport_get_next(seq_file_net(seq), &iter->hti);
 }
 
 /* Display sctp associations (/proc/net/sctp/assocs). */
@@ -457,24 +408,29 @@ void sctp_assocs_proc_exit(struct net *net)
 
 static void *sctp_remaddr_seq_start(struct seq_file *seq, loff_t *pos)
 {
-	int err = sctp_transport_walk_start(seq);
+	struct sctp_ht_iter *iter = seq->private;
+	int err = sctp_transport_walk_start(&iter->hti);
 
 	if (err)
 		return ERR_PTR(err);
 
-	return sctp_transport_get_idx(seq, *pos);
+	return sctp_transport_get_idx(seq_file_net(seq), &iter->hti, *pos);
 }
 
 static void *sctp_remaddr_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
+	struct sctp_ht_iter *iter = seq->private;
+
 	++*pos;
 
-	return sctp_transport_get_next(seq);
+	return sctp_transport_get_next(seq_file_net(seq), &iter->hti);
 }
 
 static void sctp_remaddr_seq_stop(struct seq_file *seq, void *v)
 {
-	sctp_transport_walk_stop(seq);
+	struct sctp_ht_iter *iter = seq->private;
+
+	sctp_transport_walk_stop(&iter->hti);
 }
 
 static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 8f79f23..b0bf6c7 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4288,6 +4288,130 @@ int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
 }
 EXPORT_SYMBOL_GPL(sctp_get_sctp_info);
 
+/* use callback to avoid exporting the core structure */
+int sctp_transport_walk_start(struct rhashtable_iter *iter)
+{
+	int err;
+
+	err = rhashtable_walk_init(&sctp_transport_hashtable, iter);
+	if (err)
+		return err;
+
+	err = rhashtable_walk_start(iter);
+
+	return err == -EAGAIN ? 0 : err;
+}
+
+void sctp_transport_walk_stop(struct rhashtable_iter *iter)
+{
+	rhashtable_walk_stop(iter);
+	rhashtable_walk_exit(iter);
+}
+
+struct sctp_transport *sctp_transport_get_next(struct net *net,
+					       struct rhashtable_iter *iter)
+{
+	struct sctp_transport *t;
+
+	t = rhashtable_walk_next(iter);
+	for (; t; t = rhashtable_walk_next(iter)) {
+		if (IS_ERR(t)) {
+			if (PTR_ERR(t) == -EAGAIN)
+				continue;
+			break;
+		}
+
+		if (net_eq(sock_net(t->asoc->base.sk), net) &&
+		    t->asoc->peer.primary_path == t)
+			break;
+	}
+
+	return t;
+}
+
+struct sctp_transport *sctp_transport_get_idx(struct net *net,
+					      struct rhashtable_iter *iter,
+					      int pos)
+{
+	void *obj = SEQ_START_TOKEN;
+
+	while (pos && (obj = sctp_transport_get_next(net, iter)) &&
+	       !IS_ERR(obj))
+		pos--;
+
+	return obj;
+}
+
+int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *),
+			   void *p) {
+	int err = 0;
+	int hash = 0;
+	struct sctp_ep_common *epb;
+	struct sctp_hashbucket *head;
+
+	for (head = sctp_ep_hashtable; hash < sctp_ep_hashsize;
+	     hash++, head++) {
+		read_lock(&head->lock);
+		sctp_for_each_hentry(epb, &head->chain) {
+			err = cb(sctp_ep(epb), p);
+			if (err)
+				break;
+		}
+		read_unlock(&head->lock);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(sctp_for_each_endpoint);
+
+int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
+				  struct net *net,
+				  const union sctp_addr *laddr,
+				  const union sctp_addr *paddr, void *p)
+{
+	struct sctp_transport *transport;
+	int err = 0;
+
+	rcu_read_lock();
+	transport = sctp_addrs_lookup_transport(net, laddr, paddr);
+	if (!transport || !sctp_transport_hold(transport))
+		goto out;
+	err = cb(transport, p);
+	sctp_transport_put(transport);
+
+out:
+	rcu_read_unlock();
+	return err;
+}
+EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);
+
+int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
+			    struct net *net, int pos, void *p) {
+	struct rhashtable_iter hti;
+	int err = 0;
+	void *obj;
+
+	if (sctp_transport_walk_start(&hti))
+		goto out;
+
+	sctp_transport_get_idx(net, &hti, pos);
+	obj = sctp_transport_get_next(net, &hti);
+	for (; obj && !IS_ERR(obj); obj = sctp_transport_get_next(net, &hti)) {
+		struct sctp_transport *transport = obj;
+
+		if (!sctp_transport_hold(transport))
+			continue;
+		err = cb(transport, p);
+		sctp_transport_put(transport);
+		if (err)
+			break;
+	}
+out:
+	sctp_transport_walk_stop(&hti);
+	return err;
+}
+EXPORT_SYMBOL_GPL(sctp_for_each_transport);
+
 /* 7.2.1 Association Status (SCTP_STATUS)
 
  * Applications can retrieve current status information about an
-- 
2.1.0

[PATCHv2 net-next 3/6] sctp: export some functions for sctp_diag in inet_diag

[Xin Long]

inet_diag_msg_common_fill is used to fill the diag msg common info,
we need to use it in sctp_diag as well, so export it.

We also add inet_diag_get_handler() to access inet_diag_table in sctp
diag.

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

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index bd591eb..5a0bfe0 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -66,7 +66,13 @@ static void inet_diag_unlock_handler(const struct inet_diag_handler *handler)
 	mutex_unlock(&inet_diag_table_mutex);
 }
 
-static void inet_diag_msg_common_fill(struct inet_diag_msg *r, struct sock *sk)
+const struct inet_diag_handler *inet_diag_get_handler(int proto)
+{
+	return inet_diag_table[proto];
+}
+EXPORT_SYMBOL_GPL(inet_diag_get_handler);
+
+void inet_diag_msg_common_fill(struct inet_diag_msg *r, struct sock *sk)
 {
 	r->idiag_family = sk->sk_family;
 
@@ -89,6 +95,7 @@ static void inet_diag_msg_common_fill(struct inet_diag_msg *r, struct sock *sk)
 	r->id.idiag_dst[0] = sk->sk_daddr;
 	}
 }
+EXPORT_SYMBOL_GPL(inet_diag_msg_common_fill);
 
 static size_t inet_sk_attr_size(void)
 {
-- 
2.1.0

[PATCHv2 net-next 4/6] sctp: add the sctp_diag.c file

[Xin Long]

This one will implement all the interface of inet_diag, inet_diag_handler.
which includes sctp_diag_dump, sctp_diag_dump_one and sctp_diag_get_info.

It will work as a modules, and register inet_diag_handler when loading.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/inet_diag.h |   2 +
 net/sctp/Kconfig               |   4 +
 net/sctp/Makefile              |   1 +
 net/sctp/sctp_diag.c           | 581 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 588 insertions(+)
 create mode 100644 net/sctp/sctp_diag.c

diff --git a/include/uapi/linux/inet_diag.h b/include/uapi/linux/inet_diag.h
index 68a1f71..f5f3629 100644
--- a/include/uapi/linux/inet_diag.h
+++ b/include/uapi/linux/inet_diag.h
@@ -113,6 +113,8 @@ enum {
 	INET_DIAG_DCTCPINFO,
 	INET_DIAG_PROTOCOL,  /* response attribute only */
 	INET_DIAG_SKV6ONLY,
+	INET_DIAG_LOCALS,
+	INET_DIAG_PEERS,
 };
 
 #define INET_DIAG_MAX INET_DIAG_SKV6ONLY
diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
index 71c1a59..d9c04dc 100644
--- a/net/sctp/Kconfig
+++ b/net/sctp/Kconfig
@@ -99,5 +99,9 @@ config SCTP_COOKIE_HMAC_SHA1
 	select CRYPTO_HMAC if SCTP_COOKIE_HMAC_SHA1
 	select CRYPTO_SHA1 if SCTP_COOKIE_HMAC_SHA1
 
+config INET_SCTP_DIAG
+	depends on INET_DIAG
+	def_tristate INET_DIAG
+
 
 endif # IP_SCTP
diff --git a/net/sctp/Makefile b/net/sctp/Makefile
index 3b4ffb0..0fca582 100644
--- a/net/sctp/Makefile
+++ b/net/sctp/Makefile
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_IP_SCTP) += sctp.o
 obj-$(CONFIG_NET_SCTPPROBE) += sctp_probe.o
+obj-$(CONFIG_INET_SCTP_DIAG) += sctp_diag.o
 
 sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \
 	  protocol.o endpointola.o associola.o \
diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
new file mode 100644
index 0000000..86fccc2
--- /dev/null
+++ b/net/sctp/sctp_diag.c
@@ -0,0 +1,581 @@
+#include <linux/module.h>
+#include <linux/inet_diag.h>
+#include <linux/sock_diag.h>
+#include <net/sctp/sctp.h>
+
+extern const struct inet_diag_handler *inet_diag_get_handler(int proto);
+extern void inet_diag_msg_common_fill(struct inet_diag_msg *r,
+				      struct sock *sk);
+
+static int inet_sctp_fill_laddrs(struct sk_buff *skb,
+				 struct list_head *address_list)
+{
+	struct sctp_sockaddr_entry *laddr;
+	int addrlen = sizeof(struct sockaddr_storage);
+	int addrcnt = 0;
+	struct nlattr *attr;
+	void *info = NULL;
+
+	list_for_each_entry_rcu(laddr, address_list, list)
+		addrcnt++;
+
+	attr = nla_reserve(skb, INET_DIAG_LOCALS, addrlen * addrcnt);
+	if (!attr)
+		return -EMSGSIZE;
+
+	info = nla_data(attr);
+	list_for_each_entry_rcu(laddr, address_list, list) {
+		memcpy(info, &laddr->a, addrlen);
+		info += addrlen;
+	}
+
+	return 0;
+}
+
+static int inet_sctp_fill_paddrs(struct sk_buff *skb,
+				 struct sctp_association *asoc)
+{
+	int addrlen = sizeof(struct sockaddr_storage);
+	struct sctp_transport *from;
+	struct nlattr *attr;
+	void *info = NULL;
+
+	attr = nla_reserve(skb, INET_DIAG_PEERS,
+			   addrlen * asoc->peer.transport_count);
+	if (!attr)
+		return -EMSGSIZE;
+
+	info = nla_data(attr);
+	list_for_each_entry(from, &asoc->peer.transport_addr_list,
+			    transports) {
+		memcpy(info, &from->ipaddr, addrlen);
+		info += addrlen;
+	}
+
+	return 0;
+}
+
+static int inet_assoc_diag_fill(struct sock *sk,
+				struct sctp_association *asoc,
+				struct sk_buff *skb,
+				const struct inet_diag_req_v2 *req,
+				struct user_namespace *user_ns,
+				int portid, u32 seq, u16 nlmsg_flags,
+				const struct nlmsghdr *unlh)
+{
+	const struct inet_sock *inet = inet_sk(sk);
+	const struct inet_diag_handler *handler;
+	int ext = req->idiag_ext;
+	struct inet_diag_msg *r;
+	struct nlmsghdr  *nlh;
+	struct nlattr *attr;
+	void *info = NULL;
+	union sctp_addr laddr, paddr;
+	struct dst_entry *dst;
+	struct sctp_infox infox;
+
+	handler = inet_diag_get_handler(req->sdiag_protocol);
+	BUG_ON(!handler);
+
+	nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
+			nlmsg_flags);
+	if (!nlh)
+		return -EMSGSIZE;
+
+	r = nlmsg_data(nlh);
+	BUG_ON(!sk_fullsock(sk));
+
+	laddr = list_entry(asoc->base.bind_addr.address_list.next,
+			   struct sctp_sockaddr_entry, list)->a;
+	paddr = asoc->peer.primary_path->ipaddr;
+	dst = asoc->peer.primary_path->dst;
+
+	r->idiag_family = sk->sk_family;
+	r->id.idiag_sport = htons(asoc->base.bind_addr.port);
+	r->id.idiag_dport = htons(asoc->peer.port);
+	r->id.idiag_if = dst ? dst->dev->ifindex : 0;
+	sock_diag_save_cookie(sk, r->id.idiag_cookie);
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (sk->sk_family == AF_INET6) {
+		*(struct in6_addr *)r->id.idiag_src = laddr.v6.sin6_addr;
+		*(struct in6_addr *)r->id.idiag_dst = paddr.v6.sin6_addr;
+	} else
+#endif
+	{
+		memset(&r->id.idiag_src, 0, sizeof(r->id.idiag_src));
+		memset(&r->id.idiag_dst, 0, sizeof(r->id.idiag_dst));
+
+		r->id.idiag_src[0] = laddr.v4.sin_addr.s_addr;
+		r->id.idiag_dst[0] = paddr.v4.sin_addr.s_addr;
+	}
+
+	r->idiag_state = asoc->state;
+	r->idiag_timer = SCTP_EVENT_TIMEOUT_T3_RTX;
+	r->idiag_retrans = asoc->rtx_data_chunks;
+#define EXPIRES_IN_MS(tmo)  DIV_ROUND_UP((tmo - jiffies) * 1000, HZ)
+	r->idiag_expires =
+		EXPIRES_IN_MS(asoc->timeouts[SCTP_EVENT_TIMEOUT_T3_RTX]);
+#undef EXPIRES_IN_MS
+
+	if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
+		goto errout;
+
+	/* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
+	 * hence this needs to be included regardless of socket family.
+	 */
+	if (ext & (1 << (INET_DIAG_TOS - 1)))
+		if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
+			goto errout;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (r->idiag_family == AF_INET6) {
+		if (ext & (1 << (INET_DIAG_TCLASS - 1)))
+			if (nla_put_u8(skb, INET_DIAG_TCLASS,
+				       inet6_sk(sk)->tclass) < 0)
+				goto errout;
+
+		if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
+		    nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
+			goto errout;
+	}
+#endif
+
+	r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
+	r->idiag_inode = sock_i_ino(sk);
+
+	if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
+		struct inet_diag_meminfo minfo = {
+			.idiag_rmem = sk_rmem_alloc_get(sk),
+			.idiag_wmem = sk->sk_wmem_queued,
+			.idiag_fmem = sk->sk_forward_alloc,
+			.idiag_tmem = sk_wmem_alloc_get(sk),
+		};
+
+		if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
+			goto errout;
+	}
+
+	if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
+		if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
+			goto errout;
+
+	if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
+		attr = nla_reserve(skb, INET_DIAG_INFO,
+				   handler->idiag_info_size);
+		if (!attr)
+			goto errout;
+
+		info = nla_data(attr);
+	}
+	infox.sctpinfo = (struct sctp_info *)info;
+	infox.asoc = asoc;
+	handler->idiag_get_info(sk, r, &infox);
+
+	if (ext & (1 << (INET_DIAG_CONG - 1)))
+		if (nla_put_string(skb, INET_DIAG_CONG, "reno") < 0)
+			goto errout;
+
+	if (inet_sctp_fill_laddrs(skb, &asoc->base.bind_addr.address_list))
+		goto errout;
+
+	if (inet_sctp_fill_paddrs(skb, asoc))
+		goto errout;
+
+	nlmsg_end(skb, nlh);
+	return 0;
+
+errout:
+	nlmsg_cancel(skb, nlh);
+	return -EMSGSIZE;
+}
+
+static int inet_ep_diag_fill(struct sock *sk, struct sctp_endpoint *ep,
+			     struct sk_buff *skb,
+			     const struct inet_diag_req_v2 *req,
+			     struct user_namespace *user_ns,
+			     u32 portid, u32 seq, u16 nlmsg_flags,
+			     const struct nlmsghdr *unlh)
+{
+	const struct inet_sock *inet = inet_sk(sk);
+	const struct inet_diag_handler *handler;
+	int ext = req->idiag_ext;
+	struct inet_diag_msg *r;
+	struct nlmsghdr  *nlh;
+	struct nlattr *attr;
+	void *info = NULL;
+	struct sctp_infox infox;
+
+	handler = inet_diag_get_handler(req->sdiag_protocol);
+	BUG_ON(!handler);
+
+	nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
+			nlmsg_flags);
+	if (!nlh)
+		return -EMSGSIZE;
+
+	r = nlmsg_data(nlh);
+	BUG_ON(!sk_fullsock(sk));
+
+	inet_diag_msg_common_fill(r, sk);
+	r->idiag_state = sk->sk_state;
+	r->idiag_timer = 0;
+	r->idiag_retrans = 0;
+
+	if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
+		goto errout;
+
+	/* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
+	 * hence this needs to be included regardless of socket family.
+	 */
+	if (ext & (1 << (INET_DIAG_TOS - 1)))
+		if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
+			goto errout;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (r->idiag_family == AF_INET6) {
+		if (ext & (1 << (INET_DIAG_TCLASS - 1)))
+			if (nla_put_u8(skb, INET_DIAG_TCLASS,
+				       inet6_sk(sk)->tclass) < 0)
+				goto errout;
+
+		if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
+		    nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
+			goto errout;
+	}
+#endif
+
+	r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
+	r->idiag_inode = sock_i_ino(sk);
+
+	if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
+		struct inet_diag_meminfo minfo = {
+			.idiag_rmem = sk_rmem_alloc_get(sk),
+			.idiag_wmem = sk->sk_wmem_queued,
+			.idiag_fmem = sk->sk_forward_alloc,
+			.idiag_tmem = sk_wmem_alloc_get(sk),
+		};
+
+		if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
+			goto errout;
+	}
+
+	if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
+		if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
+			goto errout;
+
+	if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
+		attr = nla_reserve(skb, INET_DIAG_INFO,
+				   handler->idiag_info_size);
+		if (!attr)
+			goto errout;
+
+		info = nla_data(attr);
+	}
+	infox.sctpinfo = (struct sctp_info *)info;
+	infox.asoc = NULL;
+	handler->idiag_get_info(sk, r, &infox);
+
+	if (inet_sctp_fill_laddrs(skb, &ep->base.bind_addr.address_list))
+		goto errout;
+
+	nlmsg_end(skb, nlh);
+	return 0;
+
+errout:
+	nlmsg_cancel(skb, nlh);
+	return -EMSGSIZE;
+}
+
+static size_t inet_assoc_attr_size(struct sctp_association *asoc)
+{
+	int addrlen = sizeof(struct sockaddr_storage);
+	int addrcnt = 0;
+	struct sctp_sockaddr_entry *laddr;
+
+	list_for_each_entry_rcu(laddr, &asoc->base.bind_addr.address_list,
+				list)
+		addrcnt++;
+
+	return	  nla_total_size(sizeof(struct tcp_info))
+		+ nla_total_size(1) /* INET_DIAG_SHUTDOWN */
+		+ nla_total_size(1) /* INET_DIAG_TOS */
+		+ nla_total_size(1) /* INET_DIAG_TCLASS */
+		+ nla_total_size(addrlen * asoc->peer.transport_count)
+		+ nla_total_size(addrlen * addrcnt)
+		+ nla_total_size(sizeof(struct inet_diag_meminfo))
+		+ nla_total_size(sizeof(struct inet_diag_msg))
+		+ nla_total_size(sizeof(struct sctp_info))
+		+ 64;
+}
+
+/* callback and param */
+struct sctp_comm_param {
+	struct sk_buff *skb;
+	struct netlink_callback *cb;
+	const struct inet_diag_req_v2 *r;
+	const struct nlmsghdr *nlh;
+};
+
+static int sctp_tsp_dump_one(struct sctp_transport *tsp, void *p)
+{
+	struct sctp_association *assoc = tsp->asoc;
+	struct sock *sk = tsp->asoc->base.sk;
+	struct sctp_comm_param *commp = p;
+	struct sk_buff *in_skb = commp->skb;
+	const struct inet_diag_req_v2 *req = commp->r;
+	const struct nlmsghdr *nlh = commp->nlh;
+	struct net *net = sock_net(in_skb->sk);
+	struct sk_buff *rep;
+	int err;
+
+	err = sock_diag_check_cookie(sk, req->id.idiag_cookie);
+	if (err)
+		goto out;
+
+	err = -ENOMEM;
+	rep = nlmsg_new(inet_assoc_attr_size(assoc), GFP_KERNEL);
+	if (!rep)
+		goto out;
+
+	err = inet_assoc_diag_fill(sk, assoc, rep, req,
+				   sk_user_ns(NETLINK_CB(in_skb).sk),
+				   NETLINK_CB(in_skb).portid,
+				   nlh->nlmsg_seq, 0, nlh);
+	if (err < 0) {
+		WARN_ON(err == -EMSGSIZE);
+		kfree_skb(rep);
+		goto out;
+	}
+
+	err = netlink_unicast(net->diag_nlsk, rep, NETLINK_CB(in_skb).portid,
+			      MSG_DONTWAIT);
+	if (err > 0)
+		err = 0;
+out:
+	return err;
+}
+
+static int sctp_tsp_dump(struct sctp_transport *tsp, void *p)
+{
+	struct sctp_endpoint *ep = tsp->asoc->ep;
+	struct sctp_comm_param *commp = p;
+	struct sock *sk = ep->base.sk;
+	struct sk_buff *skb = commp->skb;
+	struct netlink_callback *cb = commp->cb;
+	const struct inet_diag_req_v2 *r = commp->r;
+	struct sctp_association *assoc =
+		list_entry(ep->asocs.next, struct sctp_association, asocs);
+	int err = 0;
+
+	if (tsp->asoc != assoc)
+		goto out;
+
+	if (r->sdiag_family != AF_UNSPEC && sk->sk_family != r->sdiag_family)
+		goto out;
+
+	lock_sock(sk);
+	list_for_each_entry(assoc, &ep->asocs, asocs) {
+		if (cb->args[4] < cb->args[1])
+			goto next;
+
+		if (r->id.idiag_sport != htons(assoc->base.bind_addr.port) &&
+		    r->id.idiag_sport)
+			goto next;
+		if (r->id.idiag_dport != htons(assoc->peer.port) &&
+		    r->id.idiag_dport)
+			goto next;
+
+		if (!cb->args[3] &&
+		    inet_ep_diag_fill(sk, ep, skb, r,
+				      sk_user_ns(NETLINK_CB(cb->skb).sk),
+				      NETLINK_CB(cb->skb).portid,
+				      cb->nlh->nlmsg_seq,
+				      NLM_F_MULTI, cb->nlh) < 0) {
+			cb->args[3] = 1;
+			err = 2;
+			goto release;
+		}
+		cb->args[3] = 1;
+
+		if (inet_assoc_diag_fill(sk, assoc, skb, r,
+					 sk_user_ns(NETLINK_CB(cb->skb).sk),
+					 NETLINK_CB(cb->skb).portid,
+					 cb->nlh->nlmsg_seq, 0, cb->nlh) < 0) {
+			err = 2;
+			goto release;
+		}
+next:
+		cb->args[4]++;
+	}
+	cb->args[1] = 0;
+	cb->args[2]++;
+	cb->args[3] = 0;
+	cb->args[4] = 0;
+release:
+	release_sock(sk);
+	return err;
+out:
+	cb->args[2]++;
+	return err;
+}
+
+static int sctp_ep_dump(struct sctp_endpoint *ep, void *p)
+{
+	struct sctp_comm_param *commp = p;
+	struct sock *sk = ep->base.sk;
+	struct sk_buff *skb = commp->skb;
+	struct netlink_callback *cb = commp->cb;
+	const struct inet_diag_req_v2 *r = commp->r;
+	struct net *net = sock_net(skb->sk);
+	struct inet_sock *inet = inet_sk(sk);
+	int err = 0;
+
+	if (!net_eq(sock_net(sk), net))
+		goto out;
+
+	if (cb->args[4] < cb->args[1])
+		goto next;
+
+	if (r->sdiag_family != AF_UNSPEC &&
+	    sk->sk_family != r->sdiag_family)
+		goto next;
+
+	if (r->id.idiag_sport != inet->inet_sport &&
+	    r->id.idiag_sport)
+		goto next;
+
+	if (r->id.idiag_dport != inet->inet_dport &&
+	    r->id.idiag_dport)
+		goto next;
+
+	if (inet_ep_diag_fill(sk, ep, skb, r,
+			      sk_user_ns(NETLINK_CB(cb->skb).sk),
+			      NETLINK_CB(cb->skb).portid,
+			      cb->nlh->nlmsg_seq, NLM_F_MULTI,
+			      cb->nlh) < 0) {
+		err = 2;
+		goto out;
+	}
+next:
+	cb->args[4]++;
+out:
+	return err;
+}
+
+/* define the functions for sctp_diag_handler*/
+static void sctp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
+			       void *info)
+{
+	struct sctp_infox *infox = (struct sctp_infox *)info;
+
+	if (infox->asoc) {
+		r->idiag_rqueue = atomic_read(&infox->asoc->rmem_alloc);
+		r->idiag_wqueue = infox->asoc->sndbuf_used;
+	} else {
+		r->idiag_rqueue = sk->sk_ack_backlog;
+		r->idiag_wqueue = sk->sk_max_ack_backlog;
+	}
+	if (infox->sctpinfo)
+		sctp_get_sctp_info(sk, infox->asoc, infox->sctpinfo);
+}
+
+static int sctp_diag_dump_one(struct sk_buff *in_skb,
+			      const struct nlmsghdr *nlh,
+			      const struct inet_diag_req_v2 *req)
+{
+	struct net *net = sock_net(in_skb->sk);
+	union sctp_addr laddr, paddr;
+	struct sctp_comm_param commp = {
+		.skb = in_skb,
+		.r = req,
+		.nlh = nlh,
+	};
+
+	if (req->sdiag_family == AF_INET) {
+		laddr.v4.sin_port = req->id.idiag_sport;
+		laddr.v4.sin_addr.s_addr = req->id.idiag_src[0];
+		laddr.v4.sin_family = AF_INET;
+
+		paddr.v4.sin_port = req->id.idiag_dport;
+		paddr.v4.sin_addr.s_addr = req->id.idiag_dst[0];
+		paddr.v4.sin_family = AF_INET;
+	} else {
+		laddr.v6.sin6_port = req->id.idiag_sport;
+		memcpy(&laddr.v6.sin6_addr, req->id.idiag_src, 64);
+		laddr.v6.sin6_family = AF_INET6;
+
+		paddr.v6.sin6_port = req->id.idiag_dport;
+		memcpy(&paddr.v6.sin6_addr, req->id.idiag_dst, 64);
+		paddr.v6.sin6_family = AF_INET6;
+	}
+
+	return sctp_transport_lookup_process(sctp_tsp_dump_one,
+					     net, &laddr, &paddr, &commp);
+}
+
+static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
+			   const struct inet_diag_req_v2 *r, struct nlattr *bc)
+{
+	u32 idiag_states = r->idiag_states;
+	struct net *net = sock_net(skb->sk);
+	struct sctp_comm_param commp = {
+		.skb = skb,
+		.cb = cb,
+		.r = r,
+	};
+
+	/* eps hashtable dumps
+	 * args:
+	 * 0 : if it will traversal listen sock
+	 * 1 : to record the sock pos of this time's traversal
+	 * 4 : to work as a temporary variable to traversal list
+	 */
+	if (cb->args[0] == 0) {
+		if (!(idiag_states & TCPF_LISTEN))
+			goto skip;
+		if (sctp_for_each_endpoint(sctp_ep_dump, &commp))
+			goto done;
+skip:
+		cb->args[0] = 1;
+		cb->args[1] = 0;
+		cb->args[4] = 0;
+	}
+
+	/* asocs by transport hashtable dump
+	 * args:
+	 * 1 : to record the assoc pos of this time's traversal
+	 * 2 : to record the transport pos of this time's traversal
+	 * 3 : to mark if we have dumped the ep info of the current asoc
+	 * 4 : to work as a temporary variable to traversal list
+	 */
+	if (!(idiag_states & ~TCPF_LISTEN))
+		goto done;
+	sctp_for_each_transport(sctp_tsp_dump, net, cb->args[2], &commp);
+done:
+	cb->args[1] = cb->args[4];
+	cb->args[4] = 0;
+}
+
+static const struct inet_diag_handler sctp_diag_handler = {
+	.dump		 = sctp_diag_dump,
+	.dump_one	 = sctp_diag_dump_one,
+	.idiag_get_info  = sctp_diag_get_info,
+	.idiag_type	 = IPPROTO_SCTP,
+	.idiag_info_size = sizeof(struct sctp_info),
+};
+
+static int __init sctp_diag_init(void)
+{
+	return inet_diag_register(&sctp_diag_handler);
+}
+
+static void __exit sctp_diag_exit(void)
+{
+	inet_diag_unregister(&sctp_diag_handler);
+}
+
+module_init(sctp_diag_init);
+module_exit(sctp_diag_exit);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_SOCK_DIAG, 2-132);
-- 
2.1.0

[PATCHv2 net-next 5/6] sctp: merge the seq_start/next/exits in remaddrs and assocs

[Xin Long]

In sctp proc, these three functions in remaddrs and assocs are the
same. we should merge them into one.

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

diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index dd8492f..9fe1393 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -282,7 +282,7 @@ struct sctp_ht_iter {
 	struct rhashtable_iter hti;
 };
 
-static void *sctp_assocs_seq_start(struct seq_file *seq, loff_t *pos)
+static void *sctp_transport_seq_start(struct seq_file *seq, loff_t *pos)
 {
 	struct sctp_ht_iter *iter = seq->private;
 	int err = sctp_transport_walk_start(&iter->hti);
@@ -293,14 +293,14 @@ static void *sctp_assocs_seq_start(struct seq_file *seq, loff_t *pos)
 	return sctp_transport_get_idx(seq_file_net(seq), &iter->hti, *pos);
 }
 
-static void sctp_assocs_seq_stop(struct seq_file *seq, void *v)
+static void sctp_transport_seq_stop(struct seq_file *seq, void *v)
 {
 	struct sctp_ht_iter *iter = seq->private;
 
 	sctp_transport_walk_stop(&iter->hti);
 }
 
-static void *sctp_assocs_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+static void *sctp_transport_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
 	struct sctp_ht_iter *iter = seq->private;
 
@@ -367,9 +367,9 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v)
 }
 
 static const struct seq_operations sctp_assoc_ops = {
-	.start = sctp_assocs_seq_start,
-	.next  = sctp_assocs_seq_next,
-	.stop  = sctp_assocs_seq_stop,
+	.start = sctp_transport_seq_start,
+	.next  = sctp_transport_seq_next,
+	.stop  = sctp_transport_seq_stop,
 	.show  = sctp_assocs_seq_show,
 };
 
@@ -406,33 +406,6 @@ void sctp_assocs_proc_exit(struct net *net)
 	remove_proc_entry("assocs", net->sctp.proc_net_sctp);
 }
 
-static void *sctp_remaddr_seq_start(struct seq_file *seq, loff_t *pos)
-{
-	struct sctp_ht_iter *iter = seq->private;
-	int err = sctp_transport_walk_start(&iter->hti);
-
-	if (err)
-		return ERR_PTR(err);
-
-	return sctp_transport_get_idx(seq_file_net(seq), &iter->hti, *pos);
-}
-
-static void *sctp_remaddr_seq_next(struct seq_file *seq, void *v, loff_t *pos)
-{
-	struct sctp_ht_iter *iter = seq->private;
-
-	++*pos;
-
-	return sctp_transport_get_next(seq_file_net(seq), &iter->hti);
-}
-
-static void sctp_remaddr_seq_stop(struct seq_file *seq, void *v)
-{
-	struct sctp_ht_iter *iter = seq->private;
-
-	sctp_transport_walk_stop(&iter->hti);
-}
-
 static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
 {
 	struct sctp_association *assoc;
@@ -506,9 +479,9 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
 }
 
 static const struct seq_operations sctp_remaddr_ops = {
-	.start = sctp_remaddr_seq_start,
-	.next  = sctp_remaddr_seq_next,
-	.stop  = sctp_remaddr_seq_stop,
+	.start = sctp_transport_seq_start,
+	.next  = sctp_transport_seq_next,
+	.stop  = sctp_transport_seq_stop,
 	.show  = sctp_remaddr_seq_show,
 };
 
-- 
2.1.0

[PATCHv2 net-next 6/6] sctp: fix some rhashtable functions using in sctp proc/diag

[Xin Long]

When rhashtable_walk_init return err, no release function should be
called, and when rhashtable_walk_start return err, we should only invoke
rhashtable_walk_exit to release the source.

But now when sctp_transport_walk_start return err, we just call
rhashtable_walk_stop/exit, and never care about if rhashtable_walk_init
or start return err, which is so bad.

We will fix it by calling rhashtable_walk_exit if rhashtable_walk_start
return err in sctp_transport_walk_start, and if sctp_transport_walk_start
return err, we do not need to call sctp_transport_walk_stop any more.

For sctp proc, we will use 'iter->start_fail' to decide if we will call
rhashtable_walk_stop/exit.

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

diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index 9fe1393..4cb5aed 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -280,6 +280,7 @@ void sctp_eps_proc_exit(struct net *net)
 struct sctp_ht_iter {
 	struct seq_net_private p;
 	struct rhashtable_iter hti;
+	int start_fail;
 };
 
 static void *sctp_transport_seq_start(struct seq_file *seq, loff_t *pos)
@@ -287,8 +288,10 @@ static void *sctp_transport_seq_start(struct seq_file *seq, loff_t *pos)
 	struct sctp_ht_iter *iter = seq->private;
 	int err = sctp_transport_walk_start(&iter->hti);
 
-	if (err)
+	if (err) {
+		iter->start_fail = 1;
 		return ERR_PTR(err);
+	}
 
 	return sctp_transport_get_idx(seq_file_net(seq), &iter->hti, *pos);
 }
@@ -297,6 +300,8 @@ static void sctp_transport_seq_stop(struct seq_file *seq, void *v)
 {
 	struct sctp_ht_iter *iter = seq->private;
 
+	if (iter->start_fail)
+		return;
 	sctp_transport_walk_stop(&iter->hti);
 }
 
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b0bf6c7..473a40c 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4298,8 +4298,12 @@ int sctp_transport_walk_start(struct rhashtable_iter *iter)
 		return err;
 
 	err = rhashtable_walk_start(iter);
+	if (err && err != -EAGAIN) {
+		rhashtable_walk_exit(iter);
+		return err;
+	}
 
-	return err == -EAGAIN ? 0 : err;
+	return 0;
 }
 
 void sctp_transport_walk_stop(struct rhashtable_iter *iter)
@@ -4388,11 +4392,12 @@ EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);
 int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
 			    struct net *net, int pos, void *p) {
 	struct rhashtable_iter hti;
-	int err = 0;
 	void *obj;
+	int err;
 
-	if (sctp_transport_walk_start(&hti))
-		goto out;
+	err = sctp_transport_walk_start(&hti);
+	if (err)
+		return err;
 
 	sctp_transport_get_idx(net, &hti, pos);
 	obj = sctp_transport_get_next(net, &hti);
@@ -4406,8 +4411,8 @@ int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
 		if (err)
 			break;
 	}
-out:
 	sctp_transport_walk_stop(&hti);
+
 	return err;
 }
 EXPORT_SYMBOL_GPL(sctp_for_each_transport);
-- 
2.1.0

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Eric Dumazet]

On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
> sctp_diag will dump some important details of sctp's assoc or ep, we use
> sctp_info to describe them,  sctp_get_sctp_info to get them, and export
> it to sctp_diag.ko.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/linux/sctp.h    | 65 +++++++++++++++++++++++++++++++++++++
>  include/net/sctp/sctp.h |  3 ++
>  net/sctp/socket.c       | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 154 insertions(+)
> 
> diff --git a/include/linux/sctp.h b/include/linux/sctp.h
> index a9414fd..a448ebc 100644
> --- a/include/linux/sctp.h
> +++ b/include/linux/sctp.h
> @@ -705,4 +705,69 @@ typedef struct sctp_auth_chunk {
>  	sctp_authhdr_t auth_hdr;
>  } __packed sctp_auth_chunk_t;
>  
> +struct sctp_info {
> +	__u32	sctpi_tag;
> +	__u32	sctpi_state;
> +	__u32	sctpi_rwnd;
> +	__u16	sctpi_unackdata;
> +	__u16	sctpi_penddata;
> +	__u16	sctpi_instrms;
> +	__u16	sctpi_outstrms;
> +	__u32	sctpi_fragmentation_point;
> +	__u32	sctpi_inqueue;
> +	__u32	sctpi_outqueue;
> +	__u32	sctpi_overall_error;
> +	__u32	sctpi_max_burst;
> +	__u32	sctpi_maxseg;
> +	__u32	sctpi_peer_rwnd;
> +	__u32	sctpi_peer_tag;
> +	__u8	sctpi_peer_capable;
> +	__u8	sctpi_peer_sack;
> +
> +	/* assoc status info */
> +	__u64	sctpi_isacks;
> +	__u64	sctpi_osacks;
> +	__u64	sctpi_opackets;
> +	__u64	sctpi_ipackets;
> +	__u64	sctpi_rtxchunks;
> +	__u64	sctpi_outofseqtsns;
> +	__u64	sctpi_idupchunks;
> +	__u64	sctpi_gapcnt;
> +	__u64	sctpi_ouodchunks;
> +	__u64	sctpi_iuodchunks;
> +	__u64	sctpi_oodchunks;
> +	__u64	sctpi_iodchunks;
> +	__u64	sctpi_octrlchunks;
> +	__u64	sctpi_ictrlchunks;
> +
> +	/* primary transport info */
> +	struct sockaddr_storage	sctpi_p_address;
> +	__s32	sctpi_p_state;
> +	__u32	sctpi_p_cwnd;
> +	__u32	sctpi_p_srtt;
> +	__u32	sctpi_p_rto;
> +	__u32	sctpi_p_hbinterval;
> +	__u32	sctpi_p_pathmaxrxt;
> +	__u32	sctpi_p_sackdelay;
> +	__u32	sctpi_p_sackfreq;
> +	__u32	sctpi_p_ssthresh;
> +	__u32	sctpi_p_partial_bytes_acked;
> +	__u32	sctpi_p_flight_size;
> +	__u16	sctpi_p_error;
> +
> +	/* sctp sock info */
> +	__u32	sctpi_s_autoclose;
> +	__u32	sctpi_s_adaptation_ind;
> +	__u32	sctpi_s_pd_point;
> +	__u8	sctpi_s_nodelay;
> +	__u8	sctpi_s_disable_fragments;
> +	__u8	sctpi_s_v4mapped;
> +	__u8	sctpi_s_frag_interleave;
> +};
> +

Lots of holes in this structure...

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Eric Dumazet]

On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
> sctp_diag will dump some important details of sctp's assoc or ep, we use
> sctp_info to describe them,  sctp_get_sctp_info to get them, and export
> it to sctp_diag.ko.
> 


> +int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
> +		       struct sctp_info *info)
> +{
> +	struct sctp_transport *prim;
> +	struct list_head *pos, *temp;
> +	int mask;
> +
> +	memset(info, 0, sizeof(*info));
> +	if (!asoc) {
> +		struct sctp_sock *sp = sctp_sk(sk);
> +
> +		info->sctpi_s_autoclose = sp->autoclose;
> +		info->sctpi_s_adaptation_ind = sp->adaptation_ind;
> +		info->sctpi_s_pd_point = sp->pd_point;
> +		info->sctpi_s_nodelay = sp->nodelay;
> +		info->sctpi_s_disable_fragments = sp->disable_fragments;
> +		info->sctpi_s_v4mapped = sp->v4mapped;
> +		info->sctpi_s_frag_interleave = sp->frag_interleave;
> +
> +		return 0;
> +	}
> +
> +	info->sctpi_tag = asoc->c.my_vtag;
> +	info->sctpi_state = asoc->state;
> +	info->sctpi_rwnd = asoc->a_rwnd;
> +	info->sctpi_unackdata = asoc->unack_data;
> +	info->sctpi_penddata = sctp_tsnmap_pending(&asoc->peer.tsn_map);
> +	info->sctpi_instrms = asoc->c.sinit_max_instreams;
> +	info->sctpi_outstrms = asoc->c.sinit_num_ostreams;
> +	list_for_each_safe(pos, temp, &asoc->base.inqueue.in_chunk_list)
> +		info->sctpi_inqueue++;
> +	list_for_each_safe(pos, temp, &asoc->outqueue.out_chunk_list)
> +		info->sctpi_outqueue++;

Is this safe ?

Do you own the lock on socket or whatever lock protecting this list ?


> +	info->sctpi_overall_error = asoc->overall_error_count;
> +	info->sctpi_max_burst = asoc->max_burst;
> +	info->sctpi_maxseg = asoc->frag_point;
> +	info->sctpi_peer_rwnd = asoc->peer.rwnd;
> +	info->sctpi_peer_tag = asoc->c.peer_vtag;
> +
> +	mask = asoc->peer.ecn_capable << 1;
> +	mask = (mask | asoc->peer.ipv4_address) << 1;
> +	mask = (mask | asoc->peer.ipv6_address) << 1;
> +	mask = (mask | asoc->peer.hostname_address) << 1;
> +	mask = (mask | asoc->peer.asconf_capable) << 1;
> +	mask = (mask | asoc->peer.prsctp_capable) << 1;
> +	mask = (mask | asoc->peer.auth_capable);
> +	info->sctpi_peer_capable = mask;
> +	mask = asoc->peer.sack_needed << 1;
> +	mask = (mask | asoc->peer.sack_generation) << 1;
> +	mask = (mask | asoc->peer.zero_window_announced);
> +	info->sctpi_peer_sack = mask;
> +
> +	info->sctpi_isacks = asoc->stats.isacks;
> +	info->sctpi_osacks = asoc->stats.osacks;
> +	info->sctpi_opackets = asoc->stats.opackets;
> +	info->sctpi_ipackets = asoc->stats.ipackets;
> +	info->sctpi_rtxchunks = asoc->stats.rtxchunks;
> +	info->sctpi_outofseqtsns = asoc->stats.outofseqtsns;
> +	info->sctpi_idupchunks = asoc->stats.idupchunks;
> +	info->sctpi_gapcnt = asoc->stats.gapcnt;
> +	info->sctpi_ouodchunks = asoc->stats.ouodchunks;
> +	info->sctpi_iuodchunks = asoc->stats.iuodchunks;
> +	info->sctpi_oodchunks = asoc->stats.oodchunks;
> +	info->sctpi_iodchunks = asoc->stats.iodchunks;
> +	info->sctpi_octrlchunks = asoc->stats.octrlchunks;
> +	info->sctpi_ictrlchunks = asoc->stats.ictrlchunks;
> +
> +	prim = asoc->peer.primary_path;
> +	memcpy(&info->sctpi_p_address, &prim->ipaddr,
> +	       sizeof(struct sockaddr_storage));
> +	info->sctpi_p_state = prim->state;
> +	info->sctpi_p_cwnd = prim->cwnd;
> +	info->sctpi_p_srtt = prim->srtt;
> +	info->sctpi_p_rto = jiffies_to_msecs(prim->rto);
> +	info->sctpi_p_hbinterval = prim->hbinterval;
> +	info->sctpi_p_pathmaxrxt = prim->pathmaxrxt;
> +	info->sctpi_p_sackdelay = jiffies_to_msecs(prim->sackdelay);
> +	info->sctpi_p_ssthresh = prim->ssthresh;
> +	info->sctpi_p_partial_bytes_acked = prim->partial_bytes_acked;
> +	info->sctpi_p_flight_size = prim->flight_size;
> +	info->sctpi_p_error = prim->error_count;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(sctp_get_sctp_info);

info is not guaranteed to be aligned on 8 bytes.

You need to use put_unaligned()

Check commit ff5d749772018 ("tcp: beware of alignments in
tcp_get_info()") for details.

Re: [PATCHv2 net-next 4/6] sctp: add the sctp_diag.c file

[Eric Dumazet]

On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
> This one will implement all the interface of inet_diag, inet_diag_handler.
> which includes sctp_diag_dump, sctp_diag_dump_one and sctp_diag_get_info.


> +static int inet_assoc_diag_fill(struct sock *sk,
> +				struct sctp_association *asoc,
> +				struct sk_buff *skb,
> +				const struct inet_diag_req_v2 *req,
> +				struct user_namespace *user_ns,
> +				int portid, u32 seq, u16 nlmsg_flags,
> +				const struct nlmsghdr *unlh)
> +{
> +	const struct inet_sock *inet = inet_sk(sk);
> +	const struct inet_diag_handler *handler;
> +	int ext = req->idiag_ext;
> +	struct inet_diag_msg *r;
> +	struct nlmsghdr  *nlh;
> +	struct nlattr *attr;
> +	void *info = NULL;
> +	union sctp_addr laddr, paddr;
> +	struct dst_entry *dst;
> +	struct sctp_infox infox;
> +
> +	handler = inet_diag_get_handler(req->sdiag_protocol);
> +	BUG_ON(!handler);
> +
> +	nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
> +			nlmsg_flags);
> +	if (!nlh)
> +		return -EMSGSIZE;
> +
> +	r = nlmsg_data(nlh);
> +	BUG_ON(!sk_fullsock(sk));
> +
> +	laddr = list_entry(asoc->base.bind_addr.address_list.next,
> +			   struct sctp_sockaddr_entry, list)->a;
> +	paddr = asoc->peer.primary_path->ipaddr;
> +	dst = asoc->peer.primary_path->dst;
> +
> +	r->idiag_family = sk->sk_family;
> +	r->id.idiag_sport = htons(asoc->base.bind_addr.port);
> +	r->id.idiag_dport = htons(asoc->peer.port);
> +	r->id.idiag_if = dst ? dst->dev->ifindex : 0;
> +	sock_diag_save_cookie(sk, r->id.idiag_cookie);
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (sk->sk_family == AF_INET6) {
> +		*(struct in6_addr *)r->id.idiag_src = laddr.v6.sin6_addr;
> +		*(struct in6_addr *)r->id.idiag_dst = paddr.v6.sin6_addr;
> +	} else
> +#endif
> +	{
> +		memset(&r->id.idiag_src, 0, sizeof(r->id.idiag_src));
> +		memset(&r->id.idiag_dst, 0, sizeof(r->id.idiag_dst));
> +
> +		r->id.idiag_src[0] = laddr.v4.sin_addr.s_addr;
> +		r->id.idiag_dst[0] = paddr.v4.sin_addr.s_addr;
> +	}
> +
> +	r->idiag_state = asoc->state;
> +	r->idiag_timer = SCTP_EVENT_TIMEOUT_T3_RTX;
> +	r->idiag_retrans = asoc->rtx_data_chunks;
> +#define EXPIRES_IN_MS(tmo)  DIV_ROUND_UP((tmo - jiffies) * 1000, HZ)
> +	r->idiag_expires =
> +		EXPIRES_IN_MS(asoc->timeouts[SCTP_EVENT_TIMEOUT_T3_RTX]);
> +#undef EXPIRES_IN_MS
> +
> +	if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
> +		goto errout;
> +
> +	/* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
> +	 * hence this needs to be included regardless of socket family.
> +	 */
> +	if (ext & (1 << (INET_DIAG_TOS - 1)))
> +		if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
> +			goto errout;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (r->idiag_family == AF_INET6) {
> +		if (ext & (1 << (INET_DIAG_TCLASS - 1)))
> +			if (nla_put_u8(skb, INET_DIAG_TCLASS,
> +				       inet6_sk(sk)->tclass) < 0)
> +				goto errout;
> +
> +		if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
> +		    nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
> +			goto errout;
> +	}
> +#endif
> +
> +	r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
> +	r->idiag_inode = sock_i_ino(sk);
> +
> +	if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
> +		struct inet_diag_meminfo minfo = {
> +			.idiag_rmem = sk_rmem_alloc_get(sk),
> +			.idiag_wmem = sk->sk_wmem_queued,
> +			.idiag_fmem = sk->sk_forward_alloc,
> +			.idiag_tmem = sk_wmem_alloc_get(sk),
> +		};
> +

All this code looks familiar.

Why inet_sk_diag_fill() is not used instead ?

> +		if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
> +			goto errout;
> +	}
> +
> +	if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
> +		if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
> +			goto errout;
> +
> +	if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
> +		attr = nla_reserve(skb, INET_DIAG_INFO,
> +				   handler->idiag_info_size);
> +		if (!attr)
> +			goto errout;
> +
> +		info = nla_data(attr);
> +	}
> +	infox.sctpinfo = (struct sctp_info *)info;
> +	infox.asoc = asoc;
> +	handler->idiag_get_info(sk, r, &infox);
> +
> +	if (ext & (1 << (INET_DIAG_CONG - 1)))
> +		if (nla_put_string(skb, INET_DIAG_CONG, "reno") < 0)
> +			goto errout;
> +
> +	if (inet_sctp_fill_laddrs(skb, &asoc->base.bind_addr.address_list))
> +		goto errout;
> +
> +	if (inet_sctp_fill_paddrs(skb, asoc))
> +		goto errout;
> +
> +	nlmsg_end(skb, nlh);
> +	return 0;
> +
> +errout:
> +	nlmsg_cancel(skb, nlh);
> +	return -EMSGSIZE;
> +}
> +
> +static int inet_ep_diag_fill(struct sock *sk, struct sctp_endpoint *ep,
> +			     struct sk_buff *skb,
> +			     const struct inet_diag_req_v2 *req,
> +			     struct user_namespace *user_ns,
> +			     u32 portid, u32 seq, u16 nlmsg_flags,
> +			     const struct nlmsghdr *unlh)
> +{
> +	const struct inet_sock *inet = inet_sk(sk);
> +	const struct inet_diag_handler *handler;
> +	int ext = req->idiag_ext;
> +	struct inet_diag_msg *r;
> +	struct nlmsghdr  *nlh;
> +	struct nlattr *attr;
> +	void *info = NULL;
> +	struct sctp_infox infox;
> +
> +	handler = inet_diag_get_handler(req->sdiag_protocol);
> +	BUG_ON(!handler);
> +
> +	nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
> +			nlmsg_flags);
> +	if (!nlh)
> +		return -EMSGSIZE;
> +
> +	r = nlmsg_data(nlh);
> +	BUG_ON(!sk_fullsock(sk));
> +
> +	inet_diag_msg_common_fill(r, sk);
> +	r->idiag_state = sk->sk_state;
> +	r->idiag_timer = 0;
> +	r->idiag_retrans = 0;
> +
> +	if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
> +		goto errout;
> +
> +	/* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
> +	 * hence this needs to be included regardless of socket family.
> +	 */
> +	if (ext & (1 << (INET_DIAG_TOS - 1)))
> +		if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
> +			goto errout;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (r->idiag_family == AF_INET6) {
> +		if (ext & (1 << (INET_DIAG_TCLASS - 1)))
> +			if (nla_put_u8(skb, INET_DIAG_TCLASS,
> +				       inet6_sk(sk)->tclass) < 0)
> +				goto errout;
> +
> +		if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
> +		    nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
> +			goto errout;
> +	}
> +#endif
> +
> +	r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
> +	r->idiag_inode = sock_i_ino(sk);
> +
> +	if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
> +		struct inet_diag_meminfo minfo = {
> +			.idiag_rmem = sk_rmem_alloc_get(sk),
> +			.idiag_wmem = sk->sk_wmem_queued,
> +			.idiag_fmem = sk->sk_forward_alloc,
> +			.idiag_tmem = sk_wmem_alloc_get(sk),
> +		};
> +

Again, looks a lot of duplication.

Also you missed that INET_DIAG_MEMINFO is kind of obsolete,
now we have sock_diag_put_meminfo()


> +		if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
> +			goto errout;
> +	}
> +
> +	if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
> +		if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
> +			goto errout;
> +
> +	if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
> +		attr = nla_reserve(skb, INET_DIAG_INFO,
> +				   handler->idiag_info_size);
> +		if (!attr)
> +			goto errout;
> +
> +		info = nla_data(attr);
> +	}
> +	infox.sctpinfo = (struct sctp_info *)info;
> +	infox.asoc = NULL;
> +	handler->idiag_get_info(sk, r, &infox);
> +
> +	if (inet_sctp_fill_laddrs(skb, &ep->base.bind_addr.address_list))
> +		goto errout;
> +
> +	nlmsg_end(skb, nlh);
> +	return 0;
> +
> +errout:
> +	nlmsg_cancel(skb, nlh);
> +	return -EMSGSIZE;
> +}
> +
> +static size_t inet_assoc_attr_size(struct sctp_association *asoc)
> +{
> +	int addrlen = sizeof(struct sockaddr_storage);
> +	int addrcnt = 0;
> +	struct sctp_sockaddr_entry *laddr;
> +
> +	list_for_each_entry_rcu(laddr, &asoc->base.bind_addr.address_list,
> +				list)
> +		addrcnt++;
> +
> +	return	  nla_total_size(sizeof(struct tcp_info))

Are you sure you want to use tcp_info ???

> +		+ nla_total_size(1) /* INET_DIAG_SHUTDOWN */
> +		+ nla_total_size(1) /* INET_DIAG_TOS */
> +		+ nla_total_size(1) /* INET_DIAG_TCLASS */
> +		+ nla_total_size(addrlen * asoc->peer.transport_count)
> +		+ nla_total_size(addrlen * addrcnt)
> +		+ nla_total_size(sizeof(struct inet_diag_meminfo))
> +		+ nla_total_size(sizeof(struct inet_diag_msg))
> +		+ nla_total_size(sizeof(struct sctp_info))
> +		+ 64;
> +}

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Jamal Hadi Salim]

Appreciate these patches. Finally some love for sctp.
Small comment below:

On 16-04-09 12:53 AM, Xin Long wrote:
> sctp_diag will dump some important details of sctp's assoc or ep, we use
> sctp_info to describe them,  sctp_get_sctp_info to get them, and export
> it to sctp_diag.ko.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>   include/linux/sctp.h    | 65 +++++++++++++++++++++++++++++++++++++
>   include/net/sctp/sctp.h |  3 ++
>   net/sctp/socket.c       | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 154 insertions(+)
>
> diff --git a/include/linux/sctp.h b/include/linux/sctp.h
> index a9414fd..a448ebc 100644
> --- a/include/linux/sctp.h
> +++ b/include/linux/sctp.h
> @@ -705,4 +705,69 @@ typedef struct sctp_auth_chunk {
>   	sctp_authhdr_t auth_hdr;
>   } __packed sctp_auth_chunk_t;
>
> +struct sctp_info {
> +	__u32	sctpi_tag;
> +	__u32	sctpi_state;
> +	__u32	sctpi_rwnd;
> +	__u16	sctpi_unackdata;
> +	__u16	sctpi_penddata;
> +	__u16	sctpi_instrms;
> +	__u16	sctpi_outstrms;
> +	__u32	sctpi_fragmentation_point;
> +	__u32	sctpi_inqueue;
> +	__u32	sctpi_outqueue;
> +	__u32	sctpi_overall_error;
> +	__u32	sctpi_max_burst;
> +	__u32	sctpi_maxseg;
> +	__u32	sctpi_peer_rwnd;
> +	__u32	sctpi_peer_tag;
> +	__u8	sctpi_peer_capable;
> +	__u8	sctpi_peer_sack;
> +
> +	/* assoc status info */
> +	__u64	sctpi_isacks;
> +	__u64	sctpi_osacks;
> +	__u64	sctpi_opackets;
> +	__u64	sctpi_ipackets;
> +	__u64	sctpi_rtxchunks;
> +	__u64	sctpi_outofseqtsns;
> +	__u64	sctpi_idupchunks;
> +	__u64	sctpi_gapcnt;
> +	__u64	sctpi_ouodchunks;
> +	__u64	sctpi_iuodchunks;
> +	__u64	sctpi_oodchunks;
> +	__u64	sctpi_iodchunks;
> +	__u64	sctpi_octrlchunks;
> +	__u64	sctpi_ictrlchunks;
> +
> +	/* primary transport info */
> +	struct sockaddr_storage	sctpi_p_address;
> +	__s32	sctpi_p_state;
> +	__u32	sctpi_p_cwnd;
> +	__u32	sctpi_p_srtt;
> +	__u32	sctpi_p_rto;
> +	__u32	sctpi_p_hbinterval;
> +	__u32	sctpi_p_pathmaxrxt;
> +	__u32	sctpi_p_sackdelay;
> +	__u32	sctpi_p_sackfreq;
> +	__u32	sctpi_p_ssthresh;
> +	__u32	sctpi_p_partial_bytes_acked;
> +	__u32	sctpi_p_flight_size;
> +	__u16	sctpi_p_error;
> +
> +	/* sctp sock info */
> +	__u32	sctpi_s_autoclose;
> +	__u32	sctpi_s_adaptation_ind;
> +	__u32	sctpi_s_pd_point;
> +	__u8	sctpi_s_nodelay;
> +	__u8	sctpi_s_disable_fragments;
> +	__u8	sctpi_s_v4mapped;
> +	__u8	sctpi_s_frag_interleave;
> +};
> +


Can you double check to make sure this is 32 bit aligned
(no holes) maybe in your case 64 bit aligned?
Sticking  +	__u16	sctpi_p_error in there seems
to kill it.

Also, any plans to do the netlink events and destroy features?

cheers,
jamal

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Jamal Hadi Salim]

On 16-04-09 01:16 AM, Eric Dumazet wrote:

>
> Lots of holes in this structure...
>
>


I may have mentioned to you that there is 8 bit hole in tcp_info too ;->
(above tcpi_rto). Adding an 8 bit explicit pad seems useful
since it is already in the wild. I was going to send the patch after
netdev11 but  forgot.

cheers,
jamal

Re: [PATCHv2 net-next 4/6] sctp: add the sctp_diag.c file

[Xin Long]

On Sat, Apr 9, 2016 at 1:51 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
>> This one will implement all the interface of inet_diag, inet_diag_handler.
>> which includes sctp_diag_dump, sctp_diag_dump_one and sctp_diag_get_info.
>
>
>> +static int inet_assoc_diag_fill(struct sock *sk,
>> +                             struct sctp_association *asoc,
>> +                             struct sk_buff *skb,
>> +                             const struct inet_diag_req_v2 *req,
>> +                             struct user_namespace *user_ns,
>> +                             int portid, u32 seq, u16 nlmsg_flags,
>> +                             const struct nlmsghdr *unlh)
>> +{
>> +     const struct inet_sock *inet = inet_sk(sk);
>> +     const struct inet_diag_handler *handler;
>> +     int ext = req->idiag_ext;
>> +     struct inet_diag_msg *r;
>> +     struct nlmsghdr  *nlh;
>> +     struct nlattr *attr;
>> +     void *info = NULL;
>> +     union sctp_addr laddr, paddr;
>> +     struct dst_entry *dst;
>> +     struct sctp_infox infox;
>> +
>> +     handler = inet_diag_get_handler(req->sdiag_protocol);
>> +     BUG_ON(!handler);
>> +
>> +     nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
>> +                     nlmsg_flags);
>> +     if (!nlh)
>> +             return -EMSGSIZE;
>> +
>> +     r = nlmsg_data(nlh);
>> +     BUG_ON(!sk_fullsock(sk));
>> +
>> +     laddr = list_entry(asoc->base.bind_addr.address_list.next,
>> +                        struct sctp_sockaddr_entry, list)->a;
>> +     paddr = asoc->peer.primary_path->ipaddr;
>> +     dst = asoc->peer.primary_path->dst;
>> +
>> +     r->idiag_family = sk->sk_family;
>> +     r->id.idiag_sport = htons(asoc->base.bind_addr.port);
>> +     r->id.idiag_dport = htons(asoc->peer.port);
>> +     r->id.idiag_if = dst ? dst->dev->ifindex : 0;
>> +     sock_diag_save_cookie(sk, r->id.idiag_cookie);
>> +
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +     if (sk->sk_family == AF_INET6) {
>> +             *(struct in6_addr *)r->id.idiag_src = laddr.v6.sin6_addr;
>> +             *(struct in6_addr *)r->id.idiag_dst = paddr.v6.sin6_addr;
>> +     } else
>> +#endif
>> +     {
>> +             memset(&r->id.idiag_src, 0, sizeof(r->id.idiag_src));
>> +             memset(&r->id.idiag_dst, 0, sizeof(r->id.idiag_dst));
>> +
>> +             r->id.idiag_src[0] = laddr.v4.sin_addr.s_addr;
>> +             r->id.idiag_dst[0] = paddr.v4.sin_addr.s_addr;
>> +     }
>> +
>> +     r->idiag_state = asoc->state;
>> +     r->idiag_timer = SCTP_EVENT_TIMEOUT_T3_RTX;
>> +     r->idiag_retrans = asoc->rtx_data_chunks;
>> +#define EXPIRES_IN_MS(tmo)  DIV_ROUND_UP((tmo - jiffies) * 1000, HZ)
>> +     r->idiag_expires =
>> +             EXPIRES_IN_MS(asoc->timeouts[SCTP_EVENT_TIMEOUT_T3_RTX]);
>> +#undef EXPIRES_IN_MS
>> +
>> +     if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
>> +             goto errout;
>> +
>> +     /* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
>> +      * hence this needs to be included regardless of socket family.
>> +      */
>> +     if (ext & (1 << (INET_DIAG_TOS - 1)))
>> +             if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
>> +                     goto errout;
>> +
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +     if (r->idiag_family == AF_INET6) {
>> +             if (ext & (1 << (INET_DIAG_TCLASS - 1)))
>> +                     if (nla_put_u8(skb, INET_DIAG_TCLASS,
>> +                                    inet6_sk(sk)->tclass) < 0)
>> +                             goto errout;
>> +
>> +             if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
>> +                 nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
>> +                     goto errout;
>> +     }
>> +#endif
>> +
>> +     r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
>> +     r->idiag_inode = sock_i_ino(sk);
>> +
>> +     if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
>> +             struct inet_diag_meminfo minfo = {
>> +                     .idiag_rmem = sk_rmem_alloc_get(sk),
>> +                     .idiag_wmem = sk->sk_wmem_queued,
>> +                     .idiag_fmem = sk->sk_forward_alloc,
>> +                     .idiag_tmem = sk_wmem_alloc_get(sk),
>> +             };
>> +
>
> All this code looks familiar.
>
> Why inet_sk_diag_fill() is not used instead ?
>
it's hard to reuse  inet_sk_diag_fill(), cause some of them are from
assoc.

yes, there are some duplicate codes. if we want to avoid this.
we have to extract new function for this part, and it will change
more in inet_diag.


>> +             if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
>> +                     goto errout;
>> +     }
>> +
>> +     if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
>> +             if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
>> +                     goto errout;
>> +
>> +     if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
>> +             attr = nla_reserve(skb, INET_DIAG_INFO,
>> +                                handler->idiag_info_size);
>> +             if (!attr)
>> +                     goto errout;
>> +
>> +             info = nla_data(attr);
>> +     }
>> +     infox.sctpinfo = (struct sctp_info *)info;
>> +     infox.asoc = asoc;
>> +     handler->idiag_get_info(sk, r, &infox);
>> +
>> +     if (ext & (1 << (INET_DIAG_CONG - 1)))
>> +             if (nla_put_string(skb, INET_DIAG_CONG, "reno") < 0)
>> +                     goto errout;
>> +
>> +     if (inet_sctp_fill_laddrs(skb, &asoc->base.bind_addr.address_list))
>> +             goto errout;
>> +
>> +     if (inet_sctp_fill_paddrs(skb, asoc))
>> +             goto errout;
>> +
>> +     nlmsg_end(skb, nlh);
>> +     return 0;
>> +
>> +errout:
>> +     nlmsg_cancel(skb, nlh);
>> +     return -EMSGSIZE;
>> +}
>> +
>> +static int inet_ep_diag_fill(struct sock *sk, struct sctp_endpoint *ep,
>> +                          struct sk_buff *skb,
>> +                          const struct inet_diag_req_v2 *req,
>> +                          struct user_namespace *user_ns,
>> +                          u32 portid, u32 seq, u16 nlmsg_flags,
>> +                          const struct nlmsghdr *unlh)
>> +{
>> +     const struct inet_sock *inet = inet_sk(sk);
>> +     const struct inet_diag_handler *handler;
>> +     int ext = req->idiag_ext;
>> +     struct inet_diag_msg *r;
>> +     struct nlmsghdr  *nlh;
>> +     struct nlattr *attr;
>> +     void *info = NULL;
>> +     struct sctp_infox infox;
>> +
>> +     handler = inet_diag_get_handler(req->sdiag_protocol);
>> +     BUG_ON(!handler);
>> +
>> +     nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
>> +                     nlmsg_flags);
>> +     if (!nlh)
>> +             return -EMSGSIZE;
>> +
>> +     r = nlmsg_data(nlh);
>> +     BUG_ON(!sk_fullsock(sk));
>> +
>> +     inet_diag_msg_common_fill(r, sk);
>> +     r->idiag_state = sk->sk_state;
>> +     r->idiag_timer = 0;
>> +     r->idiag_retrans = 0;
>> +
>> +     if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
>> +             goto errout;
>> +
>> +     /* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
>> +      * hence this needs to be included regardless of socket family.
>> +      */
>> +     if (ext & (1 << (INET_DIAG_TOS - 1)))
>> +             if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
>> +                     goto errout;
>> +
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +     if (r->idiag_family == AF_INET6) {
>> +             if (ext & (1 << (INET_DIAG_TCLASS - 1)))
>> +                     if (nla_put_u8(skb, INET_DIAG_TCLASS,
>> +                                    inet6_sk(sk)->tclass) < 0)
>> +                             goto errout;
>> +
>> +             if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
>> +                 nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
>> +                     goto errout;
>> +     }
>> +#endif
>> +
>> +     r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
>> +     r->idiag_inode = sock_i_ino(sk);
>> +
>> +     if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
>> +             struct inet_diag_meminfo minfo = {
>> +                     .idiag_rmem = sk_rmem_alloc_get(sk),
>> +                     .idiag_wmem = sk->sk_wmem_queued,
>> +                     .idiag_fmem = sk->sk_forward_alloc,
>> +                     .idiag_tmem = sk_wmem_alloc_get(sk),
>> +             };
>> +
>
> Again, looks a lot of duplication.
>
> Also you missed that INET_DIAG_MEMINFO is kind of obsolete,
> now we have sock_diag_put_meminfo()
>
you meant we can remove it here ?
yes, it seems similar with INET_DIAG_SKMEMINFO.
but I do not know if userpace may use  INET_DIAG_MEMINFO now.

>
>> +             if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
>> +                     goto errout;
>> +     }
>> +
>> +     if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
>> +             if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
>> +                     goto errout;
>> +
>> +     if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
>> +             attr = nla_reserve(skb, INET_DIAG_INFO,
>> +                                handler->idiag_info_size);
>> +             if (!attr)
>> +                     goto errout;
>> +
>> +             info = nla_data(attr);
>> +     }
>> +     infox.sctpinfo = (struct sctp_info *)info;
>> +     infox.asoc = NULL;
>> +     handler->idiag_get_info(sk, r, &infox);
>> +
>> +     if (inet_sctp_fill_laddrs(skb, &ep->base.bind_addr.address_list))
>> +             goto errout;
>> +
>> +     nlmsg_end(skb, nlh);
>> +     return 0;
>> +
>> +errout:
>> +     nlmsg_cancel(skb, nlh);
>> +     return -EMSGSIZE;
>> +}
>> +
>> +static size_t inet_assoc_attr_size(struct sctp_association *asoc)
>> +{
>> +     int addrlen = sizeof(struct sockaddr_storage);
>> +     int addrcnt = 0;
>> +     struct sctp_sockaddr_entry *laddr;
>> +
>> +     list_for_each_entry_rcu(laddr, &asoc->base.bind_addr.address_list,
>> +                             list)
>> +             addrcnt++;
>> +
>> +     return    nla_total_size(sizeof(struct tcp_info))
>
> Are you sure you want to use tcp_info ???
here is a mistake, it should be:
    nla_total_size(sizeof(struct sctp_info))

thanks.
>
>> +             + nla_total_size(1) /* INET_DIAG_SHUTDOWN */
>> +             + nla_total_size(1) /* INET_DIAG_TOS */
>> +             + nla_total_size(1) /* INET_DIAG_TCLASS */
>> +             + nla_total_size(addrlen * asoc->peer.transport_count)
>> +             + nla_total_size(addrlen * addrcnt)
>> +             + nla_total_size(sizeof(struct inet_diag_meminfo))
>> +             + nla_total_size(sizeof(struct inet_diag_msg))
>> +             + nla_total_size(sizeof(struct sctp_info))
and will remove this one.

>> +             + 64;
>> +}
>
>
>

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Xin Long]

On Sat, Apr 9, 2016 at 1:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
>> sctp_diag will dump some important details of sctp's assoc or ep, we use
>> sctp_info to describe them,  sctp_get_sctp_info to get them, and export
>> it to sctp_diag.ko.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>>  include/linux/sctp.h    | 65 +++++++++++++++++++++++++++++++++++++
>>  include/net/sctp/sctp.h |  3 ++
>>  net/sctp/socket.c       | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 154 insertions(+)
>>
>> diff --git a/include/linux/sctp.h b/include/linux/sctp.h
>> index a9414fd..a448ebc 100644
>> --- a/include/linux/sctp.h
>> +++ b/include/linux/sctp.h
>> @@ -705,4 +705,69 @@ typedef struct sctp_auth_chunk {
>>       sctp_authhdr_t auth_hdr;
>>  } __packed sctp_auth_chunk_t;
>>
>> +struct sctp_info {
>> +     __u32   sctpi_tag;
>> +     __u32   sctpi_state;
>> +     __u32   sctpi_rwnd;
>> +     __u16   sctpi_unackdata;
>> +     __u16   sctpi_penddata;
>> +     __u16   sctpi_instrms;
>> +     __u16   sctpi_outstrms;
>> +     __u32   sctpi_fragmentation_point;
>> +     __u32   sctpi_inqueue;
>> +     __u32   sctpi_outqueue;
>> +     __u32   sctpi_overall_error;
>> +     __u32   sctpi_max_burst;
>> +     __u32   sctpi_maxseg;
>> +     __u32   sctpi_peer_rwnd;
>> +     __u32   sctpi_peer_tag;
>> +     __u8    sctpi_peer_capable;
>> +     __u8    sctpi_peer_sack;
>> +
>> +     /* assoc status info */
>> +     __u64   sctpi_isacks;
>> +     __u64   sctpi_osacks;
>> +     __u64   sctpi_opackets;
>> +     __u64   sctpi_ipackets;
>> +     __u64   sctpi_rtxchunks;
>> +     __u64   sctpi_outofseqtsns;
>> +     __u64   sctpi_idupchunks;
>> +     __u64   sctpi_gapcnt;
>> +     __u64   sctpi_ouodchunks;
>> +     __u64   sctpi_iuodchunks;
>> +     __u64   sctpi_oodchunks;
>> +     __u64   sctpi_iodchunks;
>> +     __u64   sctpi_octrlchunks;
>> +     __u64   sctpi_ictrlchunks;
>> +
>> +     /* primary transport info */
>> +     struct sockaddr_storage sctpi_p_address;
>> +     __s32   sctpi_p_state;
>> +     __u32   sctpi_p_cwnd;
>> +     __u32   sctpi_p_srtt;
>> +     __u32   sctpi_p_rto;
>> +     __u32   sctpi_p_hbinterval;
>> +     __u32   sctpi_p_pathmaxrxt;
>> +     __u32   sctpi_p_sackdelay;
>> +     __u32   sctpi_p_sackfreq;
>> +     __u32   sctpi_p_ssthresh;
>> +     __u32   sctpi_p_partial_bytes_acked;
>> +     __u32   sctpi_p_flight_size;
>> +     __u16   sctpi_p_error;
>> +
>> +     /* sctp sock info */
>> +     __u32   sctpi_s_autoclose;
>> +     __u32   sctpi_s_adaptation_ind;
>> +     __u32   sctpi_s_pd_point;
>> +     __u8    sctpi_s_nodelay;
>> +     __u8    sctpi_s_disable_fragments;
>> +     __u8    sctpi_s_v4mapped;
>> +     __u8    sctpi_s_frag_interleave;
>> +};
>> +
>
> Lots of holes in this structure...
>
>
will check and improve it later.
thanks.

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Xin Long]

On Sat, Apr 9, 2016 at 1:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
>> sctp_diag will dump some important details of sctp's assoc or ep, we use
>> sctp_info to describe them,  sctp_get_sctp_info to get them, and export
>> it to sctp_diag.ko.
>>
>
>
>> +int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
>> +                    struct sctp_info *info)
>> +{
>> +     struct sctp_transport *prim;
>> +     struct list_head *pos, *temp;
>> +     int mask;
>> +
>> +     memset(info, 0, sizeof(*info));
>> +     if (!asoc) {
>> +             struct sctp_sock *sp = sctp_sk(sk);
>> +
>> +             info->sctpi_s_autoclose = sp->autoclose;
>> +             info->sctpi_s_adaptation_ind = sp->adaptation_ind;
>> +             info->sctpi_s_pd_point = sp->pd_point;
>> +             info->sctpi_s_nodelay = sp->nodelay;
>> +             info->sctpi_s_disable_fragments = sp->disable_fragments;
>> +             info->sctpi_s_v4mapped = sp->v4mapped;
>> +             info->sctpi_s_frag_interleave = sp->frag_interleave;
>> +
>> +             return 0;
>> +     }
>> +
>> +     info->sctpi_tag = asoc->c.my_vtag;
>> +     info->sctpi_state = asoc->state;
>> +     info->sctpi_rwnd = asoc->a_rwnd;
>> +     info->sctpi_unackdata = asoc->unack_data;
>> +     info->sctpi_penddata = sctp_tsnmap_pending(&asoc->peer.tsn_map);
>> +     info->sctpi_instrms = asoc->c.sinit_max_instreams;
>> +     info->sctpi_outstrms = asoc->c.sinit_num_ostreams;
>> +     list_for_each_safe(pos, temp, &asoc->base.inqueue.in_chunk_list)
>> +             info->sctpi_inqueue++;
>> +     list_for_each_safe(pos, temp, &asoc->outqueue.out_chunk_list)
>> +             info->sctpi_outqueue++;
>
> Is this safe ?
>
> Do you own the lock on socket or whatever lock protecting this list ?
>
there are 2 places will call these codes,
1. sctp_diag_dump -> sctp_for_each_transport -> sctp_tsp_dump
this one will use lock_sock to protect them. I think this one is ok.

1. sctp_diag_dump_one -> sctp_transport_lookup_process-> sctp_tsp_dump_one
this one just holds the tsp. and we're using  list_for_each_safe here now,
isn't it enough ?


>
>> +     info->sctpi_overall_error = asoc->overall_error_count;
>> +     info->sctpi_max_burst = asoc->max_burst;
>> +     info->sctpi_maxseg = asoc->frag_point;
>> +     info->sctpi_peer_rwnd = asoc->peer.rwnd;
>> +     info->sctpi_peer_tag = asoc->c.peer_vtag;
>> +
>> +     mask = asoc->peer.ecn_capable << 1;
>> +     mask = (mask | asoc->peer.ipv4_address) << 1;
>> +     mask = (mask | asoc->peer.ipv6_address) << 1;
>> +     mask = (mask | asoc->peer.hostname_address) << 1;
>> +     mask = (mask | asoc->peer.asconf_capable) << 1;
>> +     mask = (mask | asoc->peer.prsctp_capable) << 1;
>> +     mask = (mask | asoc->peer.auth_capable);
>> +     info->sctpi_peer_capable = mask;
>> +     mask = asoc->peer.sack_needed << 1;
>> +     mask = (mask | asoc->peer.sack_generation) << 1;
>> +     mask = (mask | asoc->peer.zero_window_announced);
>> +     info->sctpi_peer_sack = mask;
>> +
>> +     info->sctpi_isacks = asoc->stats.isacks;
>> +     info->sctpi_osacks = asoc->stats.osacks;
>> +     info->sctpi_opackets = asoc->stats.opackets;
>> +     info->sctpi_ipackets = asoc->stats.ipackets;
>> +     info->sctpi_rtxchunks = asoc->stats.rtxchunks;
>> +     info->sctpi_outofseqtsns = asoc->stats.outofseqtsns;
>> +     info->sctpi_idupchunks = asoc->stats.idupchunks;
>> +     info->sctpi_gapcnt = asoc->stats.gapcnt;
>> +     info->sctpi_ouodchunks = asoc->stats.ouodchunks;
>> +     info->sctpi_iuodchunks = asoc->stats.iuodchunks;
>> +     info->sctpi_oodchunks = asoc->stats.oodchunks;
>> +     info->sctpi_iodchunks = asoc->stats.iodchunks;
>> +     info->sctpi_octrlchunks = asoc->stats.octrlchunks;
>> +     info->sctpi_ictrlchunks = asoc->stats.ictrlchunks;
>> +
>> +     prim = asoc->peer.primary_path;
>> +     memcpy(&info->sctpi_p_address, &prim->ipaddr,
>> +            sizeof(struct sockaddr_storage));
>> +     info->sctpi_p_state = prim->state;
>> +     info->sctpi_p_cwnd = prim->cwnd;
>> +     info->sctpi_p_srtt = prim->srtt;
>> +     info->sctpi_p_rto = jiffies_to_msecs(prim->rto);
>> +     info->sctpi_p_hbinterval = prim->hbinterval;
>> +     info->sctpi_p_pathmaxrxt = prim->pathmaxrxt;
>> +     info->sctpi_p_sackdelay = jiffies_to_msecs(prim->sackdelay);
>> +     info->sctpi_p_ssthresh = prim->ssthresh;
>> +     info->sctpi_p_partial_bytes_acked = prim->partial_bytes_acked;
>> +     info->sctpi_p_flight_size = prim->flight_size;
>> +     info->sctpi_p_error = prim->error_count;
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(sctp_get_sctp_info);
>
> info is not guaranteed to be aligned on 8 bytes.
>
> You need to use put_unaligned()
>
> Check commit ff5d749772018 ("tcp: beware of alignments in
> tcp_get_info()") for details.
Ok,  I will.

>
>
>

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Eric Dumazet]

On Sat, 2016-04-09 at 11:19 -0400, Jamal Hadi Salim wrote:
> On 16-04-09 01:16 AM, Eric Dumazet wrote:
> 
> >
> > Lots of holes in this structure...
> >
> >
> 
> 
> I may have mentioned to you that there is 8 bit hole in tcp_info too ;->
> (above tcpi_rto). Adding an 8 bit explicit pad seems useful
> since it is already in the wild. I was going to send the patch after
> netdev11 but  forgot.

Well, once a hole is there, nothing we can do really, because of
compatibility with old kernels / old binaries.


But when a _new_ structure is defined, this is the time where we can ask
for doing sensible things ;)

Re: [PATCHv2 net-next 4/6] sctp: add the sctp_diag.c file

[Eric Dumazet]

On Sat, 2016-04-09 at 23:40 +0800, Xin Long wrote:

> you meant we can remove it here ?
> yes, it seems similar with INET_DIAG_SKMEMINFO.
> but I do not know if userpace may use  INET_DIAG_MEMINFO now.
> 

You are adding new features here. No problem of legacy code.

Anyway, as I said you really need to reuse code, not copy paste please.

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Eric Dumazet]

On Sun, 2016-04-10 at 00:10 +0800, Xin Long wrote:
> On Sat, Apr 9, 2016 at 1:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
> >> sctp_diag will dump some important details of sctp's assoc or ep, we use
> >> sctp_info to describe them,  sctp_get_sctp_info to get them, and export
> >> it to sctp_diag.ko.
> >>
> >
> >
> >> +int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
> >> +                    struct sctp_info *info)
> >> +{
> >> +     struct sctp_transport *prim;
> >> +     struct list_head *pos, *temp;
> >> +     int mask;
> >> +
> >> +     memset(info, 0, sizeof(*info));
> >> +     if (!asoc) {
> >> +             struct sctp_sock *sp = sctp_sk(sk);
> >> +
> >> +             info->sctpi_s_autoclose = sp->autoclose;
> >> +             info->sctpi_s_adaptation_ind = sp->adaptation_ind;
> >> +             info->sctpi_s_pd_point = sp->pd_point;
> >> +             info->sctpi_s_nodelay = sp->nodelay;
> >> +             info->sctpi_s_disable_fragments = sp->disable_fragments;
> >> +             info->sctpi_s_v4mapped = sp->v4mapped;
> >> +             info->sctpi_s_frag_interleave = sp->frag_interleave;
> >> +
> >> +             return 0;
> >> +     }
> >> +
> >> +     info->sctpi_tag = asoc->c.my_vtag;
> >> +     info->sctpi_state = asoc->state;
> >> +     info->sctpi_rwnd = asoc->a_rwnd;
> >> +     info->sctpi_unackdata = asoc->unack_data;
> >> +     info->sctpi_penddata = sctp_tsnmap_pending(&asoc->peer.tsn_map);
> >> +     info->sctpi_instrms = asoc->c.sinit_max_instreams;
> >> +     info->sctpi_outstrms = asoc->c.sinit_num_ostreams;
> >> +     list_for_each_safe(pos, temp, &asoc->base.inqueue.in_chunk_list)
> >> +             info->sctpi_inqueue++;
> >> +     list_for_each_safe(pos, temp, &asoc->outqueue.out_chunk_list)
> >> +             info->sctpi_outqueue++;
> >
> > Is this safe ?
> >
> > Do you own the lock on socket or whatever lock protecting this list ?
> >
> there are 2 places will call these codes,
> 1. sctp_diag_dump -> sctp_for_each_transport -> sctp_tsp_dump
> this one will use lock_sock to protect them. I think this one is ok.
> 
> 1. sctp_diag_dump_one -> sctp_transport_lookup_process-> sctp_tsp_dump_one
> this one just holds the tsp. and we're using  list_for_each_safe here now,
> isn't it enough ?
> 

You tell me ;)

For sure in tcp_get_info() the socket is sometimes locked, and sometimes
is not locked, depending on the caller.

So we had to use only lockless accesses.

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Eric Dumazet]

On Sun, 2016-04-10 at 00:10 +0800, Xin Long wrote:

> 1. sctp_diag_dump_one -> sctp_transport_lookup_process-> sctp_tsp_dump_one
> this one just holds the tsp. and we're using  list_for_each_safe here now,
> isn't it enough ?

list_for_each_safe is 'safe' if you do a list_del() yourself.

It is not safe if other cpus are adding/deleting items in the list while
this thread is iterating it.

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Xin Long]

On Sat, Apr 9, 2016 at 11:16 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> Appreciate these patches. Finally some love for sctp.
> Small comment below:
>
>
> On 16-04-09 12:53 AM, Xin Long wrote:
>>
>> sctp_diag will dump some important details of sctp's assoc or ep, we use
>> sctp_info to describe them,  sctp_get_sctp_info to get them, and export
>> it to sctp_diag.ko.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>>   include/linux/sctp.h    | 65 +++++++++++++++++++++++++++++++++++++
>>   include/net/sctp/sctp.h |  3 ++
>>   net/sctp/socket.c       | 86
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 154 insertions(+)
>>
>> diff --git a/include/linux/sctp.h b/include/linux/sctp.h
>> index a9414fd..a448ebc 100644
>> --- a/include/linux/sctp.h
>> +++ b/include/linux/sctp.h
>> @@ -705,4 +705,69 @@ typedef struct sctp_auth_chunk {
>>         sctp_authhdr_t auth_hdr;
>>   } __packed sctp_auth_chunk_t;
>>
>> +struct sctp_info {
>> +       __u32   sctpi_tag;
>> +       __u32   sctpi_state;
>> +       __u32   sctpi_rwnd;
>> +       __u16   sctpi_unackdata;
>> +       __u16   sctpi_penddata;
>> +       __u16   sctpi_instrms;
>> +       __u16   sctpi_outstrms;
>> +       __u32   sctpi_fragmentation_point;
>> +       __u32   sctpi_inqueue;
>> +       __u32   sctpi_outqueue;
>> +       __u32   sctpi_overall_error;
>> +       __u32   sctpi_max_burst;
>> +       __u32   sctpi_maxseg;
>> +       __u32   sctpi_peer_rwnd;
>> +       __u32   sctpi_peer_tag;
>> +       __u8    sctpi_peer_capable;
>> +       __u8    sctpi_peer_sack;
>> +
>> +       /* assoc status info */
>> +       __u64   sctpi_isacks;
>> +       __u64   sctpi_osacks;
>> +       __u64   sctpi_opackets;
>> +       __u64   sctpi_ipackets;
>> +       __u64   sctpi_rtxchunks;
>> +       __u64   sctpi_outofseqtsns;
>> +       __u64   sctpi_idupchunks;
>> +       __u64   sctpi_gapcnt;
>> +       __u64   sctpi_ouodchunks;
>> +       __u64   sctpi_iuodchunks;
>> +       __u64   sctpi_oodchunks;
>> +       __u64   sctpi_iodchunks;
>> +       __u64   sctpi_octrlchunks;
>> +       __u64   sctpi_ictrlchunks;
>> +
>> +       /* primary transport info */
>> +       struct sockaddr_storage sctpi_p_address;
>> +       __s32   sctpi_p_state;
>> +       __u32   sctpi_p_cwnd;
>> +       __u32   sctpi_p_srtt;
>> +       __u32   sctpi_p_rto;
>> +       __u32   sctpi_p_hbinterval;
>> +       __u32   sctpi_p_pathmaxrxt;
>> +       __u32   sctpi_p_sackdelay;
>> +       __u32   sctpi_p_sackfreq;
>> +       __u32   sctpi_p_ssthresh;
>> +       __u32   sctpi_p_partial_bytes_acked;
>> +       __u32   sctpi_p_flight_size;
>> +       __u16   sctpi_p_error;
>> +
>> +       /* sctp sock info */
>> +       __u32   sctpi_s_autoclose;
>> +       __u32   sctpi_s_adaptation_ind;
>> +       __u32   sctpi_s_pd_point;
>> +       __u8    sctpi_s_nodelay;
>> +       __u8    sctpi_s_disable_fragments;
>> +       __u8    sctpi_s_v4mapped;
>> +       __u8    sctpi_s_frag_interleave;
>> +};
>> +
>
>
>
> Can you double check to make sure this is 32 bit aligned
> (no holes) maybe in your case 64 bit aligned?
> Sticking  +     __u16   sctpi_p_error in there seems
> to kill it.
yes, I will check this structure all.

>
> Also, any plans to do the netlink events and destroy features?
yeah, maybe after this one, we  will do it.
but for destroy feature, do you have any idea to test, cause
ss of iproute seems cannot cover it, even .dump_one, neither.
do we have to write code to test them?


netlink events? sorry, what's that for? I didn't see it in tcp_diag.



>
> cheers,
> jamal

Re: [PATCHv2 net-next 1/6] sctp: add sctp_info dump api for sctp_diag

[Jamal Hadi Salim]

On 16-04-09 01:21 PM, Eric Dumazet wrote:


> Well, once a hole is there, nothing we can do really, because of
> compatibility with old kernels / old binaries.
>
>
> But when a _new_ structure is defined, this is the time where we can ask
> for doing sensible things ;)
>

This one is fixable. sizeof() already includes the accounting of
the pad. something like:

diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index fe95446..52542eb 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -158,6 +158,7 @@ struct tcp_info {
         __u8    tcpi_options;
         __u8    tcpi_snd_wscale : 4, tcpi_rcv_wscale : 4;

+       __u8    pad;    /*reuse this space if you need 8bits for something*/
         __u32   tcpi_rto;
         __u32   tcpi_ato;
         __u32   tcpi_snd_mss;

cheers,
jamal

Re: [PATCHv2 net-next 4/6] sctp: add the sctp_diag.c file

[Marcelo Ricardo Leitner]

On Fri, Apr 08, 2016 at 10:51:56PM -0700, Eric Dumazet wrote:
> On Sat, 2016-04-09 at 12:53 +0800, Xin Long wrote:
> > This one will implement all the interface of inet_diag, inet_diag_handler.
> > which includes sctp_diag_dump, sctp_diag_dump_one and sctp_diag_get_info.
> 
> 
> > +static int inet_assoc_diag_fill(struct sock *sk,
> > +				struct sctp_association *asoc,
> > +				struct sk_buff *skb,
> > +				const struct inet_diag_req_v2 *req,
> > +				struct user_namespace *user_ns,
> > +				int portid, u32 seq, u16 nlmsg_flags,
> > +				const struct nlmsghdr *unlh)
> > +{
> > +	const struct inet_sock *inet = inet_sk(sk);
> > +	const struct inet_diag_handler *handler;
> > +	int ext = req->idiag_ext;
> > +	struct inet_diag_msg *r;
> > +	struct nlmsghdr  *nlh;
> > +	struct nlattr *attr;
> > +	void *info = NULL;
> > +	union sctp_addr laddr, paddr;
> > +	struct dst_entry *dst;
> > +	struct sctp_infox infox;
> > +
> > +	handler = inet_diag_get_handler(req->sdiag_protocol);
> > +	BUG_ON(!handler);
> > +
> > +	nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
> > +			nlmsg_flags);
> > +	if (!nlh)
> > +		return -EMSGSIZE;
> > +
> > +	r = nlmsg_data(nlh);
> > +	BUG_ON(!sk_fullsock(sk));
> > +
> > +	laddr = list_entry(asoc->base.bind_addr.address_list.next,
> > +			   struct sctp_sockaddr_entry, list)->a;
> > +	paddr = asoc->peer.primary_path->ipaddr;
> > +	dst = asoc->peer.primary_path->dst;
> > +
> > +	r->idiag_family = sk->sk_family;
> > +	r->id.idiag_sport = htons(asoc->base.bind_addr.port);
> > +	r->id.idiag_dport = htons(asoc->peer.port);
> > +	r->id.idiag_if = dst ? dst->dev->ifindex : 0;
> > +	sock_diag_save_cookie(sk, r->id.idiag_cookie);
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +	if (sk->sk_family == AF_INET6) {
> > +		*(struct in6_addr *)r->id.idiag_src = laddr.v6.sin6_addr;
> > +		*(struct in6_addr *)r->id.idiag_dst = paddr.v6.sin6_addr;
> > +	} else
> > +#endif
> > +	{
> > +		memset(&r->id.idiag_src, 0, sizeof(r->id.idiag_src));
> > +		memset(&r->id.idiag_dst, 0, sizeof(r->id.idiag_dst));
> > +
> > +		r->id.idiag_src[0] = laddr.v4.sin_addr.s_addr;
> > +		r->id.idiag_dst[0] = paddr.v4.sin_addr.s_addr;
> > +	}
> > +
> > +	r->idiag_state = asoc->state;
> > +	r->idiag_timer = SCTP_EVENT_TIMEOUT_T3_RTX;
> > +	r->idiag_retrans = asoc->rtx_data_chunks;
> > +#define EXPIRES_IN_MS(tmo)  DIV_ROUND_UP((tmo - jiffies) * 1000, HZ)
> > +	r->idiag_expires =
> > +		EXPIRES_IN_MS(asoc->timeouts[SCTP_EVENT_TIMEOUT_T3_RTX]);
> > +#undef EXPIRES_IN_MS
> > +

vvv
> > +	if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
> > +		goto errout;
> > +
> > +	/* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
> > +	 * hence this needs to be included regardless of socket family.
> > +	 */
> > +	if (ext & (1 << (INET_DIAG_TOS - 1)))
> > +		if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
> > +			goto errout;
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +	if (r->idiag_family == AF_INET6) {
> > +		if (ext & (1 << (INET_DIAG_TCLASS - 1)))
> > +			if (nla_put_u8(skb, INET_DIAG_TCLASS,
> > +				       inet6_sk(sk)->tclass) < 0)
> > +				goto errout;
> > +
> > +		if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
> > +		    nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
> > +			goto errout;
> > +	}
> > +#endif
> > +
> > +	r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
> > +	r->idiag_inode = sock_i_ino(sk);
> > +
> > +	if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
> > +		struct inet_diag_meminfo minfo = {
> > +			.idiag_rmem = sk_rmem_alloc_get(sk),
> > +			.idiag_wmem = sk->sk_wmem_queued,
> > +			.idiag_fmem = sk->sk_forward_alloc,
> > +			.idiag_tmem = sk_wmem_alloc_get(sk),
> > +		};
> > +
> 
> All this code looks familiar.
> 
> Why inet_sk_diag_fill() is not used instead ?

Actually we have an issue here. idiag_tmem is using sk_wmem_alloc_get()   
directly, but it shouldn't. SCTP supports using sndbuf per socket or      
per assoc on that socket. We need an if and report the correct value,     
as it's done in sctp_wspace().                                            

For inet_ep_diag_fill, it's reporting an endpoint, so this is not needed
as we don't have this option in there.                                    

> > +		if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
> > +			goto errout;
> > +	}
> > +
> > +	if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
> > +		if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
> > +			goto errout;

Same happens in sock_diag_put_meminfo().

I highlighted the section that would have been common to
inet_sk_diag_fill() if it wasn't for this wmem_alloc difference using
vvv and ^^^ marks. There is one or another common lines out of it. I
suggest we try to factor out this common code but only from within these
two functions in sctp_diag, leaving inet_diag alone for now, just until
this soak a bit more.

> > +
> > +	if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
> > +		attr = nla_reserve(skb, INET_DIAG_INFO,
> > +				   handler->idiag_info_size);
> > +		if (!attr)
> > +			goto errout;
> > +
> > +		info = nla_data(attr);
> > +	}
^^^

> > +	infox.sctpinfo = (struct sctp_info *)info;
> > +	infox.asoc = asoc;
> > +	handler->idiag_get_info(sk, r, &infox);
> > +
> > +	if (ext & (1 << (INET_DIAG_CONG - 1)))
> > +		if (nla_put_string(skb, INET_DIAG_CONG, "reno") < 0)
> > +			goto errout;

the above block is not common to inet_sk_diag_fill

> > +
> > +	if (inet_sctp_fill_laddrs(skb, &asoc->base.bind_addr.address_list))
> > +		goto errout;
> > +
> > +	if (inet_sctp_fill_paddrs(skb, asoc))
> > +		goto errout;
> > +
> > +	nlmsg_end(skb, nlh);
> > +	return 0;
> > +
> > +errout:
> > +	nlmsg_cancel(skb, nlh);
> > +	return -EMSGSIZE;
> > +}
> > +
> > +static int inet_ep_diag_fill(struct sock *sk, struct sctp_endpoint *ep,
> > +			     struct sk_buff *skb,
> > +			     const struct inet_diag_req_v2 *req,
> > +			     struct user_namespace *user_ns,
> > +			     u32 portid, u32 seq, u16 nlmsg_flags,
> > +			     const struct nlmsghdr *unlh)
> > +{
> > +	const struct inet_sock *inet = inet_sk(sk);
> > +	const struct inet_diag_handler *handler;
> > +	int ext = req->idiag_ext;
> > +	struct inet_diag_msg *r;
> > +	struct nlmsghdr  *nlh;
> > +	struct nlattr *attr;
> > +	void *info = NULL;
> > +	struct sctp_infox infox;
> > +
> > +	handler = inet_diag_get_handler(req->sdiag_protocol);
> > +	BUG_ON(!handler);
> > +
> > +	nlh = nlmsg_put(skb, portid, seq, unlh->nlmsg_type, sizeof(*r),
> > +			nlmsg_flags);
> > +	if (!nlh)
> > +		return -EMSGSIZE;
> > +
> > +	r = nlmsg_data(nlh);
> > +	BUG_ON(!sk_fullsock(sk));
> > +
> > +	inet_diag_msg_common_fill(r, sk);
> > +	r->idiag_state = sk->sk_state;
> > +	r->idiag_timer = 0;
> > +	r->idiag_retrans = 0;
> > +
> > +	if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
> > +		goto errout;
> > +
> > +	/* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
> > +	 * hence this needs to be included regardless of socket family.
> > +	 */
> > +	if (ext & (1 << (INET_DIAG_TOS - 1)))
> > +		if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
> > +			goto errout;
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +	if (r->idiag_family == AF_INET6) {
> > +		if (ext & (1 << (INET_DIAG_TCLASS - 1)))
> > +			if (nla_put_u8(skb, INET_DIAG_TCLASS,
> > +				       inet6_sk(sk)->tclass) < 0)
> > +				goto errout;
> > +
> > +		if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
> > +		    nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
> > +			goto errout;
> > +	}
> > +#endif
> > +
> > +	r->idiag_uid = from_kuid_munged(user_ns, sock_i_uid(sk));
> > +	r->idiag_inode = sock_i_ino(sk);
> > +
> > +	if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
> > +		struct inet_diag_meminfo minfo = {
> > +			.idiag_rmem = sk_rmem_alloc_get(sk),
> > +			.idiag_wmem = sk->sk_wmem_queued,
> > +			.idiag_fmem = sk->sk_forward_alloc,
> > +			.idiag_tmem = sk_wmem_alloc_get(sk),
> > +		};
> > +
> 
> Again, looks a lot of duplication.
> 
> Also you missed that INET_DIAG_MEMINFO is kind of obsolete,
> now we have sock_diag_put_meminfo()
> 
> 
> > +		if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
> > +			goto errout;
> > +	}
> > +
> > +	if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
> > +		if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
> > +			goto errout;
> > +
> > +	if ((ext & (1 << (INET_DIAG_INFO - 1))) && handler->idiag_info_size) {
> > +		attr = nla_reserve(skb, INET_DIAG_INFO,
> > +				   handler->idiag_info_size);
> > +		if (!attr)
> > +			goto errout;
> > +
> > +		info = nla_data(attr);
> > +	}
> > +	infox.sctpinfo = (struct sctp_info *)info;
> > +	infox.asoc = NULL;
> > +	handler->idiag_get_info(sk, r, &infox);
> > +
> > +	if (inet_sctp_fill_laddrs(skb, &ep->base.bind_addr.address_list))
> > +		goto errout;
> > +
> > +	nlmsg_end(skb, nlh);
> > +	return 0;
> > +
> > +errout:
> > +	nlmsg_cancel(skb, nlh);
> > +	return -EMSGSIZE;
> > +}
> > +
> > +static size_t inet_assoc_attr_size(struct sctp_association *asoc)
> > +{
> > +	int addrlen = sizeof(struct sockaddr_storage);
> > +	int addrcnt = 0;
> > +	struct sctp_sockaddr_entry *laddr;
> > +
> > +	list_for_each_entry_rcu(laddr, &asoc->base.bind_addr.address_list,
> > +				list)
> > +		addrcnt++;
> > +
> > +	return	  nla_total_size(sizeof(struct tcp_info))
> 
> Are you sure you want to use tcp_info ???
> 
> > +		+ nla_total_size(1) /* INET_DIAG_SHUTDOWN */
> > +		+ nla_total_size(1) /* INET_DIAG_TOS */
> > +		+ nla_total_size(1) /* INET_DIAG_TCLASS */
> > +		+ nla_total_size(addrlen * asoc->peer.transport_count)
> > +		+ nla_total_size(addrlen * addrcnt)
> > +		+ nla_total_size(sizeof(struct inet_diag_meminfo))
> > +		+ nla_total_size(sizeof(struct inet_diag_msg))
> > +		+ nla_total_size(sizeof(struct sctp_info))
> > +		+ 64;
> > +}
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>