* [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received @ 2016-06-03 14:42 Xin Long 2016-06-03 17:49 ` Marcelo Ricardo Leitner 2016-06-06 3:12 ` David Miller 0 siblings, 2 replies; 10+ messages in thread From: Xin Long @ 2016-06-03 14:42 UTC (permalink / raw) To: network dev, linux-sctp Cc: Marcelo Ricardo Leitner, Vlad Yasevich, daniel, davem, eric.dumazet Now sctp doesn't change socket state upon shutdown reception. It changes just the assoc state, even though it's a TCP-style socket. For some cases, if we really need to check sk->sk_state, it's necessary to fix this issue, at least when we use ss or netstat to dump, we can get a more exact information. As an improvement, we will change sk->sk_state when we change asoc->state to SHUTDOWN_RECEIVED, and also do it in sctp_shutdown to keep consistent with sctp_close. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/sctp/sm_sideeffect.c | 4 +++- net/sctp/socket.c | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c index aa37122..12d4519 100644 --- a/net/sctp/sm_sideeffect.c +++ b/net/sctp/sm_sideeffect.c @@ -806,8 +806,10 @@ static void sctp_cmd_new_state(sctp_cmd_seq_t *cmds, /* Set the RCV_SHUTDOWN flag when a SHUTDOWN is received. */ if (sctp_state(asoc, SHUTDOWN_RECEIVED) && - sctp_sstate(sk, ESTABLISHED)) + sctp_sstate(sk, ESTABLISHED)) { + sk->sk_state = SCTP_SS_CLOSING; sk->sk_shutdown |= RCV_SHUTDOWN; + } } if (sctp_state(asoc, COOKIE_WAIT)) { diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 67154b8..91a460d 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -4193,6 +4193,7 @@ static void sctp_shutdown(struct sock *sk, int how) return; if (how & SEND_SHUTDOWN) { + sk->sk_state = SCTP_SS_CLOSING; ep = sctp_sk(sk)->ep; if (!list_empty(&ep->asocs)) { asoc = list_entry(ep->asocs.next, -- 2.1.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-03 14:42 [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received Xin Long @ 2016-06-03 17:49 ` Marcelo Ricardo Leitner 2016-06-04 9:45 ` Xin Long 2016-06-06 3:12 ` David Miller 1 sibling, 1 reply; 10+ messages in thread From: Marcelo Ricardo Leitner @ 2016-06-03 17:49 UTC (permalink / raw) To: Xin Long Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, eric.dumazet On Fri, Jun 03, 2016 at 10:42:45PM +0800, Xin Long wrote: > Now sctp doesn't change socket state upon shutdown reception. It changes > just the assoc state, even though it's a TCP-style socket. > > For some cases, if we really need to check sk->sk_state, it's necessary to > fix this issue, at least when we use ss or netstat to dump, we can get a > more exact information. > > As an improvement, we will change sk->sk_state when we change asoc->state > to SHUTDOWN_RECEIVED, and also do it in sctp_shutdown to keep consistent with > sctp_close. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/sctp/sm_sideeffect.c | 4 +++- > net/sctp/socket.c | 1 + > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c > index aa37122..12d4519 100644 > --- a/net/sctp/sm_sideeffect.c > +++ b/net/sctp/sm_sideeffect.c > @@ -806,8 +806,10 @@ static void sctp_cmd_new_state(sctp_cmd_seq_t *cmds, > > /* Set the RCV_SHUTDOWN flag when a SHUTDOWN is received. */ > if (sctp_state(asoc, SHUTDOWN_RECEIVED) && > - sctp_sstate(sk, ESTABLISHED)) > + sctp_sstate(sk, ESTABLISHED)) { > + sk->sk_state = SCTP_SS_CLOSING; > sk->sk_shutdown |= RCV_SHUTDOWN; > + } > } > > if (sctp_state(asoc, COOKIE_WAIT)) { > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 67154b8..91a460d 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -4193,6 +4193,7 @@ static void sctp_shutdown(struct sock *sk, int how) > return; > > if (how & SEND_SHUTDOWN) { > + sk->sk_state = SCTP_SS_CLOSING; > ep = sctp_sk(sk)->ep; > if (!list_empty(&ep->asocs)) { > asoc = list_entry(ep->asocs.next, Oh but I think the removed chunk was important, it just needed fixing. A comment in there said: /* If the association on the newsk is already closed before * accept() is called, set RCV_SHUTDOWN flag. */ Meaning that there is a way that it will create a socket in the state you're trying to avoid here, which now is not handled. Marcelo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-03 17:49 ` Marcelo Ricardo Leitner @ 2016-06-04 9:45 ` Xin Long 2016-06-04 12:22 ` Marcelo Ricardo Leitner 0 siblings, 1 reply; 10+ messages in thread From: Xin Long @ 2016-06-04 9:45 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, Eric Dumazet On Sat, Jun 4, 2016 at 1:49 AM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Fri, Jun 03, 2016 at 10:42:45PM +0800, Xin Long wrote: >> Now sctp doesn't change socket state upon shutdown reception. It changes >> just the assoc state, even though it's a TCP-style socket. >> >> For some cases, if we really need to check sk->sk_state, it's necessary to >> fix this issue, at least when we use ss or netstat to dump, we can get a >> more exact information. >> >> As an improvement, we will change sk->sk_state when we change asoc->state >> to SHUTDOWN_RECEIVED, and also do it in sctp_shutdown to keep consistent with >> sctp_close. >> >> Signed-off-by: Xin Long <lucien.xin@gmail.com> >> --- >> net/sctp/sm_sideeffect.c | 4 +++- >> net/sctp/socket.c | 1 + >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c >> index aa37122..12d4519 100644 >> --- a/net/sctp/sm_sideeffect.c >> +++ b/net/sctp/sm_sideeffect.c >> @@ -806,8 +806,10 @@ static void sctp_cmd_new_state(sctp_cmd_seq_t *cmds, >> >> /* Set the RCV_SHUTDOWN flag when a SHUTDOWN is received. */ >> if (sctp_state(asoc, SHUTDOWN_RECEIVED) && >> - sctp_sstate(sk, ESTABLISHED)) >> + sctp_sstate(sk, ESTABLISHED)) { >> + sk->sk_state = SCTP_SS_CLOSING; >> sk->sk_shutdown |= RCV_SHUTDOWN; >> + } >> } >> >> if (sctp_state(asoc, COOKIE_WAIT)) { >> diff --git a/net/sctp/socket.c b/net/sctp/socket.c >> index 67154b8..91a460d 100644 >> --- a/net/sctp/socket.c >> +++ b/net/sctp/socket.c >> @@ -4193,6 +4193,7 @@ static void sctp_shutdown(struct sock *sk, int how) >> return; >> >> if (how & SEND_SHUTDOWN) { >> + sk->sk_state = SCTP_SS_CLOSING; >> ep = sctp_sk(sk)->ep; >> if (!list_empty(&ep->asocs)) { >> asoc = list_entry(ep->asocs.next, > > Oh but I think the removed chunk was important, it just needed fixing. > > A comment in there said: > /* If the association on the newsk is already closed before > * accept() is called, set RCV_SHUTDOWN flag. > */ > Meaning that there is a way that it will create a socket in the state > you're trying to avoid here, which now is not handled. agreed, but it's another problem there, we'd better free newsk and assoc after sctp_sock_migrate, and let sctp_accept return err. I will think more about it and see if I cook another patch for it. > > Marcelo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-04 9:45 ` Xin Long @ 2016-06-04 12:22 ` Marcelo Ricardo Leitner 2016-06-07 11:03 ` Xin Long 0 siblings, 1 reply; 10+ messages in thread From: Marcelo Ricardo Leitner @ 2016-06-04 12:22 UTC (permalink / raw) To: Xin Long Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, Eric Dumazet Em 04-06-2016 06:45, Xin Long escreveu: > On Sat, Jun 4, 2016 at 1:49 AM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: >> On Fri, Jun 03, 2016 at 10:42:45PM +0800, Xin Long wrote: >>> Now sctp doesn't change socket state upon shutdown reception. It changes >>> just the assoc state, even though it's a TCP-style socket. >>> >>> For some cases, if we really need to check sk->sk_state, it's necessary to >>> fix this issue, at least when we use ss or netstat to dump, we can get a >>> more exact information. >>> >>> As an improvement, we will change sk->sk_state when we change asoc->state >>> to SHUTDOWN_RECEIVED, and also do it in sctp_shutdown to keep consistent with >>> sctp_close. >>> >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> >>> --- >>> net/sctp/sm_sideeffect.c | 4 +++- >>> net/sctp/socket.c | 1 + >>> 2 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c >>> index aa37122..12d4519 100644 >>> --- a/net/sctp/sm_sideeffect.c >>> +++ b/net/sctp/sm_sideeffect.c >>> @@ -806,8 +806,10 @@ static void sctp_cmd_new_state(sctp_cmd_seq_t *cmds, >>> >>> /* Set the RCV_SHUTDOWN flag when a SHUTDOWN is received. */ >>> if (sctp_state(asoc, SHUTDOWN_RECEIVED) && >>> - sctp_sstate(sk, ESTABLISHED)) >>> + sctp_sstate(sk, ESTABLISHED)) { >>> + sk->sk_state = SCTP_SS_CLOSING; >>> sk->sk_shutdown |= RCV_SHUTDOWN; >>> + } >>> } >>> >>> if (sctp_state(asoc, COOKIE_WAIT)) { >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c >>> index 67154b8..91a460d 100644 >>> --- a/net/sctp/socket.c >>> +++ b/net/sctp/socket.c >>> @@ -4193,6 +4193,7 @@ static void sctp_shutdown(struct sock *sk, int how) >>> return; >>> >>> if (how & SEND_SHUTDOWN) { >>> + sk->sk_state = SCTP_SS_CLOSING; >>> ep = sctp_sk(sk)->ep; >>> if (!list_empty(&ep->asocs)) { >>> asoc = list_entry(ep->asocs.next, >> >> Oh but I think the removed chunk was important, it just needed fixing. >> >> A comment in there said: >> /* If the association on the newsk is already closed before >> * accept() is called, set RCV_SHUTDOWN flag. >> */ >> Meaning that there is a way that it will create a socket in the state >> you're trying to avoid here, which now is not handled. > agreed, but it's another problem > there, we'd better free newsk and assoc after sctp_sock_migrate, > and let sctp_accept return err. I will think more about it and see if > I cook another patch for it. Return error? Please don't. Adam Endrodi asked in May (linux-sctp@) a way to return the addresses used on such attempts and currently this address returned by accept() is the only one we can get. Marcelo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-04 12:22 ` Marcelo Ricardo Leitner @ 2016-06-07 11:03 ` Xin Long 2016-06-07 12:08 ` Marcelo Ricardo Leitner 2016-06-08 10:42 ` Xin Long 0 siblings, 2 replies; 10+ messages in thread From: Xin Long @ 2016-06-07 11:03 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, Eric Dumazet On Sat, Jun 4, 2016 at 8:22 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > > Return error? Please don't. Adam Endrodi asked in May (linux-sctp@) a way to > return the addresses used on such attempts and currently this address > returned by accept() is the only one we can get. [1] I've checked Adam's email, what he asked was different case, in his case, the assoc closed *after* sctp_accept, that's why he can catch the SCTP_COMM_UP event on accept_sk but asoc is freed already. [2] if assoc close *before* sctp_accept return, I mean asoc close during sctp_accept schedule out. listen sk will be the assoc's parent sk, in sctp_cmd_delete_tcb (SCTP_CMD_DELETE_TCB): if (sctp_style(sk, TCP) && sctp_sstate(sk, LISTENING) && (!asoc->temp) && (sk->sk_shutdown != SHUTDOWN_MASK)) return; assoc can't be freed. and the event skb will be appended to listen_sk->sk_receive_queue when sctp_accept schedule back, it will transfer to accept_sk->sk_receive_queue. we can still get the closed assoc information. until we call sctp_close: if (sctp_state(asoc, CLOSED)) { sctp_association_free(asoc); continue; } and your suggestion to improve for his case [1] is to not schedule SCTP_CMD_DELETE_TCB (only for tcp style). right ? if so, in the case [2] that assoc close *before* sctp_accept return, we should also update the newsk->sk_state, like: --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -7565,10 +7565,12 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, /* If the association on the newsk is already closed before accept() * is called, set RCV_SHUTDOWN flag. */ - if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) + if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) { + newsk->sk_state = SCTP_SS_CLOSING; newsk->sk_shutdown |= RCV_SHUTDOWN; + } else + newsk->sk_state = SCTP_SS_ESTABLISHED; - newsk->sk_state = SCTP_SS_ESTABLISHED; so that this two cases have the similar process, and wait for sctp_close to clean assoc. what do you think ? > > Marcelo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-07 11:03 ` Xin Long @ 2016-06-07 12:08 ` Marcelo Ricardo Leitner 2016-06-08 10:17 ` Xin Long 2016-06-08 10:42 ` Xin Long 1 sibling, 1 reply; 10+ messages in thread From: Marcelo Ricardo Leitner @ 2016-06-07 12:08 UTC (permalink / raw) To: Xin Long Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, Eric Dumazet On Tue, Jun 07, 2016 at 07:03:55PM +0800, Xin Long wrote: > On Sat, Jun 4, 2016 at 8:22 PM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: > > > > Return error? Please don't. Adam Endrodi asked in May (linux-sctp@) a way to > > return the addresses used on such attempts and currently this address > > returned by accept() is the only one we can get. > [1] I've checked Adam's email, what he asked was different case, in > his case, the assoc > closed *after* sctp_accept, that's why he can catch the SCTP_COMM_UP event on > accept_sk but asoc is freed already. It doesn't have to be after sctp_accept because we will queue SCTP_COMM_UP as reactions to COOKIE_ECHO and COOKIE_ACK, and then such event will be moved to the new socket upon sctp_accept() call (which calls sctp_sock_migrate()). > > [2] if assoc close *before* sctp_accept return, I mean asoc close > during sctp_accept schedule out. > listen sk will be the assoc's parent sk, in sctp_cmd_delete_tcb > (SCTP_CMD_DELETE_TCB): > > if (sctp_style(sk, TCP) && sctp_sstate(sk, LISTENING) && > (!asoc->temp) && (sk->sk_shutdown != SHUTDOWN_MASK)) > return; > > assoc can't be freed. and the event skb will be appended to > listen_sk->sk_receive_queue > when sctp_accept schedule back, it will transfer to > accept_sk->sk_receive_queue. we can > still get the closed assoc information. until we call sctp_close: > if (sctp_state(asoc, CLOSED)) { > sctp_association_free(asoc); > continue; > } > > and your suggestion to improve for his case [1] is to not schedule > SCTP_CMD_DELETE_TCB > (only for tcp style). right ? Yes > > if so, in the case [2] that assoc close *before* sctp_accept return, > we should also update > the newsk->sk_state, like: > > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -7565,10 +7565,12 @@ static void sctp_sock_migrate(struct sock > *oldsk, struct sock *newsk, > /* If the association on the newsk is already closed before accept() > * is called, set RCV_SHUTDOWN flag. > */ > - if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) > + if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) { > + newsk->sk_state = SCTP_SS_CLOSING; > newsk->sk_shutdown |= RCV_SHUTDOWN; > + } else > + newsk->sk_state = SCTP_SS_ESTABLISHED; > > - newsk->sk_state = SCTP_SS_ESTABLISHED; > > so that this two cases have the similar process, and wait for > sctp_close to clean assoc. > what do you think ? I don't agree with before/after thing, but I agree with the solution. :) Marcelo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-07 12:08 ` Marcelo Ricardo Leitner @ 2016-06-08 10:17 ` Xin Long 0 siblings, 0 replies; 10+ messages in thread From: Xin Long @ 2016-06-08 10:17 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, Eric Dumazet On Tue, Jun 7, 2016 at 8:08 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Tue, Jun 07, 2016 at 07:03:55PM +0800, Xin Long wrote: >> On Sat, Jun 4, 2016 at 8:22 PM, Marcelo Ricardo Leitner >> <marcelo.leitner@gmail.com> wrote: >> > >> > Return error? Please don't. Adam Endrodi asked in May (linux-sctp@) a way to >> > return the addresses used on such attempts and currently this address >> > returned by accept() is the only one we can get. >> [1] I've checked Adam's email, what he asked was different case, in >> his case, the assoc >> closed *after* sctp_accept, that's why he can catch the SCTP_COMM_UP event on >> accept_sk but asoc is freed already. > > It doesn't have to be after sctp_accept because we will queue > SCTP_COMM_UP as reactions to COOKIE_ECHO and COOKIE_ACK, and then such > event will be moved to the new socket upon sctp_accept() call (which > calls sctp_sock_migrate()). I meant in his case, assoc close must be *after* sctp_accept. and he said he couldn't get it as asoc was gone. cause if assoc close is *before* sctp_accept return, assoc won't be freed [2], he can still get the addrs from asoc in SCTP_COMM_UP event > >> >> [2] if assoc close *before* sctp_accept return, I mean asoc close >> during sctp_accept schedule out. >> listen sk will be the assoc's parent sk, in sctp_cmd_delete_tcb >> (SCTP_CMD_DELETE_TCB): >> >> if (sctp_style(sk, TCP) && sctp_sstate(sk, LISTENING) && >> (!asoc->temp) && (sk->sk_shutdown != SHUTDOWN_MASK)) >> return; >> >> assoc can't be freed. and the event skb will be appended to >> listen_sk->sk_receive_queue >> when sctp_accept schedule back, it will transfer to >> accept_sk->sk_receive_queue. we can >> still get the closed assoc information. until we call sctp_close: >> if (sctp_state(asoc, CLOSED)) { >> sctp_association_free(asoc); >> continue; >> } >> >> and your suggestion to improve for his case [1] is to not schedule >> SCTP_CMD_DELETE_TCB >> (only for tcp style). right ? > > Yes > >> >> if so, in the case [2] that assoc close *before* sctp_accept return, >> we should also update >> the newsk->sk_state, like: >> >> --- a/net/sctp/socket.c >> +++ b/net/sctp/socket.c >> @@ -7565,10 +7565,12 @@ static void sctp_sock_migrate(struct sock >> *oldsk, struct sock *newsk, >> /* If the association on the newsk is already closed before accept() >> * is called, set RCV_SHUTDOWN flag. >> */ >> - if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) >> + if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) { >> + newsk->sk_state = SCTP_SS_CLOSING; >> newsk->sk_shutdown |= RCV_SHUTDOWN; >> + } else >> + newsk->sk_state = SCTP_SS_ESTABLISHED; >> >> - newsk->sk_state = SCTP_SS_ESTABLISHED; >> >> so that this two cases have the similar process, and wait for >> sctp_close to clean assoc. >> what do you think ? > > I don't agree with before/after thing, but I agree with the solution. :) ok, I will post v3. :D > > Marcelo > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-07 11:03 ` Xin Long 2016-06-07 12:08 ` Marcelo Ricardo Leitner @ 2016-06-08 10:42 ` Xin Long 2016-06-08 12:05 ` Marcelo Ricardo Leitner 1 sibling, 1 reply; 10+ messages in thread From: Xin Long @ 2016-06-08 10:42 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, Eric Dumazet On Tue, Jun 7, 2016 at 7:03 PM, Xin Long <lucien.xin@gmail.com> wrote: > On Sat, Jun 4, 2016 at 8:22 PM, Marcelo Ricardo Leitner > > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -7565,10 +7565,12 @@ static void sctp_sock_migrate(struct sock > *oldsk, struct sock *newsk, > /* If the association on the newsk is already closed before accept() > * is called, set RCV_SHUTDOWN flag. > */ > - if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) > + if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) { > + newsk->sk_state = SCTP_SS_CLOSING; > newsk->sk_shutdown |= RCV_SHUTDOWN; > + } else > + newsk->sk_state = SCTP_SS_ESTABLISHED; > > - newsk->sk_state = SCTP_SS_ESTABLISHED; > I'm still thinking about this, if we want to get addrs info from closed assoc, like Adam's requirement. this will make it more impossible. case in sctp_getsockopt_peer_addrs()->sctp_id2assoc(): if (!sctp_sstate(sk, ESTABLISHED)) return NULL; we can't get assoc as sstate is SS_CLOSING already. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-08 10:42 ` Xin Long @ 2016-06-08 12:05 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 10+ messages in thread From: Marcelo Ricardo Leitner @ 2016-06-08 12:05 UTC (permalink / raw) To: Xin Long Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem, Eric Dumazet Em 08-06-2016 07:42, Xin Long escreveu: > On Tue, Jun 7, 2016 at 7:03 PM, Xin Long <lucien.xin@gmail.com> wrote: >> On Sat, Jun 4, 2016 at 8:22 PM, Marcelo Ricardo Leitner >> >> --- a/net/sctp/socket.c >> +++ b/net/sctp/socket.c >> @@ -7565,10 +7565,12 @@ static void sctp_sock_migrate(struct sock >> *oldsk, struct sock *newsk, >> /* If the association on the newsk is already closed before accept() >> * is called, set RCV_SHUTDOWN flag. >> */ >> - if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) >> + if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) { >> + newsk->sk_state = SCTP_SS_CLOSING; >> newsk->sk_shutdown |= RCV_SHUTDOWN; >> + } else >> + newsk->sk_state = SCTP_SS_ESTABLISHED; >> >> - newsk->sk_state = SCTP_SS_ESTABLISHED; >> > I'm still thinking about this, if we want to get addrs info from > closed assoc, like Adam's requirement. this will make it more > impossible. > > case in > sctp_getsockopt_peer_addrs()->sctp_id2assoc(): > > if (!sctp_sstate(sk, ESTABLISHED)) > return NULL; > > we can't get assoc as sstate is SS_CLOSING already. > We probably can add another state to that if(). So far ESTABLISHED was the only reasonable state in there, but SS_CLOSING will make sense too then, I guess. A throughout check is necessary for this. Marcelo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received 2016-06-03 14:42 [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received Xin Long 2016-06-03 17:49 ` Marcelo Ricardo Leitner @ 2016-06-06 3:12 ` David Miller 1 sibling, 0 replies; 10+ messages in thread From: David Miller @ 2016-06-06 3:12 UTC (permalink / raw) To: lucien.xin Cc: netdev, linux-sctp, marcelo.leitner, vyasevich, daniel, eric.dumazet From: Xin Long <lucien.xin@gmail.com> Date: Fri, 3 Jun 2016 22:42:45 +0800 > Now sctp doesn't change socket state upon shutdown reception. It changes > just the assoc state, even though it's a TCP-style socket. > > For some cases, if we really need to check sk->sk_state, it's necessary to > fix this issue, at least when we use ss or netstat to dump, we can get a > more exact information. > > As an improvement, we will change sk->sk_state when we change asoc->state > to SHUTDOWN_RECEIVED, and also do it in sctp_shutdown to keep consistent with > sctp_close. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> It looks like some changes are still needed for this. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-06-08 12:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-03 14:42 [PATCHv2 net-next] sctp: sctp should change socket state when shutdown is received Xin Long 2016-06-03 17:49 ` Marcelo Ricardo Leitner 2016-06-04 9:45 ` Xin Long 2016-06-04 12:22 ` Marcelo Ricardo Leitner 2016-06-07 11:03 ` Xin Long 2016-06-07 12:08 ` Marcelo Ricardo Leitner 2016-06-08 10:17 ` Xin Long 2016-06-08 10:42 ` Xin Long 2016-06-08 12:05 ` Marcelo Ricardo Leitner 2016-06-06 3:12 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).