Skip to content

Commit

Permalink
Merge pull request Mbed-TLS#318 from hanno-arm/mps_simplify
Browse files Browse the repository at this point in the history
MPS: Numerous minor simplifications and improvements
  • Loading branch information
Hanno Becker authored Jul 27, 2021
2 parents 0e367ad + 3430517 commit 2556d8a
Show file tree
Hide file tree
Showing 7 changed files with 487 additions and 655 deletions.
2 changes: 1 addition & 1 deletion include/mbedtls/mps/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@
#define MBEDTLS_MPS_TRANSFORM_VALIDATION

/*! This flag controls whether tracing for MPS should be enabled. */
//#define MBEDTLS_MPS_TRACE
//#define MBEDTLS_MPS_ENABLE_TRACE

/*! This internal macro determines whether all Layers of MPS should
* be compiled into a single source file.
Expand Down
34 changes: 12 additions & 22 deletions include/mbedtls/mps/mps.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,32 +74,22 @@ typedef struct mbedtls_mps_app_out mbedtls_mps_app_out;
#define MBEDTLS_MPS_RETRANSMISSION_CALLBACK_PAUSE 1

/*! The type of reassembly/buffering states handshake messages.
*
* Possible values are:
* - #MPS_REASSEMBLY_NONE
* Reassembly hasn't started.
* - #MPS_REASSEMBLY_NO_FRAGMENTATION
* The message has been received in a single fragment
* and no reassembly was necessary; a reader is available
* which gives access to the contents.
* The message has been received in a single fragment and no reassembly was
* necessary; a reader is available which gives access to the contents.
* - #MPS_REASSEMBLY_WINDOW
* Some fragments of the message have been received
* and reassembly is in progress.
*
* The state #MPS_REASSEMBLY_NO_FRAGMENTATION is only
* possible for the next message, as for future messages
* we need to make a copy of the Layer 3 data anyway.
*
* NOTE: There are more alternatives, for example
* one could always wait until a new fragment
* comes in which continues the initial part
* of the message that has already been received;
* this way, no additional buffers would be needed
* (if the parsing routines make use of pausing).
* However, this seems to be suitable only for very
* reliable networks, or in DTLS-1.3 where a more
* elaborate acknowledgement scheme is available.
*
* Some fragments have been received and reassembly is in progress.
* The state #MPS_REASSEMBLY_NO_FRAGMENTATION is only possible for the next
* message, as for future messages we need to make a copy of the L3 data anyway.
*
* NOTE: There are more alternatives, for example one could always wait until a
* new fragment comes in which continues the initial part of the message
* that has already been received; this way, no additional buffers would
* be needed (if the parsing routines make use of pausing). However, this
* seems to be suitable only for very reliable networks, or in DTLS-1.3
* where a more elaborate acknowledgement scheme is available.
*/
typedef uint8_t mbedtls_mps_msg_reassembly_state;
#define MBEDTLS_MPS_REASSEMBLY_NONE \
Expand Down
5 changes: 5 additions & 0 deletions include/mbedtls/mps/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,9 @@ void mbedtls_mps_trace_print_msg( int id, int line, const char *format, ... );

#endif /* MBEDTLS_MPS_TRACE */

#define MBEDTLS_MPS_TRACE_COMMENT(...) \
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, __VA_ARGS__ )
#define MBEDTLS_MPS_TRACE_ERROR(...) \
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_ERROR, __VA_ARGS__ )

#endif /* MBEDTLS_MPS_TRACE_H */
42 changes: 21 additions & 21 deletions library/mps/layer1.c
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,13 @@ int l1_fetch_stream( mps_l1_stream_read *p,
read_ptr += br;
while( data_need > 0 )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "attempt to receive %u", (unsigned) data_need );
MBEDTLS_MPS_TRACE_COMMENT( "attempt to receive %u", (unsigned) data_need );
ret = recv( recv_ctx, read_ptr, data_need );
if( ret == 0 )
ret = MBEDTLS_ERR_MPS_CONN_EOF;
if( ret < 0 )
break;
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "got %u", (unsigned) ret );
MBEDTLS_MPS_TRACE_COMMENT( "got %u", (unsigned) ret );

/* TODO: FIX! */
#if( MAX_INT > SIZE_MAX )
Expand Down Expand Up @@ -397,7 +397,7 @@ int l1_flush_stream( mps_l1_stream_write *p )
ret = send( send_ctx, buf, data_remaining );
if( ret <= 0 )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "send failed with %d", ret );
MBEDTLS_MPS_TRACE_COMMENT( "send failed with %d", ret );
/* The underlying transport's send callback should return
* WANT_WRITE instead of 0 if no data can currently be sent.
* Fail with a fatal internal error if this spec is not obeyed. */
Expand Down Expand Up @@ -544,12 +544,12 @@ int l1_check_flush_stream( mps_l1_stream_write *p )
*/
if( br > 0 && br >= 4 * bl / 5 )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "L1 check flush -- flush" );
MBEDTLS_MPS_TRACE_COMMENT( "L1 check flush -- flush" );
p->status = MPS_L1_STREAM_STATUS_FLUSH;
MBEDTLS_MPS_TRACE_RETURN( 0 );
}

MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "L1 check flush -- no flush" );
MBEDTLS_MPS_TRACE_COMMENT( "L1 check flush -- no flush" );
MBEDTLS_MPS_TRACE_RETURN( 0 );
}

Expand All @@ -574,7 +574,7 @@ int l1_dispatch_stream( mps_l1_stream_write *p,
data_remaining = br - bl;
if( len > data_remaining )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "out of bounds %u > %u",
MBEDTLS_MPS_TRACE_COMMENT( "out of bounds %u > %u",
(unsigned) len, (unsigned) data_remaining );
MBEDTLS_MPS_TRACE_RETURN( MBEDTLS_ERR_MPS_REQUEST_OUT_OF_BOUNDS );
}
Expand Down Expand Up @@ -763,7 +763,7 @@ int l1_ensure_in_dgram( mps_l1_dgram_read *p )
ml = p->msg_len;
if( ml == 0 )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "Request datagram from underlying transport." );
MBEDTLS_MPS_TRACE_COMMENT( "Request datagram from underlying transport." );
/* Q: Will the underlying transport error out
* if the receive buffer is not large enough
* to hold the entire datagram? */
Expand All @@ -786,7 +786,7 @@ int l1_ensure_in_dgram( mps_l1_dgram_read *p )
if( ml > bl )
MBEDTLS_MPS_TRACE_RETURN( MBEDTLS_ERR_MPS_BAD_TRANSPORT );

MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "Obtained datagram of size %u", (unsigned) ml );
MBEDTLS_MPS_TRACE_COMMENT( "Obtained datagram of size %u", (unsigned) ml );
p->msg_len = ml;
}

Expand All @@ -811,9 +811,9 @@ int l1_fetch_dgram( mps_l1_dgram_read *p,
if( ret != 0 )
MBEDTLS_MPS_TRACE_RETURN( ret );

MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "* Datagram length: %u", (unsigned) p->msg_len );
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "* Window base: %u", (unsigned) p->window_base );
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "* Window length: %u", (unsigned) p->window_len );
MBEDTLS_MPS_TRACE_COMMENT( "* Datagram length: %u", (unsigned) p->msg_len );
MBEDTLS_MPS_TRACE_COMMENT( "* Window base: %u", (unsigned) p->window_base );
MBEDTLS_MPS_TRACE_COMMENT( "* Window length: %u", (unsigned) p->window_len );

wb = p->window_base;
wl = p->window_len;
Expand All @@ -832,7 +832,7 @@ int l1_fetch_dgram( mps_l1_dgram_read *p,
data_avail = ml - ( wb + wl );
if( data_need > data_avail )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_ERROR, "Read request goes beyond the datagram boundary - requested %u, available %u",
MBEDTLS_MPS_TRACE_ERROR( "Read request goes beyond the datagram boundary - requested %u, available %u",
(unsigned) data_need, (unsigned) data_avail );
MBEDTLS_MPS_TRACE_RETURN( MBEDTLS_ERR_MPS_REQUEST_OUT_OF_BOUNDS );
}
Expand Down Expand Up @@ -862,7 +862,7 @@ int l1_consume_dgram( mps_l1_dgram_read *p )

if( wb + wl == ml )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "Reached the end of the datagram." );
MBEDTLS_MPS_TRACE_COMMENT( "Reached the end of the datagram." );

/*
* Releasing the buffer as soon as a datagram
Expand All @@ -888,7 +888,7 @@ int l1_consume_dgram( mps_l1_dgram_read *p )
}
else
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "More data left in the current datagram." );
MBEDTLS_MPS_TRACE_COMMENT( "More data left in the current datagram." );
p->window_base = wb + wl;
p->window_len = 0;
}
Expand Down Expand Up @@ -932,11 +932,11 @@ int l1_write_dgram( mps_l1_dgram_write *p,
flush = p->flush;
if( flush )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "Need to flush first" );
MBEDTLS_MPS_TRACE_COMMENT( "Need to flush first" );
ret = l1_flush_dgram( p );
if( ret != 0 )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_ERROR, "Flush failed with %d", ret );
MBEDTLS_MPS_TRACE_ERROR( "Flush failed with %d", ret );
MBEDTLS_MPS_TRACE_RETURN( ret );
}
}
Expand Down Expand Up @@ -994,12 +994,12 @@ int l1_check_flush_dgram( mps_l1_dgram_write *p )
* data is available. */
if( br > 0 && br >= 4 * bl / 5 )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "L1 check flush -- flush" );
MBEDTLS_MPS_TRACE_COMMENT( "L1 check flush -- flush" );
p->flush = 1;
MBEDTLS_MPS_TRACE_RETURN( 0 );
}

MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "L1 check flush -- no flush" );
MBEDTLS_MPS_TRACE_COMMENT( "L1 check flush -- no flush" );
MBEDTLS_MPS_TRACE_RETURN( 0 );
}

Expand All @@ -1017,11 +1017,11 @@ int l1_flush_dgram( mps_l1_dgram_write *p )
buf = p->buf;
if( buf == NULL )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_ERROR, "No outgoing datagram open." );
MBEDTLS_MPS_TRACE_ERROR( "No outgoing datagram open." );
MBEDTLS_MPS_TRACE_RETURN( 0 );
}

MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "Datagram size: %u", (unsigned) p->bytes_ready );
MBEDTLS_MPS_TRACE_COMMENT( "Datagram size: %u", (unsigned) p->bytes_ready );

br = p->bytes_ready;

Expand All @@ -1030,7 +1030,7 @@ int l1_flush_dgram( mps_l1_dgram_write *p )
ret = send( send_ctx, buf, br );
if( ret <= 0 )
{
MBEDTLS_MPS_TRACE( MBEDTLS_MPS_TRACE_TYPE_COMMENT, "send failed with %d", ret );
MBEDTLS_MPS_TRACE_COMMENT( "send failed with %d", ret );
/* The underlying transport's send callback should return
* WANT_WRITE instead of 0 if no data can currently be sent.
* Fail with a fatal internal error if this spec is not obeyed. */
Expand Down
Loading

0 comments on commit 2556d8a

Please sign in to comment.