* [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK
@ 2019-05-09 6:28 Xin Long
2019-05-09 11:32 ` Neil Horman
0 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2019-05-09 6:28 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
SCTP_CMD_GEN_INIT_ACK was introduced since very beginning, but never
got used. So remove it.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/command.h | 1 -
net/sctp/sm_sideeffect.c | 11 -----------
2 files changed, 12 deletions(-)
diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
index 6d5beac..b4e8706 100644
--- a/include/net/sctp/command.h
+++ b/include/net/sctp/command.h
@@ -48,7 +48,6 @@ enum sctp_verb {
SCTP_CMD_REPORT_TSN, /* Record the arrival of a TSN. */
SCTP_CMD_GEN_SACK, /* Send a Selective ACK (maybe). */
SCTP_CMD_PROCESS_SACK, /* Process an inbound SACK. */
- SCTP_CMD_GEN_INIT_ACK, /* Generate an INIT ACK chunk. */
SCTP_CMD_PEER_INIT, /* Process a INIT from the peer. */
SCTP_CMD_GEN_COOKIE_ECHO, /* Generate a COOKIE ECHO chunk. */
SCTP_CMD_CHUNK_ULP, /* Send a chunk to the sockets layer. */
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 4aa0358..233ee80 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1364,17 +1364,6 @@ static int sctp_cmd_interpreter(enum sctp_event_type event_type,
cmd->obj.chunk);
break;
- case SCTP_CMD_GEN_INIT_ACK:
- /* Generate an INIT ACK chunk. */
- new_obj = sctp_make_init_ack(asoc, chunk, GFP_ATOMIC,
- 0);
- if (!new_obj)
- goto nomem;
-
- sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
- SCTP_CHUNK(new_obj));
- break;
-
case SCTP_CMD_PEER_INIT:
/* Process a unified INIT from the peer.
* Note: Only used during INIT-ACK processing. If
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK
2019-05-09 6:28 [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK Xin Long
@ 2019-05-09 11:32 ` Neil Horman
2019-05-09 16:39 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Neil Horman @ 2019-05-09 11:32 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem
On Thu, May 09, 2019 at 02:28:00PM +0800, Xin Long wrote:
> SCTP_CMD_GEN_INIT_ACK was introduced since very beginning, but never
> got used. So remove it.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> include/net/sctp/command.h | 1 -
> net/sctp/sm_sideeffect.c | 11 -----------
> 2 files changed, 12 deletions(-)
>
> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> index 6d5beac..b4e8706 100644
> --- a/include/net/sctp/command.h
> +++ b/include/net/sctp/command.h
> @@ -48,7 +48,6 @@ enum sctp_verb {
> SCTP_CMD_REPORT_TSN, /* Record the arrival of a TSN. */
> SCTP_CMD_GEN_SACK, /* Send a Selective ACK (maybe). */
> SCTP_CMD_PROCESS_SACK, /* Process an inbound SACK. */
> - SCTP_CMD_GEN_INIT_ACK, /* Generate an INIT ACK chunk. */
> SCTP_CMD_PEER_INIT, /* Process a INIT from the peer. */
> SCTP_CMD_GEN_COOKIE_ECHO, /* Generate a COOKIE ECHO chunk. */
> SCTP_CMD_CHUNK_ULP, /* Send a chunk to the sockets layer. */
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 4aa0358..233ee80 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1364,17 +1364,6 @@ static int sctp_cmd_interpreter(enum sctp_event_type event_type,
> cmd->obj.chunk);
> break;
>
> - case SCTP_CMD_GEN_INIT_ACK:
> - /* Generate an INIT ACK chunk. */
> - new_obj = sctp_make_init_ack(asoc, chunk, GFP_ATOMIC,
> - 0);
> - if (!new_obj)
> - goto nomem;
> -
> - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> - SCTP_CHUNK(new_obj));
> - break;
> -
> case SCTP_CMD_PEER_INIT:
> /* Process a unified INIT from the peer.
> * Note: Only used during INIT-ACK processing. If
> --
> 2.1.0
>
>
This is definately a valid cleanup, but I wonder if it wouldn't be better to,
instead of removing it, to use it. We have 2 locations where we actually call
sctp_make_init_ack, and then have to check the return code and abort the
operation if we get a NULL return. Would it be a better solution (in the sense
of keeping our control flow in line with how the rest of the state machine is
supposed to work), if we didn't just add a SCTP_CMD_GEN_INIT_ACK sideeffect to
the state machine queue in the locations where we otherwise would call
sctp_make_init_ack/sctp_add_cmd_sf(...SCTP_CMD_REPLY)?
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK
2019-05-09 11:32 ` Neil Horman
@ 2019-05-09 16:39 ` David Miller
2019-05-10 11:27 ` Neil Horman
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-05-09 16:39 UTC (permalink / raw)
To: nhorman; +Cc: lucien.xin, netdev, linux-sctp, marcelo.leitner
From: Neil Horman <nhorman@tuxdriver.com>
Date: Thu, 9 May 2019 07:32:35 -0400
> This is definately a valid cleanup, but I wonder if it wouldn't be better to,
> instead of removing it, to use it. We have 2 locations where we actually call
> sctp_make_init_ack, and then have to check the return code and abort the
> operation if we get a NULL return. Would it be a better solution (in the sense
> of keeping our control flow in line with how the rest of the state machine is
> supposed to work), if we didn't just add a SCTP_CMD_GEN_INIT_ACK sideeffect to
> the state machine queue in the locations where we otherwise would call
> sctp_make_init_ack/sctp_add_cmd_sf(...SCTP_CMD_REPLY)?
Also, net-next is closed 8-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK
2019-05-09 16:39 ` David Miller
@ 2019-05-10 11:27 ` Neil Horman
2019-05-12 5:52 ` Xin Long
0 siblings, 1 reply; 6+ messages in thread
From: Neil Horman @ 2019-05-10 11:27 UTC (permalink / raw)
To: David Miller; +Cc: lucien.xin, netdev, linux-sctp, marcelo.leitner
On Thu, May 09, 2019 at 09:39:13AM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Thu, 9 May 2019 07:32:35 -0400
>
> > This is definately a valid cleanup, but I wonder if it wouldn't be better to,
> > instead of removing it, to use it. We have 2 locations where we actually call
> > sctp_make_init_ack, and then have to check the return code and abort the
> > operation if we get a NULL return. Would it be a better solution (in the sense
> > of keeping our control flow in line with how the rest of the state machine is
> > supposed to work), if we didn't just add a SCTP_CMD_GEN_INIT_ACK sideeffect to
> > the state machine queue in the locations where we otherwise would call
> > sctp_make_init_ack/sctp_add_cmd_sf(...SCTP_CMD_REPLY)?
>
> Also, net-next is closed 8-)
>
Details, details :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK
2019-05-10 11:27 ` Neil Horman
@ 2019-05-12 5:52 ` Xin Long
2019-05-15 14:12 ` Neil Horman
0 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2019-05-12 5:52 UTC (permalink / raw)
To: Neil Horman
Cc: David Miller, network dev, linux-sctp, Marcelo Ricardo Leitner
On Fri, May 10, 2019 at 7:27 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Thu, May 09, 2019 at 09:39:13AM -0700, David Miller wrote:
> > From: Neil Horman <nhorman@tuxdriver.com>
> > Date: Thu, 9 May 2019 07:32:35 -0400
> >
> > > This is definately a valid cleanup, but I wonder if it wouldn't be better to,
> > > instead of removing it, to use it. We have 2 locations where we actually call
> > > sctp_make_init_ack, and then have to check the return code and abort the
> > > operation if we get a NULL return. Would it be a better solution (in the sense
> > > of keeping our control flow in line with how the rest of the state machine is
> > > supposed to work), if we didn't just add a SCTP_CMD_GEN_INIT_ACK sideeffect to
> > > the state machine queue in the locations where we otherwise would call
> > > sctp_make_init_ack/sctp_add_cmd_sf(...SCTP_CMD_REPLY)?
I think they didn't do that, as the new INIT_ACK needs to add unk_param from
the err_chunk which is allocated and freed in those two places
sctp_sf_do_5_1B_init()/sctp_sf_do_unexpected_init().
It looks not good to pass that err_chunk as a param to the state machine.
> >
> > Also, net-next is closed 8-)
> >
> Details, details :)
>
So everytime before posting a patch on net-next,
I should check http://vger.kernel.org/~davem/net-next.html first, right?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK
2019-05-12 5:52 ` Xin Long
@ 2019-05-15 14:12 ` Neil Horman
0 siblings, 0 replies; 6+ messages in thread
From: Neil Horman @ 2019-05-15 14:12 UTC (permalink / raw)
To: Xin Long
Cc: David Miller, network dev, linux-sctp, Marcelo Ricardo Leitner,
hange-folder>?
On Sun, May 12, 2019 at 01:52:48PM +0800, Xin Long wrote:
> On Fri, May 10, 2019 at 7:27 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Thu, May 09, 2019 at 09:39:13AM -0700, David Miller wrote:
> > > From: Neil Horman <nhorman@tuxdriver.com>
> > > Date: Thu, 9 May 2019 07:32:35 -0400
> > >
> > > > This is definately a valid cleanup, but I wonder if it wouldn't be better to,
> > > > instead of removing it, to use it. We have 2 locations where we actually call
> > > > sctp_make_init_ack, and then have to check the return code and abort the
> > > > operation if we get a NULL return. Would it be a better solution (in the sense
> > > > of keeping our control flow in line with how the rest of the state machine is
> > > > supposed to work), if we didn't just add a SCTP_CMD_GEN_INIT_ACK sideeffect to
> > > > the state machine queue in the locations where we otherwise would call
> > > > sctp_make_init_ack/sctp_add_cmd_sf(...SCTP_CMD_REPLY)?
> I think they didn't do that, as the new INIT_ACK needs to add unk_param from
> the err_chunk which is allocated and freed in those two places
> sctp_sf_do_5_1B_init()/sctp_sf_do_unexpected_init().
>
> It looks not good to pass that err_chunk as a param to the state machine.
>
Hmm, perhaps you're right, this does look like the more clean way to do
this, even if its outside the state machine ordering
Neil
> > >
> > > Also, net-next is closed 8-)
> > >
> > Details, details :)
> >
> So everytime before posting a patch on net-next,
> I should check http://vger.kernel.org/~davem/net-next.html first, right?
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-05-15 14:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-09 6:28 [PATCH net-next] sctp: remove unused cmd SCTP_CMD_GEN_INIT_ACK Xin Long
2019-05-09 11:32 ` Neil Horman
2019-05-09 16:39 ` David Miller
2019-05-10 11:27 ` Neil Horman
2019-05-12 5:52 ` Xin Long
2019-05-15 14:12 ` Neil Horman
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).