* [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
@ 2017-12-12 9:25 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
validate_ftsn is added as a member of sctp_stream_interleave, used to
validate ssn/chunk type for fwdtsn or mid (message id)/chunk type for
ifwdtsn, called in sctp_sf_eat_fwd_tsn, just as validate_data.
If this check fails, an abort packet will be sent, as said in section
2.3.1 of RFC8260.
As ifwdtsn and fwdtsn chunks have different length, it also defines
ftsn_chunk_len for sctp_stream_interleave to describe the chunk size.
Then it replaces all sizeof(struct sctp_fwdtsn_chunk) with
sctp_ftsnchk_len.
It also adds the process for ifwdtsn in rx path. As Marcelo pointed
out, there's no need to add event table for ifwdtsn, but just share
prsctp_chunk_event_table with fwdtsn's. It would drop fwdtsn chunk
for ifwdtsn and drop ifwdtsn chunk for fwdtsn by calling validate_ftsn
in sctp_sf_eat_fwd_tsn.
After this patch, the ifwdtsn can be accepted.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/stream_interleave.h | 2 ++
include/net/sctp/structs.h | 10 ++++++++
net/sctp/sm_statefuns.c | 24 +++++++-------------
net/sctp/sm_statetable.c | 3 ++-
net/sctp/stream_interleave.c | 44 ++++++++++++++++++++++++++++++++++++
5 files changed, 66 insertions(+), 17 deletions(-)
diff --git a/include/net/sctp/stream_interleave.h b/include/net/sctp/stream_interleave.h
index 66267db..0db15b5 100644
--- a/include/net/sctp/stream_interleave.h
+++ b/include/net/sctp/stream_interleave.h
@@ -33,6 +33,7 @@
struct sctp_stream_interleave {
__u16 data_chunk_len;
+ __u16 ftsn_chunk_len;
/* (I-)DATA process */
struct sctp_chunk *(*make_datafrag)(const struct sctp_association *asoc,
const struct sctp_sndrcvinfo *sinfo,
@@ -49,6 +50,7 @@ struct sctp_stream_interleave {
void (*abort_pd)(struct sctp_ulpq *ulpq, gfp_t gfp);
/* (I-)FORWARD-TSN process */
void (*generate_ftsn)(struct sctp_outq *q, __u32 ctsn);
+ bool (*validate_ftsn)(struct sctp_chunk *chunk);
};
void sctp_stream_interleave_init(struct sctp_stream *stream);
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index b7720d6..8ac4d5c 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1443,6 +1443,16 @@ static inline __u16 sctp_datahdr_len(const struct sctp_stream *stream)
return stream->si->data_chunk_len - sizeof(struct sctp_chunkhdr);
}
+static inline __u16 sctp_ftsnchk_len(const struct sctp_stream *stream)
+{
+ return stream->si->ftsn_chunk_len;
+}
+
+static inline __u16 sctp_ftsnhdr_len(const struct sctp_stream *stream)
+{
+ return stream->si->ftsn_chunk_len - sizeof(struct sctp_chunkhdr);
+}
+
/* SCTP_GET_ASSOC_STATS counters */
struct sctp_priv_assoc_stats {
/* Maximum observed rto in the association during subsequent
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index c609c54..541f347 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -3957,7 +3957,6 @@ enum sctp_disposition sctp_sf_eat_fwd_tsn(struct net *net,
{
struct sctp_fwdtsn_hdr *fwdtsn_hdr;
struct sctp_chunk *chunk = arg;
- struct sctp_fwdtsn_skip *skip;
__u16 len;
__u32 tsn;
@@ -3971,7 +3970,7 @@ enum sctp_disposition sctp_sf_eat_fwd_tsn(struct net *net,
return sctp_sf_unk_chunk(net, ep, asoc, type, arg, commands);
/* Make sure that the FORWARD_TSN chunk has valid length. */
- if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_fwdtsn_chunk)))
+ if (!sctp_chunk_length_valid(chunk, sctp_ftsnchk_len(&asoc->stream)))
return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
commands);
@@ -3990,14 +3989,11 @@ enum sctp_disposition sctp_sf_eat_fwd_tsn(struct net *net,
if (sctp_tsnmap_check(&asoc->peer.tsn_map, tsn) < 0)
goto discard_noforce;
- /* Silently discard the chunk if stream-id is not valid */
- sctp_walk_fwdtsn(skip, chunk) {
- if (ntohs(skip->stream) >= asoc->stream.incnt)
- goto discard_noforce;
- }
+ if (!asoc->stream.si->validate_ftsn(chunk))
+ goto discard_noforce;
sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_FWDTSN, SCTP_U32(tsn));
- if (len > sizeof(struct sctp_fwdtsn_hdr))
+ if (len > sctp_ftsnhdr_len(&asoc->stream))
sctp_add_cmd_sf(commands, SCTP_CMD_PROCESS_FWDTSN,
SCTP_CHUNK(chunk));
@@ -4028,7 +4024,6 @@ enum sctp_disposition sctp_sf_eat_fwd_tsn_fast(
{
struct sctp_fwdtsn_hdr *fwdtsn_hdr;
struct sctp_chunk *chunk = arg;
- struct sctp_fwdtsn_skip *skip;
__u16 len;
__u32 tsn;
@@ -4042,7 +4037,7 @@ enum sctp_disposition sctp_sf_eat_fwd_tsn_fast(
return sctp_sf_unk_chunk(net, ep, asoc, type, arg, commands);
/* Make sure that the FORWARD_TSN chunk has a valid length. */
- if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_fwdtsn_chunk)))
+ if (!sctp_chunk_length_valid(chunk, sctp_ftsnchk_len(&asoc->stream)))
return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
commands);
@@ -4061,14 +4056,11 @@ enum sctp_disposition sctp_sf_eat_fwd_tsn_fast(
if (sctp_tsnmap_check(&asoc->peer.tsn_map, tsn) < 0)
goto gen_shutdown;
- /* Silently discard the chunk if stream-id is not valid */
- sctp_walk_fwdtsn(skip, chunk) {
- if (ntohs(skip->stream) >= asoc->stream.incnt)
- goto gen_shutdown;
- }
+ if (!asoc->stream.si->validate_ftsn(chunk))
+ goto gen_shutdown;
sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_FWDTSN, SCTP_U32(tsn));
- if (len > sizeof(struct sctp_fwdtsn_hdr))
+ if (len > sctp_ftsnhdr_len(&asoc->stream))
sctp_add_cmd_sf(commands, SCTP_CMD_PROCESS_FWDTSN,
SCTP_CHUNK(chunk));
diff --git a/net/sctp/sm_statetable.c b/net/sctp/sm_statetable.c
index 8c9bb41..03a5d7e 100644
--- a/net/sctp/sm_statetable.c
+++ b/net/sctp/sm_statetable.c
@@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
return &chunk_event_table[cid][state];
if (net->sctp.prsctp_enable) {
- if (cid = SCTP_CID_FWD_TSN)
+ if (cid = SCTP_CID_FWD_TSN ||
+ (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
return &prsctp_chunk_event_table[0][state];
}
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index 2ead372..cc4a5e3 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
}
}
+#define _sctp_walk_ifwdtsn(pos, chunk, end) \
+ for (pos = chunk->subh.ifwdtsn_hdr->skip; \
+ (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
+
+#define sctp_walk_ifwdtsn(pos, ch) \
+ _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
+ sizeof(struct sctp_ifwdtsn_chunk))
+
+static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
+{
+ struct sctp_fwdtsn_skip *skip;
+ __u16 incnt;
+
+ if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
+ return false;
+
+ incnt = chunk->asoc->stream.incnt;
+ sctp_walk_fwdtsn(skip, chunk)
+ if (ntohs(skip->stream) >= incnt)
+ return false;
+
+ return true;
+}
+
+static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
+{
+ struct sctp_ifwdtsn_skip *skip;
+ __u16 incnt;
+
+ if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
+ return false;
+
+ incnt = chunk->asoc->stream.incnt;
+ sctp_walk_ifwdtsn(skip, chunk)
+ if (ntohs(skip->stream) >= incnt)
+ return false;
+
+ return true;
+}
+
static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.data_chunk_len = sizeof(struct sctp_data_chunk),
+ .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
/* DATA process functions */
.make_datafrag = sctp_make_datafrag_empty,
.assign_number = sctp_chunk_assign_ssn,
@@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.abort_pd = sctp_ulpq_abort_pd,
/* FORWARD-TSN process functions */
.generate_ftsn = sctp_generate_fwdtsn,
+ .validate_ftsn = sctp_validate_fwdtsn,
};
static struct sctp_stream_interleave sctp_stream_interleave_1 = {
.data_chunk_len = sizeof(struct sctp_idata_chunk),
+ .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
/* I-DATA process functions */
.make_datafrag = sctp_make_idatafrag_empty,
.assign_number = sctp_chunk_assign_mid,
@@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
.abort_pd = sctp_intl_abort_pd,
/* I-FORWARD-TSN process functions */
.generate_ftsn = sctp_generate_iftsn,
+ .validate_ftsn = sctp_validate_iftsn,
};
void sctp_stream_interleave_init(struct sctp_stream *stream)
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 4/8] sctp: implement report_ftsn for sctp_stream_interleave
2017-12-12 9:25 ` Xin Long
@ 2017-12-12 9:25 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
report_ftsn is added as a member of sctp_stream_interleave, used to
skip tsn from tsnmap, remove old events from reasm or lobby queue,
and abort pd for data or idata, called for SCTP_CMD_REPORT_FWDTSN
cmd and asoc reset.
sctp_report_iftsn works for ifwdtsn, and sctp_report_fwdtsn works
for fwdtsn. Note that sctp_report_iftsn doesn't do asoc abort_pd,
as stream abort_pd will be done when handling ifwdtsn. But when
ftsn is equal with ftsn, which means asoc reset, asoc abot_pd has
to be done.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/stream_interleave.h | 1 +
net/sctp/sm_sideeffect.c | 9 +------
net/sctp/stream.c | 6 ++---
net/sctp/stream_interleave.c | 48 ++++++++++++++++++++++++++++++++++++
4 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/include/net/sctp/stream_interleave.h b/include/net/sctp/stream_interleave.h
index 0db15b5..0b55c70 100644
--- a/include/net/sctp/stream_interleave.h
+++ b/include/net/sctp/stream_interleave.h
@@ -51,6 +51,7 @@ struct sctp_stream_interleave {
/* (I-)FORWARD-TSN process */
void (*generate_ftsn)(struct sctp_outq *q, __u32 ctsn);
bool (*validate_ftsn)(struct sctp_chunk *chunk);
+ void (*report_ftsn)(struct sctp_ulpq *ulpq, __u32 ftsn);
};
void sctp_stream_interleave_init(struct sctp_stream *stream);
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 8adde71..be7c6dbd 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1368,14 +1368,7 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,
break;
case SCTP_CMD_REPORT_FWDTSN:
- /* Move the Cumulattive TSN Ack ahead. */
- sctp_tsnmap_skip(&asoc->peer.tsn_map, cmd->obj.u32);
-
- /* purge the fragmentation queue */
- sctp_ulpq_reasm_flushtsn(&asoc->ulpq, cmd->obj.u32);
-
- /* Abort any in progress partial delivery. */
- sctp_ulpq_abort_pd(&asoc->ulpq, GFP_ATOMIC);
+ asoc->stream.si->report_ftsn(&asoc->ulpq, cmd->obj.u32);
break;
case SCTP_CMD_PROCESS_FWDTSN:
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 8370e6c..b3a9f37 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -754,8 +754,7 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
* performed.
*/
max_tsn_seen = sctp_tsnmap_get_max_tsn_seen(&asoc->peer.tsn_map);
- sctp_ulpq_reasm_flushtsn(&asoc->ulpq, max_tsn_seen);
- sctp_ulpq_abort_pd(&asoc->ulpq, GFP_ATOMIC);
+ asoc->stream.si->report_ftsn(&asoc->ulpq, max_tsn_seen);
/* G1: Compute an appropriate value for the Receiver's Next TSN -- the
* TSN that the peer should use to send the next DATA chunk. The
@@ -1024,8 +1023,7 @@ struct sctp_chunk *sctp_process_strreset_resp(
&asoc->peer.tsn_map);
LIST_HEAD(temp);
- sctp_ulpq_reasm_flushtsn(&asoc->ulpq, mtsn);
- sctp_ulpq_abort_pd(&asoc->ulpq, GFP_ATOMIC);
+ asoc->stream.si->report_ftsn(&asoc->ulpq, mtsn);
sctp_tsnmap_init(&asoc->peer.tsn_map,
SCTP_TSN_MAP_INITIAL,
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index cc4a5e3..f62771cc 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1193,6 +1193,52 @@ static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
return true;
}
+static void sctp_report_fwdtsn(struct sctp_ulpq *ulpq, __u32 ftsn)
+{
+ /* Move the Cumulattive TSN Ack ahead. */
+ sctp_tsnmap_skip(&ulpq->asoc->peer.tsn_map, ftsn);
+ /* purge the fragmentation queue */
+ sctp_ulpq_reasm_flushtsn(ulpq, ftsn);
+ /* Abort any in progress partial delivery. */
+ sctp_ulpq_abort_pd(ulpq, GFP_ATOMIC);
+}
+
+static void sctp_intl_reasm_flushtsn(struct sctp_ulpq *ulpq, __u32 ftsn)
+{
+ struct sk_buff *pos, *tmp;
+
+ skb_queue_walk_safe(&ulpq->reasm, pos, tmp) {
+ struct sctp_ulpevent *event = sctp_skb2event(pos);
+ __u32 tsn = event->tsn;
+
+ if (TSN_lte(tsn, ftsn)) {
+ __skb_unlink(pos, &ulpq->reasm);
+ sctp_ulpevent_free(event);
+ }
+ }
+
+ skb_queue_walk_safe(&ulpq->reasm_uo, pos, tmp) {
+ struct sctp_ulpevent *event = sctp_skb2event(pos);
+ __u32 tsn = event->tsn;
+
+ if (TSN_lte(tsn, ftsn)) {
+ __skb_unlink(pos, &ulpq->reasm_uo);
+ sctp_ulpevent_free(event);
+ }
+ }
+}
+
+static void sctp_report_iftsn(struct sctp_ulpq *ulpq, __u32 ftsn)
+{
+ /* Move the Cumulattive TSN Ack ahead. */
+ sctp_tsnmap_skip(&ulpq->asoc->peer.tsn_map, ftsn);
+ /* purge the fragmentation queue */
+ sctp_intl_reasm_flushtsn(ulpq, ftsn);
+ /* abort only when it's for all data */
+ if (ftsn == sctp_tsnmap_get_max_tsn_seen(&ulpq->asoc->peer.tsn_map))
+ sctp_intl_abort_pd(ulpq, GFP_ATOMIC);
+}
+
static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.data_chunk_len = sizeof(struct sctp_data_chunk),
.ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
@@ -1208,6 +1254,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
/* FORWARD-TSN process functions */
.generate_ftsn = sctp_generate_fwdtsn,
.validate_ftsn = sctp_validate_fwdtsn,
+ .report_ftsn = sctp_report_fwdtsn,
};
static struct sctp_stream_interleave sctp_stream_interleave_1 = {
@@ -1225,6 +1272,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
/* I-FORWARD-TSN process functions */
.generate_ftsn = sctp_generate_iftsn,
.validate_ftsn = sctp_validate_iftsn,
+ .report_ftsn = sctp_report_iftsn,
};
void sctp_stream_interleave_init(struct sctp_stream *stream)
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 4/8] sctp: implement report_ftsn for sctp_stream_interleave
@ 2017-12-12 9:25 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
report_ftsn is added as a member of sctp_stream_interleave, used to
skip tsn from tsnmap, remove old events from reasm or lobby queue,
and abort pd for data or idata, called for SCTP_CMD_REPORT_FWDTSN
cmd and asoc reset.
sctp_report_iftsn works for ifwdtsn, and sctp_report_fwdtsn works
for fwdtsn. Note that sctp_report_iftsn doesn't do asoc abort_pd,
as stream abort_pd will be done when handling ifwdtsn. But when
ftsn is equal with ftsn, which means asoc reset, asoc abot_pd has
to be done.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/stream_interleave.h | 1 +
net/sctp/sm_sideeffect.c | 9 +------
net/sctp/stream.c | 6 ++---
net/sctp/stream_interleave.c | 48 ++++++++++++++++++++++++++++++++++++
4 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/include/net/sctp/stream_interleave.h b/include/net/sctp/stream_interleave.h
index 0db15b5..0b55c70 100644
--- a/include/net/sctp/stream_interleave.h
+++ b/include/net/sctp/stream_interleave.h
@@ -51,6 +51,7 @@ struct sctp_stream_interleave {
/* (I-)FORWARD-TSN process */
void (*generate_ftsn)(struct sctp_outq *q, __u32 ctsn);
bool (*validate_ftsn)(struct sctp_chunk *chunk);
+ void (*report_ftsn)(struct sctp_ulpq *ulpq, __u32 ftsn);
};
void sctp_stream_interleave_init(struct sctp_stream *stream);
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 8adde71..be7c6dbd 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1368,14 +1368,7 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,
break;
case SCTP_CMD_REPORT_FWDTSN:
- /* Move the Cumulattive TSN Ack ahead. */
- sctp_tsnmap_skip(&asoc->peer.tsn_map, cmd->obj.u32);
-
- /* purge the fragmentation queue */
- sctp_ulpq_reasm_flushtsn(&asoc->ulpq, cmd->obj.u32);
-
- /* Abort any in progress partial delivery. */
- sctp_ulpq_abort_pd(&asoc->ulpq, GFP_ATOMIC);
+ asoc->stream.si->report_ftsn(&asoc->ulpq, cmd->obj.u32);
break;
case SCTP_CMD_PROCESS_FWDTSN:
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 8370e6c..b3a9f37 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -754,8 +754,7 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
* performed.
*/
max_tsn_seen = sctp_tsnmap_get_max_tsn_seen(&asoc->peer.tsn_map);
- sctp_ulpq_reasm_flushtsn(&asoc->ulpq, max_tsn_seen);
- sctp_ulpq_abort_pd(&asoc->ulpq, GFP_ATOMIC);
+ asoc->stream.si->report_ftsn(&asoc->ulpq, max_tsn_seen);
/* G1: Compute an appropriate value for the Receiver's Next TSN -- the
* TSN that the peer should use to send the next DATA chunk. The
@@ -1024,8 +1023,7 @@ struct sctp_chunk *sctp_process_strreset_resp(
&asoc->peer.tsn_map);
LIST_HEAD(temp);
- sctp_ulpq_reasm_flushtsn(&asoc->ulpq, mtsn);
- sctp_ulpq_abort_pd(&asoc->ulpq, GFP_ATOMIC);
+ asoc->stream.si->report_ftsn(&asoc->ulpq, mtsn);
sctp_tsnmap_init(&asoc->peer.tsn_map,
SCTP_TSN_MAP_INITIAL,
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index cc4a5e3..f62771cc 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1193,6 +1193,52 @@ static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
return true;
}
+static void sctp_report_fwdtsn(struct sctp_ulpq *ulpq, __u32 ftsn)
+{
+ /* Move the Cumulattive TSN Ack ahead. */
+ sctp_tsnmap_skip(&ulpq->asoc->peer.tsn_map, ftsn);
+ /* purge the fragmentation queue */
+ sctp_ulpq_reasm_flushtsn(ulpq, ftsn);
+ /* Abort any in progress partial delivery. */
+ sctp_ulpq_abort_pd(ulpq, GFP_ATOMIC);
+}
+
+static void sctp_intl_reasm_flushtsn(struct sctp_ulpq *ulpq, __u32 ftsn)
+{
+ struct sk_buff *pos, *tmp;
+
+ skb_queue_walk_safe(&ulpq->reasm, pos, tmp) {
+ struct sctp_ulpevent *event = sctp_skb2event(pos);
+ __u32 tsn = event->tsn;
+
+ if (TSN_lte(tsn, ftsn)) {
+ __skb_unlink(pos, &ulpq->reasm);
+ sctp_ulpevent_free(event);
+ }
+ }
+
+ skb_queue_walk_safe(&ulpq->reasm_uo, pos, tmp) {
+ struct sctp_ulpevent *event = sctp_skb2event(pos);
+ __u32 tsn = event->tsn;
+
+ if (TSN_lte(tsn, ftsn)) {
+ __skb_unlink(pos, &ulpq->reasm_uo);
+ sctp_ulpevent_free(event);
+ }
+ }
+}
+
+static void sctp_report_iftsn(struct sctp_ulpq *ulpq, __u32 ftsn)
+{
+ /* Move the Cumulattive TSN Ack ahead. */
+ sctp_tsnmap_skip(&ulpq->asoc->peer.tsn_map, ftsn);
+ /* purge the fragmentation queue */
+ sctp_intl_reasm_flushtsn(ulpq, ftsn);
+ /* abort only when it's for all data */
+ if (ftsn = sctp_tsnmap_get_max_tsn_seen(&ulpq->asoc->peer.tsn_map))
+ sctp_intl_abort_pd(ulpq, GFP_ATOMIC);
+}
+
static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.data_chunk_len = sizeof(struct sctp_data_chunk),
.ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
@@ -1208,6 +1254,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
/* FORWARD-TSN process functions */
.generate_ftsn = sctp_generate_fwdtsn,
.validate_ftsn = sctp_validate_fwdtsn,
+ .report_ftsn = sctp_report_fwdtsn,
};
static struct sctp_stream_interleave sctp_stream_interleave_1 = {
@@ -1225,6 +1272,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
/* I-FORWARD-TSN process functions */
.generate_ftsn = sctp_generate_iftsn,
.validate_ftsn = sctp_validate_iftsn,
+ .report_ftsn = sctp_report_iftsn,
};
void sctp_stream_interleave_init(struct sctp_stream *stream)
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 5/8] sctp: implement handle_ftsn for sctp_stream_interleave
2017-12-12 9:25 ` Xin Long
@ 2017-12-12 9:25 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
handle_ftsn is added as a member of sctp_stream_interleave, used to skip
ssn for data or mid for idata, called for SCTP_CMD_PROCESS_FWDTSN cmd.
sctp_handle_iftsn works for ifwdtsn, and sctp_handle_fwdtsn works for
fwdtsn. Note that different from sctp_handle_fwdtsn, sctp_handle_iftsn
could do stream abort pd.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/stream_interleave.h | 2 ++
net/sctp/sm_sideeffect.c | 15 ++---------
net/sctp/stream_interleave.c | 49 ++++++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+), 13 deletions(-)
diff --git a/include/net/sctp/stream_interleave.h b/include/net/sctp/stream_interleave.h
index 0b55c70..6657711 100644
--- a/include/net/sctp/stream_interleave.h
+++ b/include/net/sctp/stream_interleave.h
@@ -52,6 +52,8 @@ struct sctp_stream_interleave {
void (*generate_ftsn)(struct sctp_outq *q, __u32 ctsn);
bool (*validate_ftsn)(struct sctp_chunk *chunk);
void (*report_ftsn)(struct sctp_ulpq *ulpq, __u32 ftsn);
+ void (*handle_ftsn)(struct sctp_ulpq *ulpq,
+ struct sctp_chunk *chunk);
};
void sctp_stream_interleave_init(struct sctp_stream *stream);
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index be7c6dbd..16ddf2c 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1007,18 +1007,6 @@ static void sctp_cmd_process_operr(struct sctp_cmd_seq *cmds,
}
}
-/* Process variable FWDTSN chunk information. */
-static void sctp_cmd_process_fwdtsn(struct sctp_ulpq *ulpq,
- struct sctp_chunk *chunk)
-{
- struct sctp_fwdtsn_skip *skip;
-
- /* Walk through all the skipped SSNs */
- sctp_walk_fwdtsn(skip, chunk) {
- sctp_ulpq_skip(ulpq, ntohs(skip->stream), ntohs(skip->ssn));
- }
-}
-
/* Helper function to remove the association non-primary peer
* transports.
*/
@@ -1372,7 +1360,8 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,
break;
case SCTP_CMD_PROCESS_FWDTSN:
- sctp_cmd_process_fwdtsn(&asoc->ulpq, cmd->obj.chunk);
+ asoc->stream.si->handle_ftsn(&asoc->ulpq,
+ cmd->obj.chunk);
break;
case SCTP_CMD_GEN_SACK:
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index f62771cc..8c7cf8f 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1239,6 +1239,53 @@ static void sctp_report_iftsn(struct sctp_ulpq *ulpq, __u32 ftsn)
sctp_intl_abort_pd(ulpq, GFP_ATOMIC);
}
+static void sctp_handle_fwdtsn(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk)
+{
+ struct sctp_fwdtsn_skip *skip;
+
+ /* Walk through all the skipped SSNs */
+ sctp_walk_fwdtsn(skip, chunk)
+ sctp_ulpq_skip(ulpq, ntohs(skip->stream), ntohs(skip->ssn));
+}
+
+static void sctp_intl_skip(struct sctp_ulpq *ulpq, __u16 sid, __u32 mid,
+ __u8 flags)
+{
+ struct sctp_stream_in *sin = sctp_stream_in(ulpq->asoc, sid);
+ struct sctp_stream *stream = &ulpq->asoc->stream;
+
+ if (flags & SCTP_FTSN_U_BIT) {
+ if (sin->pd_mode_uo && MID_lt(sin->mid_uo, mid)) {
+ sin->pd_mode_uo = 0;
+ sctp_intl_stream_abort_pd(ulpq, sid, mid, 0x1,
+ GFP_ATOMIC);
+ }
+ return;
+ }
+
+ if (MID_lt(mid, sctp_mid_peek(stream, in, sid)))
+ return;
+
+ if (sin->pd_mode) {
+ sin->pd_mode = 0;
+ sctp_intl_stream_abort_pd(ulpq, sid, mid, 0x0, GFP_ATOMIC);
+ }
+
+ sctp_mid_skip(stream, in, sid, mid);
+
+ sctp_intl_reap_ordered(ulpq, sid);
+}
+
+static void sctp_handle_iftsn(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk)
+{
+ struct sctp_ifwdtsn_skip *skip;
+
+ /* Walk through all the skipped MIDs and abort stream pd if possible */
+ sctp_walk_ifwdtsn(skip, chunk)
+ sctp_intl_skip(ulpq, ntohs(skip->stream),
+ ntohl(skip->mid), skip->flags);
+}
+
static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.data_chunk_len = sizeof(struct sctp_data_chunk),
.ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
@@ -1255,6 +1302,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.generate_ftsn = sctp_generate_fwdtsn,
.validate_ftsn = sctp_validate_fwdtsn,
.report_ftsn = sctp_report_fwdtsn,
+ .handle_ftsn = sctp_handle_fwdtsn,
};
static struct sctp_stream_interleave sctp_stream_interleave_1 = {
@@ -1273,6 +1321,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
.generate_ftsn = sctp_generate_iftsn,
.validate_ftsn = sctp_validate_iftsn,
.report_ftsn = sctp_report_iftsn,
+ .handle_ftsn = sctp_handle_iftsn,
};
void sctp_stream_interleave_init(struct sctp_stream *stream)
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 5/8] sctp: implement handle_ftsn for sctp_stream_interleave
@ 2017-12-12 9:25 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
handle_ftsn is added as a member of sctp_stream_interleave, used to skip
ssn for data or mid for idata, called for SCTP_CMD_PROCESS_FWDTSN cmd.
sctp_handle_iftsn works for ifwdtsn, and sctp_handle_fwdtsn works for
fwdtsn. Note that different from sctp_handle_fwdtsn, sctp_handle_iftsn
could do stream abort pd.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/stream_interleave.h | 2 ++
net/sctp/sm_sideeffect.c | 15 ++---------
net/sctp/stream_interleave.c | 49 ++++++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+), 13 deletions(-)
diff --git a/include/net/sctp/stream_interleave.h b/include/net/sctp/stream_interleave.h
index 0b55c70..6657711 100644
--- a/include/net/sctp/stream_interleave.h
+++ b/include/net/sctp/stream_interleave.h
@@ -52,6 +52,8 @@ struct sctp_stream_interleave {
void (*generate_ftsn)(struct sctp_outq *q, __u32 ctsn);
bool (*validate_ftsn)(struct sctp_chunk *chunk);
void (*report_ftsn)(struct sctp_ulpq *ulpq, __u32 ftsn);
+ void (*handle_ftsn)(struct sctp_ulpq *ulpq,
+ struct sctp_chunk *chunk);
};
void sctp_stream_interleave_init(struct sctp_stream *stream);
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index be7c6dbd..16ddf2c 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1007,18 +1007,6 @@ static void sctp_cmd_process_operr(struct sctp_cmd_seq *cmds,
}
}
-/* Process variable FWDTSN chunk information. */
-static void sctp_cmd_process_fwdtsn(struct sctp_ulpq *ulpq,
- struct sctp_chunk *chunk)
-{
- struct sctp_fwdtsn_skip *skip;
-
- /* Walk through all the skipped SSNs */
- sctp_walk_fwdtsn(skip, chunk) {
- sctp_ulpq_skip(ulpq, ntohs(skip->stream), ntohs(skip->ssn));
- }
-}
-
/* Helper function to remove the association non-primary peer
* transports.
*/
@@ -1372,7 +1360,8 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,
break;
case SCTP_CMD_PROCESS_FWDTSN:
- sctp_cmd_process_fwdtsn(&asoc->ulpq, cmd->obj.chunk);
+ asoc->stream.si->handle_ftsn(&asoc->ulpq,
+ cmd->obj.chunk);
break;
case SCTP_CMD_GEN_SACK:
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index f62771cc..8c7cf8f 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1239,6 +1239,53 @@ static void sctp_report_iftsn(struct sctp_ulpq *ulpq, __u32 ftsn)
sctp_intl_abort_pd(ulpq, GFP_ATOMIC);
}
+static void sctp_handle_fwdtsn(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk)
+{
+ struct sctp_fwdtsn_skip *skip;
+
+ /* Walk through all the skipped SSNs */
+ sctp_walk_fwdtsn(skip, chunk)
+ sctp_ulpq_skip(ulpq, ntohs(skip->stream), ntohs(skip->ssn));
+}
+
+static void sctp_intl_skip(struct sctp_ulpq *ulpq, __u16 sid, __u32 mid,
+ __u8 flags)
+{
+ struct sctp_stream_in *sin = sctp_stream_in(ulpq->asoc, sid);
+ struct sctp_stream *stream = &ulpq->asoc->stream;
+
+ if (flags & SCTP_FTSN_U_BIT) {
+ if (sin->pd_mode_uo && MID_lt(sin->mid_uo, mid)) {
+ sin->pd_mode_uo = 0;
+ sctp_intl_stream_abort_pd(ulpq, sid, mid, 0x1,
+ GFP_ATOMIC);
+ }
+ return;
+ }
+
+ if (MID_lt(mid, sctp_mid_peek(stream, in, sid)))
+ return;
+
+ if (sin->pd_mode) {
+ sin->pd_mode = 0;
+ sctp_intl_stream_abort_pd(ulpq, sid, mid, 0x0, GFP_ATOMIC);
+ }
+
+ sctp_mid_skip(stream, in, sid, mid);
+
+ sctp_intl_reap_ordered(ulpq, sid);
+}
+
+static void sctp_handle_iftsn(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk)
+{
+ struct sctp_ifwdtsn_skip *skip;
+
+ /* Walk through all the skipped MIDs and abort stream pd if possible */
+ sctp_walk_ifwdtsn(skip, chunk)
+ sctp_intl_skip(ulpq, ntohs(skip->stream),
+ ntohl(skip->mid), skip->flags);
+}
+
static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.data_chunk_len = sizeof(struct sctp_data_chunk),
.ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
@@ -1255,6 +1302,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
.generate_ftsn = sctp_generate_fwdtsn,
.validate_ftsn = sctp_validate_fwdtsn,
.report_ftsn = sctp_report_fwdtsn,
+ .handle_ftsn = sctp_handle_fwdtsn,
};
static struct sctp_stream_interleave sctp_stream_interleave_1 = {
@@ -1273,6 +1321,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
.generate_ftsn = sctp_generate_iftsn,
.validate_ftsn = sctp_validate_iftsn,
.report_ftsn = sctp_report_iftsn,
+ .handle_ftsn = sctp_handle_iftsn,
};
void sctp_stream_interleave_init(struct sctp_stream *stream)
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 6/8] sctp: add stream interleave support in stream scheduler
2017-12-12 9:25 ` Xin Long
@ 2017-12-12 9:25 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
As Marcelo said in the stream scheduler patch:
Support for I-DATA chunks, also described in RFC8260, with user message
interleaving is straightforward as it just requires the schedulers to
probe for the feature and ignore datamsg boundaries when dequeueing.
All needs to do is just to ignore datamsg boundaries when dequeueing.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/stream_sched.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sctp/stream_sched.c b/net/sctp/stream_sched.c
index d8c162a..f5fcd42 100644
--- a/net/sctp/stream_sched.c
+++ b/net/sctp/stream_sched.c
@@ -242,7 +242,8 @@ int sctp_sched_get_value(struct sctp_association *asoc, __u16 sid,
void sctp_sched_dequeue_done(struct sctp_outq *q, struct sctp_chunk *ch)
{
- if (!list_is_last(&ch->frag_list, &ch->msg->chunks)) {
+ if (!list_is_last(&ch->frag_list, &ch->msg->chunks) &&
+ !q->asoc->intl_enable) {
struct sctp_stream_out *sout;
__u16 sid;
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 6/8] sctp: add stream interleave support in stream scheduler
@ 2017-12-12 9:25 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
As Marcelo said in the stream scheduler patch:
Support for I-DATA chunks, also described in RFC8260, with user message
interleaving is straightforward as it just requires the schedulers to
probe for the feature and ignore datamsg boundaries when dequeueing.
All needs to do is just to ignore datamsg boundaries when dequeueing.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/stream_sched.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sctp/stream_sched.c b/net/sctp/stream_sched.c
index d8c162a..f5fcd42 100644
--- a/net/sctp/stream_sched.c
+++ b/net/sctp/stream_sched.c
@@ -242,7 +242,8 @@ int sctp_sched_get_value(struct sctp_association *asoc, __u16 sid,
void sctp_sched_dequeue_done(struct sctp_outq *q, struct sctp_chunk *ch)
{
- if (!list_is_last(&ch->frag_list, &ch->msg->chunks)) {
+ if (!list_is_last(&ch->frag_list, &ch->msg->chunks) &&
+ !q->asoc->intl_enable) {
struct sctp_stream_out *sout;
__u16 sid;
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 7/8] sctp: update mid instead of ssn when doing stream and asoc reset
2017-12-12 9:25 ` Xin Long
@ 2017-12-12 9:25 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
When using idata and doing stream and asoc reset, setting ssn with
0 could only clear the 1st 16 bits of mid.
So to make this work for both data and idata, it sets mid with 0
instead of ssn, and also mid_uo for unordered idata also need to
be cleared, as said in section 2.3.2 of RFC8260.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/stream.c | 40 +++++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index b3a9f37..06b644d 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -216,11 +216,13 @@ void sctp_stream_clear(struct sctp_stream *stream)
{
int i;
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
}
void sctp_stream_update(struct sctp_stream *stream, struct sctp_stream *new)
@@ -607,10 +609,10 @@ struct sctp_chunk *sctp_process_strreset_outreq(
}
for (i = 0; i < nums; i++)
- stream->in[ntohs(str_p[i])].ssn = 0;
+ stream->in[ntohs(str_p[i])].mid = 0;
} else {
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
}
result = SCTP_STRRESET_PERFORMED;
@@ -783,10 +785,12 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
/* G5: The next expected and outgoing SSNs MUST be reset to 0 for all
* incoming and outgoing streams.
*/
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
result = SCTP_STRRESET_PERFORMED;
@@ -976,11 +980,15 @@ struct sctp_chunk *sctp_process_strreset_resp(
if (result == SCTP_STRRESET_PERFORMED) {
if (nums) {
- for (i = 0; i < nums; i++)
- stream->out[ntohs(str_p[i])].ssn = 0;
+ for (i = 0; i < nums; i++) {
+ stream->out[ntohs(str_p[i])].mid = 0;
+ stream->out[ntohs(str_p[i])].mid_uo = 0;
+ }
} else {
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
}
flags = SCTP_STREAM_RESET_OUTGOING_SSN;
@@ -1041,10 +1049,12 @@ struct sctp_chunk *sctp_process_strreset_resp(
asoc->ctsn_ack_point = asoc->next_tsn - 1;
asoc->adv_peer_ack_point = asoc->ctsn_ack_point;
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
}
for (i = 0; i < stream->outcnt; i++)
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 7/8] sctp: update mid instead of ssn when doing stream and asoc reset
@ 2017-12-12 9:25 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
When using idata and doing stream and asoc reset, setting ssn with
0 could only clear the 1st 16 bits of mid.
So to make this work for both data and idata, it sets mid with 0
instead of ssn, and also mid_uo for unordered idata also need to
be cleared, as said in section 2.3.2 of RFC8260.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/stream.c | 40 +++++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index b3a9f37..06b644d 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -216,11 +216,13 @@ void sctp_stream_clear(struct sctp_stream *stream)
{
int i;
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
}
void sctp_stream_update(struct sctp_stream *stream, struct sctp_stream *new)
@@ -607,10 +609,10 @@ struct sctp_chunk *sctp_process_strreset_outreq(
}
for (i = 0; i < nums; i++)
- stream->in[ntohs(str_p[i])].ssn = 0;
+ stream->in[ntohs(str_p[i])].mid = 0;
} else {
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
}
result = SCTP_STRRESET_PERFORMED;
@@ -783,10 +785,12 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
/* G5: The next expected and outgoing SSNs MUST be reset to 0 for all
* incoming and outgoing streams.
*/
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
result = SCTP_STRRESET_PERFORMED;
@@ -976,11 +980,15 @@ struct sctp_chunk *sctp_process_strreset_resp(
if (result = SCTP_STRRESET_PERFORMED) {
if (nums) {
- for (i = 0; i < nums; i++)
- stream->out[ntohs(str_p[i])].ssn = 0;
+ for (i = 0; i < nums; i++) {
+ stream->out[ntohs(str_p[i])].mid = 0;
+ stream->out[ntohs(str_p[i])].mid_uo = 0;
+ }
} else {
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
}
flags = SCTP_STREAM_RESET_OUTGOING_SSN;
@@ -1041,10 +1049,12 @@ struct sctp_chunk *sctp_process_strreset_resp(
asoc->ctsn_ack_point = asoc->next_tsn - 1;
asoc->adv_peer_ack_point = asoc->ctsn_ack_point;
- for (i = 0; i < stream->outcnt; i++)
- stream->out[i].ssn = 0;
+ for (i = 0; i < stream->outcnt; i++) {
+ stream->out[i].mid = 0;
+ stream->out[i].mid_uo = 0;
+ }
for (i = 0; i < stream->incnt; i++)
- stream->in[i].ssn = 0;
+ stream->in[i].mid = 0;
}
for (i = 0; i < stream->outcnt; i++)
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 8/8] sctp: support sysctl to allow users to use stream interleave
2017-12-12 9:25 ` Xin Long
@ 2017-12-12 9:25 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This is the last patch for support of stream interleave, after this patch,
users could enable stream interleave by systcl -w net.sctp.intl_enable=1.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/sysctl.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index ef7ca44..33ca5b7 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -289,6 +289,13 @@ static struct ctl_table sctp_net_table[] = {
.proc_handler = proc_sctp_do_auth,
},
{
+ .procname = "intl_enable",
+ .data = &init_net.sctp.intl_enable,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
.procname = "addr_scope_policy",
.data = &init_net.sctp.scope_policy,
.maxlen = sizeof(int),
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH net-next 8/8] sctp: support sysctl to allow users to use stream interleave
@ 2017-12-12 9:25 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 9:25 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This is the last patch for support of stream interleave, after this patch,
users could enable stream interleave by systcl -w net.sctp.intl_enable=1.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/sysctl.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index ef7ca44..33ca5b7 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -289,6 +289,13 @@ static struct ctl_table sctp_net_table[] = {
.proc_handler = proc_sctp_do_auth,
},
{
+ .procname = "intl_enable",
+ .data = &init_net.sctp.intl_enable,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
.procname = "addr_scope_policy",
.data = &init_net.sctp.scope_policy,
.maxlen = sizeof(int),
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 4/8] sctp: implement report_ftsn for sctp_stream_interleave
2017-12-12 9:25 ` Xin Long
@ 2017-12-12 13:25 ` Marcelo Ricardo Leitner
-1 siblings, 0 replies; 30+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-12-12 13:25 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 05:25:55PM +0800, Xin Long wrote:
> report_ftsn is added as a member of sctp_stream_interleave, used to
> skip tsn from tsnmap, remove old events from reasm or lobby queue,
> and abort pd for data or idata, called for SCTP_CMD_REPORT_FWDTSN
> cmd and asoc reset.
>
> sctp_report_iftsn works for ifwdtsn, and sctp_report_fwdtsn works
> for fwdtsn. Note that sctp_report_iftsn doesn't do asoc abort_pd,
> as stream abort_pd will be done when handling ifwdtsn. But when
> ftsn is equal with ftsn, which means asoc reset, asoc abot_pd has
nit... aboRt ^^^^ :-)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 4/8] sctp: implement report_ftsn for sctp_stream_interleave
@ 2017-12-12 13:25 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 30+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-12-12 13:25 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 05:25:55PM +0800, Xin Long wrote:
> report_ftsn is added as a member of sctp_stream_interleave, used to
> skip tsn from tsnmap, remove old events from reasm or lobby queue,
> and abort pd for data or idata, called for SCTP_CMD_REPORT_FWDTSN
> cmd and asoc reset.
>
> sctp_report_iftsn works for ifwdtsn, and sctp_report_fwdtsn works
> for fwdtsn. Note that sctp_report_iftsn doesn't do asoc abort_pd,
> as stream abort_pd will be done when handling ifwdtsn. But when
> ftsn is equal with ftsn, which means asoc reset, asoc abot_pd has
nit... aboRt ^^^^ :-)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 4/8] sctp: implement report_ftsn for sctp_stream_interleave
2017-12-12 13:25 ` Marcelo Ricardo Leitner
@ 2017-12-12 14:02 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 14:02 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 9:25 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 05:25:55PM +0800, Xin Long wrote:
>> report_ftsn is added as a member of sctp_stream_interleave, used to
>> skip tsn from tsnmap, remove old events from reasm or lobby queue,
>> and abort pd for data or idata, called for SCTP_CMD_REPORT_FWDTSN
>> cmd and asoc reset.
>>
>> sctp_report_iftsn works for ifwdtsn, and sctp_report_fwdtsn works
>> for fwdtsn. Note that sctp_report_iftsn doesn't do asoc abort_pd,
>> as stream abort_pd will be done when handling ifwdtsn. But when
>> ftsn is equal with ftsn, which means asoc reset, asoc abot_pd has
> nit... aboRt ^^^^ :-)
ah sorry, 'set spell' could tell when it's together with _pd. :D
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 4/8] sctp: implement report_ftsn for sctp_stream_interleave
@ 2017-12-12 14:02 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 14:02 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 9:25 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 05:25:55PM +0800, Xin Long wrote:
>> report_ftsn is added as a member of sctp_stream_interleave, used to
>> skip tsn from tsnmap, remove old events from reasm or lobby queue,
>> and abort pd for data or idata, called for SCTP_CMD_REPORT_FWDTSN
>> cmd and asoc reset.
>>
>> sctp_report_iftsn works for ifwdtsn, and sctp_report_fwdtsn works
>> for fwdtsn. Note that sctp_report_iftsn doesn't do asoc abort_pd,
>> as stream abort_pd will be done when handling ifwdtsn. But when
>> ftsn is equal with ftsn, which means asoc reset, asoc abot_pd has
> nit... aboRt ^^^^ :-)
ah sorry, 'set spell' could tell when it's together with _pd. :D
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
2017-12-12 9:25 ` Xin Long
@ 2017-12-12 13:18 ` Marcelo Ricardo Leitner
-1 siblings, 0 replies; 30+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-12-12 13:18 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
...
> --- a/net/sctp/sm_statetable.c
> +++ b/net/sctp/sm_statetable.c
> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
> return &chunk_event_table[cid][state];
>
> if (net->sctp.prsctp_enable) {
> - if (cid == SCTP_CID_FWD_TSN)
> + if (cid == SCTP_CID_FWD_TSN ||
> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
We don't really need to check intl_enable here, do we?
Because a) it should actually look like:
- if (cid == SCTP_CID_FWD_TSN)
+ if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) ||
+ (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
but b) we will validate the chunk format/feature later with
validate_ftsn(), similarly to what happens with data chunks, so it
seems the check on intl_enable here is not necessary.
Same happens with data chunks, btw.
> return &prsctp_chunk_event_table[0][state];
> }
>
> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
> index 2ead372..cc4a5e3 100644
> --- a/net/sctp/stream_interleave.c
> +++ b/net/sctp/stream_interleave.c
> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
> }
> }
>
> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
> +
> +#define sctp_walk_ifwdtsn(pos, ch) \
> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
> + sizeof(struct sctp_ifwdtsn_chunk))
> +
> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
> +{
> + struct sctp_fwdtsn_skip *skip;
> + __u16 incnt;
> +
> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
> + return false;
> +
> + incnt = chunk->asoc->stream.incnt;
> + sctp_walk_fwdtsn(skip, chunk)
> + if (ntohs(skip->stream) >= incnt)
> + return false;
> +
> + return true;
> +}
> +
> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
> +{
> + struct sctp_ifwdtsn_skip *skip;
> + __u16 incnt;
> +
> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
> + return false;
> +
> + incnt = chunk->asoc->stream.incnt;
> + sctp_walk_ifwdtsn(skip, chunk)
> + if (ntohs(skip->stream) >= incnt)
> + return false;
> +
> + return true;
> +}
> +
> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> .data_chunk_len = sizeof(struct sctp_data_chunk),
> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
> /* DATA process functions */
> .make_datafrag = sctp_make_datafrag_empty,
> .assign_number = sctp_chunk_assign_ssn,
> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> .abort_pd = sctp_ulpq_abort_pd,
> /* FORWARD-TSN process functions */
> .generate_ftsn = sctp_generate_fwdtsn,
> + .validate_ftsn = sctp_validate_fwdtsn,
> };
>
> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> .data_chunk_len = sizeof(struct sctp_idata_chunk),
> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
> /* I-DATA process functions */
> .make_datafrag = sctp_make_idatafrag_empty,
> .assign_number = sctp_chunk_assign_mid,
> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> .abort_pd = sctp_intl_abort_pd,
> /* I-FORWARD-TSN process functions */
> .generate_ftsn = sctp_generate_iftsn,
> + .validate_ftsn = sctp_validate_iftsn,
> };
>
> void sctp_stream_interleave_init(struct sctp_stream *stream)
> --
> 2.1.0
>
> --
> 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
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
@ 2017-12-12 13:18 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 30+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-12-12 13:18 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
...
> --- a/net/sctp/sm_statetable.c
> +++ b/net/sctp/sm_statetable.c
> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
> return &chunk_event_table[cid][state];
>
> if (net->sctp.prsctp_enable) {
> - if (cid = SCTP_CID_FWD_TSN)
> + if (cid = SCTP_CID_FWD_TSN ||
> + (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
We don't really need to check intl_enable here, do we?
Because a) it should actually look like:
- if (cid = SCTP_CID_FWD_TSN)
+ if ((!net->sctp.intl_enable && cid = SCTP_CID_FWD_TSN) ||
+ (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
but b) we will validate the chunk format/feature later with
validate_ftsn(), similarly to what happens with data chunks, so it
seems the check on intl_enable here is not necessary.
Same happens with data chunks, btw.
> return &prsctp_chunk_event_table[0][state];
> }
>
> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
> index 2ead372..cc4a5e3 100644
> --- a/net/sctp/stream_interleave.c
> +++ b/net/sctp/stream_interleave.c
> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
> }
> }
>
> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
> +
> +#define sctp_walk_ifwdtsn(pos, ch) \
> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
> + sizeof(struct sctp_ifwdtsn_chunk))
> +
> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
> +{
> + struct sctp_fwdtsn_skip *skip;
> + __u16 incnt;
> +
> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
> + return false;
> +
> + incnt = chunk->asoc->stream.incnt;
> + sctp_walk_fwdtsn(skip, chunk)
> + if (ntohs(skip->stream) >= incnt)
> + return false;
> +
> + return true;
> +}
> +
> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
> +{
> + struct sctp_ifwdtsn_skip *skip;
> + __u16 incnt;
> +
> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
> + return false;
> +
> + incnt = chunk->asoc->stream.incnt;
> + sctp_walk_ifwdtsn(skip, chunk)
> + if (ntohs(skip->stream) >= incnt)
> + return false;
> +
> + return true;
> +}
> +
> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> .data_chunk_len = sizeof(struct sctp_data_chunk),
> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
> /* DATA process functions */
> .make_datafrag = sctp_make_datafrag_empty,
> .assign_number = sctp_chunk_assign_ssn,
> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> .abort_pd = sctp_ulpq_abort_pd,
> /* FORWARD-TSN process functions */
> .generate_ftsn = sctp_generate_fwdtsn,
> + .validate_ftsn = sctp_validate_fwdtsn,
> };
>
> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> .data_chunk_len = sizeof(struct sctp_idata_chunk),
> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
> /* I-DATA process functions */
> .make_datafrag = sctp_make_idatafrag_empty,
> .assign_number = sctp_chunk_assign_mid,
> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> .abort_pd = sctp_intl_abort_pd,
> /* I-FORWARD-TSN process functions */
> .generate_ftsn = sctp_generate_iftsn,
> + .validate_ftsn = sctp_validate_iftsn,
> };
>
> void sctp_stream_interleave_init(struct sctp_stream *stream)
> --
> 2.1.0
>
> --
> 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
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
2017-12-12 13:18 ` Marcelo Ricardo Leitner
@ 2017-12-12 13:31 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 13:31 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
> ...
>> --- a/net/sctp/sm_statetable.c
>> +++ b/net/sctp/sm_statetable.c
>> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
>> return &chunk_event_table[cid][state];
>>
>> if (net->sctp.prsctp_enable) {
>> - if (cid == SCTP_CID_FWD_TSN)
>> + if (cid == SCTP_CID_FWD_TSN ||
>> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
>
> We don't really need to check intl_enable here, do we?
We think net->sctp.xxx_enable as a main switch here,
that means it only accepts SCTP_CID_I_FWD_TSN
when this switch is open.
As for the check in validate_ftsn(), it follows the RFC checking
on asoc's intl_enable.
does it make sense from this perspective ?
> Because a) it should actually look like:
> - if (cid == SCTP_CID_FWD_TSN)
> + if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) ||
> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
> but b) we will validate the chunk format/feature later with
> validate_ftsn(), similarly to what happens with data chunks, so it
> seems the check on intl_enable here is not necessary.
>
> Same happens with data chunks, btw.
>
>> return &prsctp_chunk_event_table[0][state];
>> }
>>
>> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
>> index 2ead372..cc4a5e3 100644
>> --- a/net/sctp/stream_interleave.c
>> +++ b/net/sctp/stream_interleave.c
>> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
>> }
>> }
>>
>> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
>> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
>> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
>> +
>> +#define sctp_walk_ifwdtsn(pos, ch) \
>> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
>> + sizeof(struct sctp_ifwdtsn_chunk))
>> +
>> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
>> +{
>> + struct sctp_fwdtsn_skip *skip;
>> + __u16 incnt;
>> +
>> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
>> + return false;
>> +
>> + incnt = chunk->asoc->stream.incnt;
>> + sctp_walk_fwdtsn(skip, chunk)
>> + if (ntohs(skip->stream) >= incnt)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
>> +{
>> + struct sctp_ifwdtsn_skip *skip;
>> + __u16 incnt;
>> +
>> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
>> + return false;
>> +
>> + incnt = chunk->asoc->stream.incnt;
>> + sctp_walk_ifwdtsn(skip, chunk)
>> + if (ntohs(skip->stream) >= incnt)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> .data_chunk_len = sizeof(struct sctp_data_chunk),
>> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
>> /* DATA process functions */
>> .make_datafrag = sctp_make_datafrag_empty,
>> .assign_number = sctp_chunk_assign_ssn,
>> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> .abort_pd = sctp_ulpq_abort_pd,
>> /* FORWARD-TSN process functions */
>> .generate_ftsn = sctp_generate_fwdtsn,
>> + .validate_ftsn = sctp_validate_fwdtsn,
>> };
>>
>> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> .data_chunk_len = sizeof(struct sctp_idata_chunk),
>> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
>> /* I-DATA process functions */
>> .make_datafrag = sctp_make_idatafrag_empty,
>> .assign_number = sctp_chunk_assign_mid,
>> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> .abort_pd = sctp_intl_abort_pd,
>> /* I-FORWARD-TSN process functions */
>> .generate_ftsn = sctp_generate_iftsn,
>> + .validate_ftsn = sctp_validate_iftsn,
>> };
>>
>> void sctp_stream_interleave_init(struct sctp_stream *stream)
>> --
>> 2.1.0
>>
>> --
>> 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
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
@ 2017-12-12 13:31 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 13:31 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
> ...
>> --- a/net/sctp/sm_statetable.c
>> +++ b/net/sctp/sm_statetable.c
>> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
>> return &chunk_event_table[cid][state];
>>
>> if (net->sctp.prsctp_enable) {
>> - if (cid = SCTP_CID_FWD_TSN)
>> + if (cid = SCTP_CID_FWD_TSN ||
>> + (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
>
> We don't really need to check intl_enable here, do we?
We think net->sctp.xxx_enable as a main switch here,
that means it only accepts SCTP_CID_I_FWD_TSN
when this switch is open.
As for the check in validate_ftsn(), it follows the RFC checking
on asoc's intl_enable.
does it make sense from this perspective ?
> Because a) it should actually look like:
> - if (cid = SCTP_CID_FWD_TSN)
> + if ((!net->sctp.intl_enable && cid = SCTP_CID_FWD_TSN) ||
> + (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
> but b) we will validate the chunk format/feature later with
> validate_ftsn(), similarly to what happens with data chunks, so it
> seems the check on intl_enable here is not necessary.
>
> Same happens with data chunks, btw.
>
>> return &prsctp_chunk_event_table[0][state];
>> }
>>
>> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
>> index 2ead372..cc4a5e3 100644
>> --- a/net/sctp/stream_interleave.c
>> +++ b/net/sctp/stream_interleave.c
>> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
>> }
>> }
>>
>> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
>> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
>> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
>> +
>> +#define sctp_walk_ifwdtsn(pos, ch) \
>> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
>> + sizeof(struct sctp_ifwdtsn_chunk))
>> +
>> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
>> +{
>> + struct sctp_fwdtsn_skip *skip;
>> + __u16 incnt;
>> +
>> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
>> + return false;
>> +
>> + incnt = chunk->asoc->stream.incnt;
>> + sctp_walk_fwdtsn(skip, chunk)
>> + if (ntohs(skip->stream) >= incnt)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
>> +{
>> + struct sctp_ifwdtsn_skip *skip;
>> + __u16 incnt;
>> +
>> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
>> + return false;
>> +
>> + incnt = chunk->asoc->stream.incnt;
>> + sctp_walk_ifwdtsn(skip, chunk)
>> + if (ntohs(skip->stream) >= incnt)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> .data_chunk_len = sizeof(struct sctp_data_chunk),
>> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
>> /* DATA process functions */
>> .make_datafrag = sctp_make_datafrag_empty,
>> .assign_number = sctp_chunk_assign_ssn,
>> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> .abort_pd = sctp_ulpq_abort_pd,
>> /* FORWARD-TSN process functions */
>> .generate_ftsn = sctp_generate_fwdtsn,
>> + .validate_ftsn = sctp_validate_fwdtsn,
>> };
>>
>> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> .data_chunk_len = sizeof(struct sctp_idata_chunk),
>> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
>> /* I-DATA process functions */
>> .make_datafrag = sctp_make_idatafrag_empty,
>> .assign_number = sctp_chunk_assign_mid,
>> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> .abort_pd = sctp_intl_abort_pd,
>> /* I-FORWARD-TSN process functions */
>> .generate_ftsn = sctp_generate_iftsn,
>> + .validate_ftsn = sctp_validate_iftsn,
>> };
>>
>> void sctp_stream_interleave_init(struct sctp_stream *stream)
>> --
>> 2.1.0
>>
>> --
>> 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
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
2017-12-12 13:31 ` Xin Long
@ 2017-12-12 13:37 ` Marcelo Ricardo Leitner
-1 siblings, 0 replies; 30+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-12-12 13:37 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 09:31:36PM +0800, Xin Long wrote:
> On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
> > ...
> >> --- a/net/sctp/sm_statetable.c
> >> +++ b/net/sctp/sm_statetable.c
> >> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
> >> return &chunk_event_table[cid][state];
> >>
> >> if (net->sctp.prsctp_enable) {
> >> - if (cid == SCTP_CID_FWD_TSN)
> >> + if (cid == SCTP_CID_FWD_TSN ||
> >> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
> >
> > We don't really need to check intl_enable here, do we?
> We think net->sctp.xxx_enable as a main switch here,
> that means it only accepts SCTP_CID_I_FWD_TSN
> when this switch is open.
Yes, but
>
> As for the check in validate_ftsn(), it follows the RFC checking
> on asoc's intl_enable.
Yes.
>
> does it make sense from this perspective ?
Sort of. We need consistency. If we want do discard such invalid
chunks right here, that condition also needs to ensure that fwd_tsn
chunks are actually allowed (IOW, that intl_enable is false).
And we don't do any check for idata chunks. We just just accept them
and discard later when trying to consume it.
>
>
>
> > Because a) it should actually look like:
> > - if (cid == SCTP_CID_FWD_TSN)
> > + if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) ||
> > + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
> > but b) we will validate the chunk format/feature later with
> > validate_ftsn(), similarly to what happens with data chunks, so it
> > seems the check on intl_enable here is not necessary.
> >
> > Same happens with data chunks, btw.
> >
> >> return &prsctp_chunk_event_table[0][state];
> >> }
> >>
> >> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
> >> index 2ead372..cc4a5e3 100644
> >> --- a/net/sctp/stream_interleave.c
> >> +++ b/net/sctp/stream_interleave.c
> >> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
> >> }
> >> }
> >>
> >> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
> >> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
> >> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
> >> +
> >> +#define sctp_walk_ifwdtsn(pos, ch) \
> >> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
> >> + sizeof(struct sctp_ifwdtsn_chunk))
> >> +
> >> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
> >> +{
> >> + struct sctp_fwdtsn_skip *skip;
> >> + __u16 incnt;
> >> +
> >> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
> >> + return false;
> >> +
> >> + incnt = chunk->asoc->stream.incnt;
> >> + sctp_walk_fwdtsn(skip, chunk)
> >> + if (ntohs(skip->stream) >= incnt)
> >> + return false;
> >> +
> >> + return true;
> >> +}
> >> +
> >> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
> >> +{
> >> + struct sctp_ifwdtsn_skip *skip;
> >> + __u16 incnt;
> >> +
> >> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
> >> + return false;
> >> +
> >> + incnt = chunk->asoc->stream.incnt;
> >> + sctp_walk_ifwdtsn(skip, chunk)
> >> + if (ntohs(skip->stream) >= incnt)
> >> + return false;
> >> +
> >> + return true;
> >> +}
> >> +
> >> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> >> .data_chunk_len = sizeof(struct sctp_data_chunk),
> >> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
> >> /* DATA process functions */
> >> .make_datafrag = sctp_make_datafrag_empty,
> >> .assign_number = sctp_chunk_assign_ssn,
> >> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> >> .abort_pd = sctp_ulpq_abort_pd,
> >> /* FORWARD-TSN process functions */
> >> .generate_ftsn = sctp_generate_fwdtsn,
> >> + .validate_ftsn = sctp_validate_fwdtsn,
> >> };
> >>
> >> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> >> .data_chunk_len = sizeof(struct sctp_idata_chunk),
> >> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
> >> /* I-DATA process functions */
> >> .make_datafrag = sctp_make_idatafrag_empty,
> >> .assign_number = sctp_chunk_assign_mid,
> >> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> >> .abort_pd = sctp_intl_abort_pd,
> >> /* I-FORWARD-TSN process functions */
> >> .generate_ftsn = sctp_generate_iftsn,
> >> + .validate_ftsn = sctp_validate_iftsn,
> >> };
> >>
> >> void sctp_stream_interleave_init(struct sctp_stream *stream)
> >> --
> >> 2.1.0
> >>
> >> --
> >> 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
> >>
> --
> 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
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
@ 2017-12-12 13:37 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 30+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-12-12 13:37 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 09:31:36PM +0800, Xin Long wrote:
> On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
> > ...
> >> --- a/net/sctp/sm_statetable.c
> >> +++ b/net/sctp/sm_statetable.c
> >> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
> >> return &chunk_event_table[cid][state];
> >>
> >> if (net->sctp.prsctp_enable) {
> >> - if (cid = SCTP_CID_FWD_TSN)
> >> + if (cid = SCTP_CID_FWD_TSN ||
> >> + (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
> >
> > We don't really need to check intl_enable here, do we?
> We think net->sctp.xxx_enable as a main switch here,
> that means it only accepts SCTP_CID_I_FWD_TSN
> when this switch is open.
Yes, but
>
> As for the check in validate_ftsn(), it follows the RFC checking
> on asoc's intl_enable.
Yes.
>
> does it make sense from this perspective ?
Sort of. We need consistency. If we want do discard such invalid
chunks right here, that condition also needs to ensure that fwd_tsn
chunks are actually allowed (IOW, that intl_enable is false).
And we don't do any check for idata chunks. We just just accept them
and discard later when trying to consume it.
>
>
>
> > Because a) it should actually look like:
> > - if (cid = SCTP_CID_FWD_TSN)
> > + if ((!net->sctp.intl_enable && cid = SCTP_CID_FWD_TSN) ||
> > + (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
> > but b) we will validate the chunk format/feature later with
> > validate_ftsn(), similarly to what happens with data chunks, so it
> > seems the check on intl_enable here is not necessary.
> >
> > Same happens with data chunks, btw.
> >
> >> return &prsctp_chunk_event_table[0][state];
> >> }
> >>
> >> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
> >> index 2ead372..cc4a5e3 100644
> >> --- a/net/sctp/stream_interleave.c
> >> +++ b/net/sctp/stream_interleave.c
> >> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
> >> }
> >> }
> >>
> >> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
> >> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
> >> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
> >> +
> >> +#define sctp_walk_ifwdtsn(pos, ch) \
> >> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
> >> + sizeof(struct sctp_ifwdtsn_chunk))
> >> +
> >> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
> >> +{
> >> + struct sctp_fwdtsn_skip *skip;
> >> + __u16 incnt;
> >> +
> >> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
> >> + return false;
> >> +
> >> + incnt = chunk->asoc->stream.incnt;
> >> + sctp_walk_fwdtsn(skip, chunk)
> >> + if (ntohs(skip->stream) >= incnt)
> >> + return false;
> >> +
> >> + return true;
> >> +}
> >> +
> >> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
> >> +{
> >> + struct sctp_ifwdtsn_skip *skip;
> >> + __u16 incnt;
> >> +
> >> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
> >> + return false;
> >> +
> >> + incnt = chunk->asoc->stream.incnt;
> >> + sctp_walk_ifwdtsn(skip, chunk)
> >> + if (ntohs(skip->stream) >= incnt)
> >> + return false;
> >> +
> >> + return true;
> >> +}
> >> +
> >> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> >> .data_chunk_len = sizeof(struct sctp_data_chunk),
> >> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
> >> /* DATA process functions */
> >> .make_datafrag = sctp_make_datafrag_empty,
> >> .assign_number = sctp_chunk_assign_ssn,
> >> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> >> .abort_pd = sctp_ulpq_abort_pd,
> >> /* FORWARD-TSN process functions */
> >> .generate_ftsn = sctp_generate_fwdtsn,
> >> + .validate_ftsn = sctp_validate_fwdtsn,
> >> };
> >>
> >> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> >> .data_chunk_len = sizeof(struct sctp_idata_chunk),
> >> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
> >> /* I-DATA process functions */
> >> .make_datafrag = sctp_make_idatafrag_empty,
> >> .assign_number = sctp_chunk_assign_mid,
> >> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> >> .abort_pd = sctp_intl_abort_pd,
> >> /* I-FORWARD-TSN process functions */
> >> .generate_ftsn = sctp_generate_iftsn,
> >> + .validate_ftsn = sctp_validate_iftsn,
> >> };
> >>
> >> void sctp_stream_interleave_init(struct sctp_stream *stream)
> >> --
> >> 2.1.0
> >>
> >> --
> >> 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
> >>
> --
> 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
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
2017-12-12 13:37 ` Marcelo Ricardo Leitner
@ 2017-12-12 13:59 ` Xin Long
-1 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 13:59 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 9:37 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 09:31:36PM +0800, Xin Long wrote:
>> On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
>> > ...
>> >> --- a/net/sctp/sm_statetable.c
>> >> +++ b/net/sctp/sm_statetable.c
>> >> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
>> >> return &chunk_event_table[cid][state];
>> >>
>> >> if (net->sctp.prsctp_enable) {
>> >> - if (cid == SCTP_CID_FWD_TSN)
>> >> + if (cid == SCTP_CID_FWD_TSN ||
>> >> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
>> >
>> > We don't really need to check intl_enable here, do we?
>> We think net->sctp.xxx_enable as a main switch here,
>> that means it only accepts SCTP_CID_I_FWD_TSN
>> when this switch is open.
>
> Yes, but
>
>>
>> As for the check in validate_ftsn(), it follows the RFC checking
>> on asoc's intl_enable.
>
> Yes.
>
>>
>> does it make sense from this perspective ?
>
> Sort of. We need consistency. If we want do discard such invalid
> chunks right here, that condition also needs to ensure that fwd_tsn
> chunks are actually allowed (IOW, that intl_enable is false).
>
> And we don't do any check for idata chunks. We just just accept them
> and discard later when trying to consume it.
Yeah, seems not a good idea to check intl_enable here, it's not like
other _enable.
intl_enable is more like a 'which' option, while other _enables are
like 'yes or no' option.
I will just remove intl_enable checks, for both ifwdtsn and idata.
Thanks.
>
>>
>>
>>
>> > Because a) it should actually look like:
>> > - if (cid == SCTP_CID_FWD_TSN)
>> > + if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) ||
>> > + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
>> > but b) we will validate the chunk format/feature later with
>> > validate_ftsn(), similarly to what happens with data chunks, so it
>> > seems the check on intl_enable here is not necessary.
>> >
>> > Same happens with data chunks, btw.
>> >
>> >> return &prsctp_chunk_event_table[0][state];
>> >> }
>> >>
>> >> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
>> >> index 2ead372..cc4a5e3 100644
>> >> --- a/net/sctp/stream_interleave.c
>> >> +++ b/net/sctp/stream_interleave.c
>> >> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
>> >> }
>> >> }
>> >>
>> >> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
>> >> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
>> >> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
>> >> +
>> >> +#define sctp_walk_ifwdtsn(pos, ch) \
>> >> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
>> >> + sizeof(struct sctp_ifwdtsn_chunk))
>> >> +
>> >> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
>> >> +{
>> >> + struct sctp_fwdtsn_skip *skip;
>> >> + __u16 incnt;
>> >> +
>> >> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
>> >> + return false;
>> >> +
>> >> + incnt = chunk->asoc->stream.incnt;
>> >> + sctp_walk_fwdtsn(skip, chunk)
>> >> + if (ntohs(skip->stream) >= incnt)
>> >> + return false;
>> >> +
>> >> + return true;
>> >> +}
>> >> +
>> >> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
>> >> +{
>> >> + struct sctp_ifwdtsn_skip *skip;
>> >> + __u16 incnt;
>> >> +
>> >> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
>> >> + return false;
>> >> +
>> >> + incnt = chunk->asoc->stream.incnt;
>> >> + sctp_walk_ifwdtsn(skip, chunk)
>> >> + if (ntohs(skip->stream) >= incnt)
>> >> + return false;
>> >> +
>> >> + return true;
>> >> +}
>> >> +
>> >> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> >> .data_chunk_len = sizeof(struct sctp_data_chunk),
>> >> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
>> >> /* DATA process functions */
>> >> .make_datafrag = sctp_make_datafrag_empty,
>> >> .assign_number = sctp_chunk_assign_ssn,
>> >> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> >> .abort_pd = sctp_ulpq_abort_pd,
>> >> /* FORWARD-TSN process functions */
>> >> .generate_ftsn = sctp_generate_fwdtsn,
>> >> + .validate_ftsn = sctp_validate_fwdtsn,
>> >> };
>> >>
>> >> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> >> .data_chunk_len = sizeof(struct sctp_idata_chunk),
>> >> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
>> >> /* I-DATA process functions */
>> >> .make_datafrag = sctp_make_idatafrag_empty,
>> >> .assign_number = sctp_chunk_assign_mid,
>> >> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> >> .abort_pd = sctp_intl_abort_pd,
>> >> /* I-FORWARD-TSN process functions */
>> >> .generate_ftsn = sctp_generate_iftsn,
>> >> + .validate_ftsn = sctp_validate_iftsn,
>> >> };
>> >>
>> >> void sctp_stream_interleave_init(struct sctp_stream *stream)
>> >> --
>> >> 2.1.0
>> >>
>> >> --
>> >> 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
>> >>
>> --
>> 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
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
@ 2017-12-12 13:59 ` Xin Long
0 siblings, 0 replies; 30+ messages in thread
From: Xin Long @ 2017-12-12 13:59 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
On Tue, Dec 12, 2017 at 9:37 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 09:31:36PM +0800, Xin Long wrote:
>> On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
>> > ...
>> >> --- a/net/sctp/sm_statetable.c
>> >> +++ b/net/sctp/sm_statetable.c
>> >> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
>> >> return &chunk_event_table[cid][state];
>> >>
>> >> if (net->sctp.prsctp_enable) {
>> >> - if (cid = SCTP_CID_FWD_TSN)
>> >> + if (cid = SCTP_CID_FWD_TSN ||
>> >> + (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
>> >
>> > We don't really need to check intl_enable here, do we?
>> We think net->sctp.xxx_enable as a main switch here,
>> that means it only accepts SCTP_CID_I_FWD_TSN
>> when this switch is open.
>
> Yes, but
>
>>
>> As for the check in validate_ftsn(), it follows the RFC checking
>> on asoc's intl_enable.
>
> Yes.
>
>>
>> does it make sense from this perspective ?
>
> Sort of. We need consistency. If we want do discard such invalid
> chunks right here, that condition also needs to ensure that fwd_tsn
> chunks are actually allowed (IOW, that intl_enable is false).
>
> And we don't do any check for idata chunks. We just just accept them
> and discard later when trying to consume it.
Yeah, seems not a good idea to check intl_enable here, it's not like
other _enable.
intl_enable is more like a 'which' option, while other _enables are
like 'yes or no' option.
I will just remove intl_enable checks, for both ifwdtsn and idata.
Thanks.
>
>>
>>
>>
>> > Because a) it should actually look like:
>> > - if (cid = SCTP_CID_FWD_TSN)
>> > + if ((!net->sctp.intl_enable && cid = SCTP_CID_FWD_TSN) ||
>> > + (net->sctp.intl_enable && cid = SCTP_CID_I_FWD_TSN))
>> > but b) we will validate the chunk format/feature later with
>> > validate_ftsn(), similarly to what happens with data chunks, so it
>> > seems the check on intl_enable here is not necessary.
>> >
>> > Same happens with data chunks, btw.
>> >
>> >> return &prsctp_chunk_event_table[0][state];
>> >> }
>> >>
>> >> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
>> >> index 2ead372..cc4a5e3 100644
>> >> --- a/net/sctp/stream_interleave.c
>> >> +++ b/net/sctp/stream_interleave.c
>> >> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
>> >> }
>> >> }
>> >>
>> >> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
>> >> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
>> >> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
>> >> +
>> >> +#define sctp_walk_ifwdtsn(pos, ch) \
>> >> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
>> >> + sizeof(struct sctp_ifwdtsn_chunk))
>> >> +
>> >> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
>> >> +{
>> >> + struct sctp_fwdtsn_skip *skip;
>> >> + __u16 incnt;
>> >> +
>> >> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
>> >> + return false;
>> >> +
>> >> + incnt = chunk->asoc->stream.incnt;
>> >> + sctp_walk_fwdtsn(skip, chunk)
>> >> + if (ntohs(skip->stream) >= incnt)
>> >> + return false;
>> >> +
>> >> + return true;
>> >> +}
>> >> +
>> >> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
>> >> +{
>> >> + struct sctp_ifwdtsn_skip *skip;
>> >> + __u16 incnt;
>> >> +
>> >> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
>> >> + return false;
>> >> +
>> >> + incnt = chunk->asoc->stream.incnt;
>> >> + sctp_walk_ifwdtsn(skip, chunk)
>> >> + if (ntohs(skip->stream) >= incnt)
>> >> + return false;
>> >> +
>> >> + return true;
>> >> +}
>> >> +
>> >> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> >> .data_chunk_len = sizeof(struct sctp_data_chunk),
>> >> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
>> >> /* DATA process functions */
>> >> .make_datafrag = sctp_make_datafrag_empty,
>> >> .assign_number = sctp_chunk_assign_ssn,
>> >> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> >> .abort_pd = sctp_ulpq_abort_pd,
>> >> /* FORWARD-TSN process functions */
>> >> .generate_ftsn = sctp_generate_fwdtsn,
>> >> + .validate_ftsn = sctp_validate_fwdtsn,
>> >> };
>> >>
>> >> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> >> .data_chunk_len = sizeof(struct sctp_idata_chunk),
>> >> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
>> >> /* I-DATA process functions */
>> >> .make_datafrag = sctp_make_idatafrag_empty,
>> >> .assign_number = sctp_chunk_assign_mid,
>> >> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> >> .abort_pd = sctp_intl_abort_pd,
>> >> /* I-FORWARD-TSN process functions */
>> >> .generate_ftsn = sctp_generate_iftsn,
>> >> + .validate_ftsn = sctp_validate_iftsn,
>> >> };
>> >>
>> >> void sctp_stream_interleave_init(struct sctp_stream *stream)
>> >> --
>> >> 2.1.0
>> >>
>> >> --
>> >> 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
>> >>
>> --
>> 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
>>
^ permalink raw reply [flat|nested] 30+ messages in thread