On Tue, Jan 17, 2017 at 12:27:48PM +0800, Xin Long wrote:
> On Mon, Jan 16, 2017 at 11:56 AM, Xin Long <lucien.xin@xxxxxxxxx> wrote:
> > On Sun, Jan 15, 2017 at 11:51 PM, Marcelo Ricardo Leitner
> > <marcelo.leitner@xxxxxxxxx> wrote:
> >> On Sat, Jan 14, 2017 at 03:15:36AM +0800, Xin Long wrote:
> >>> This patch is to add asoc strreset_outseq and strreset_inseq for
> >>> saving the reconf request sequence, initialize them when create
> >>> assoc and process init, and also to define Incoming and Outgoing
> >>> SSN Reset Request Parameter described in rfc6525 section 4.1 and
> >>> 4.2, As they can be in one same chunk as section rfc6525 3.1-3
> >>> describes, it makes them in one function.
> >>>
> >>> Signed-off-by: Xin Long <lucien.xin@xxxxxxxxx>
> >>> ---
> >>> include/linux/sctp.h | 26 ++++++++++++++
> >>> include/net/sctp/sm.h | 5 ++-
> >>> include/net/sctp/structs.h | 3 ++
> >>> net/sctp/associola.c | 1 +
> >>> net/sctp/sm_make_chunk.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++
> >>> 5 files changed, 122 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/include/linux/sctp.h b/include/linux/sctp.h
> >>> index cdc3b05..d5da19c 100644
> >>> --- a/include/linux/sctp.h
> >>> +++ b/include/linux/sctp.h
> >>> @@ -200,6 +200,13 @@ typedef enum {
> >>> SCTP_PARAM_SUCCESS_REPORT = cpu_to_be16(0xc005),
> >>> SCTP_PARAM_ADAPTATION_LAYER_IND = cpu_to_be16(0xc006),
> >>>
> >>> + /* RE-CONFIG. Section 4 */
> >>> + SCTP_PARAM_RESET_OUT_REQUEST = cpu_to_be16(0x000d),
> >>> + SCTP_PARAM_RESET_IN_REQUEST = cpu_to_be16(0x000e),
> >>> + SCTP_PARAM_RESET_TSN_REQUEST = cpu_to_be16(0x000f),
> >>> + SCTP_PARAM_RESET_RESPONSE = cpu_to_be16(0x0010),
> >>> + SCTP_PARAM_RESET_ADD_OUT_STREAMS = cpu_to_be16(0x0011),
> >>> + SCTP_PARAM_RESET_ADD_IN_STREAMS = cpu_to_be16(0x0012),
> >>> } sctp_param_t; /* enum */
> >>>
> >>>
> >>> @@ -716,4 +723,23 @@ struct sctp_reconf_chunk {
> >>> __u8 params[0];
> >>> } __packed;
> >>>
> >>> +struct sctp_strreset_req {
> >>> + sctp_paramhdr_t param_hdr;
> >>> + __u32 request_seq;
> >>> +} __packed;
> >>> +
> >>> +struct sctp_strreset_outreq {
> >>> + sctp_paramhdr_t param_hdr;
> >>> + __u32 request_seq;
> >>
> >> This should be:
> >> + struct sctp_strreset_req strreset_req;
> >> Use the definition you created above for the encapsulation and make the
> >> embedding evident.
> >> Like it's done for sctp_chunkhdr_t.
> > I'm not sure if it's good to do it like sctp_chunkhdr_t.
> >
> > As sctp_chunkhdr is a very common data:
> > Chunk Type | Chunk Flags | Chunk Length
> > and the next must be "Chunk Value"
> >
> > But here sctp_strreset_req is more used to access
> > the request_seq of the params in asoc->strreset_chunk
> > without knowing params' type. like in sctp_chunk_lookup_strreset_param()
Exactly.
> > and sctp_process_strreset_resp() [see it from the big patchset].
> >
> > struct sctp_strreset_outreq {
> > sctp_paramhdr_t param_hdr;
> > __u32 request_seq;
> > ------------------------------------[1]
> > __u32 response_seq;
> > __u32 send_reset_at_tsn;
> > __u16 list_of_streams[0];
> > } __packed;
> >
> > it seems not good to split it by [1].
> > __u32 request_seq;
> > __u32 response_seq;
> > __u32 send_reset_at_tsn;
> > __u16 list_of_streams[0];
> > these should to together and equal.
> >
> > what do you think ?
> >
> Hi, Marcelo, I'm planning to drop "struct sctp_strreset_req" so
> that it will not be confusing here, and only introduce it when
> it's used somewhere. agreed ?
Works for me. Then we will check about this part:
from sctp_process_strreset_resp:
+ if (req->param_hdr.type == SCTP_PARAM_RESET_OUT_REQUEST) {
+ struct sctp_strreset_outreq *outreq;
+ __u16 *str_p = NULL;
+
+ outreq = (struct sctp_strreset_outreq *)req;
If the structs could be embedded, such cast would be clearer to be safe.
Marcelo
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
