On Sun, 2016-04-10 at 00:10 +0800, Xin Long wrote:
> On Sat, Apr 9, 2016 at 1:19 PM, Eric Dumazet <eric.duma...@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.
