Code/bug after testing

1. Fixing astyle and docs
2. Extra mutex lock was removed
3. Bytes are updated when send/recv > 0 and not in case of -ve error
4. Review comments
5. Guard statistics implementation in test with MBED_NW_STATS_ENABLED
pull/8592/head
Deepika 2018-11-16 12:02:19 -06:00 committed by deepikabhavnani
parent eec54a0fc9
commit a851df66e3
13 changed files with 54 additions and 47 deletions

View File

@ -119,14 +119,12 @@ int split2half_rmng_tcp_test_time()
return (tcp_global::TESTS_TIMEOUT - tc_bucket.read()) / 2; return (tcp_global::TESTS_TIMEOUT - tc_bucket.read()) / 2;
} }
#if defined(MBED_NW_STATS_ENABLED)
int fetch_stats() int fetch_stats()
{ {
#if defined(MBED_NW_STATS_ENABLED)
return SocketStats::mbed_stats_socket_get_each(&tcp_stats[0], MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT); return SocketStats::mbed_stats_socket_get_each(&tcp_stats[0], MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT);
#else
return 0;
#endif
} }
#endif
// Test setup // Test setup
utest::v1::status_t greentea_setup(const size_t number_of_cases) utest::v1::status_t greentea_setup(const size_t number_of_cases)

View File

@ -23,10 +23,10 @@ void drop_bad_packets(TCPSocket &sock, int orig_timeout);
void fill_tx_buffer_ascii(char *buff, size_t len); void fill_tx_buffer_ascii(char *buff, size_t len);
nsapi_error_t tcpsocket_connect_to_echo_srv(TCPSocket &sock); nsapi_error_t tcpsocket_connect_to_echo_srv(TCPSocket &sock);
nsapi_error_t tcpsocket_connect_to_discard_srv(TCPSocket &sock); nsapi_error_t tcpsocket_connect_to_discard_srv(TCPSocket &sock);
int fetch_stats(void);
#if defined(MBED_NW_STATS_ENABLED) #if defined(MBED_NW_STATS_ENABLED)
extern mbed_stats_socket_t tcp_stats[MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT]; extern mbed_stats_socket_t tcp_stats[MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT];
int fetch_stats(void);
#endif #endif
/** /**

View File

@ -114,12 +114,13 @@ void tcpsocket_echotest_nonblock_receiver(void *receive_bytes)
void TCPSOCKET_ECHOTEST_NONBLOCK() void TCPSOCKET_ECHOTEST_NONBLOCK()
{ {
#if defined(MBED_NW_STATS_ENABLED)
int j = 0; int j = 0;
int count = fetch_stats(); int count = fetch_stats();
for (; j < count; j++) { for (; j < count; j++) {
TEST_ASSERT_EQUAL(SOCK_CLOSED, tcp_stats[j].state); TEST_ASSERT_EQUAL(SOCK_CLOSED, tcp_stats[j].state);
} }
#endif
tc_exec_time.start(); tc_exec_time.start();
time_allotted = split2half_rmng_tcp_test_time(); // [s] time_allotted = split2half_rmng_tcp_test_time(); // [s]
@ -166,6 +167,7 @@ void TCPSOCKET_ECHOTEST_NONBLOCK()
bytes2send -= sent; bytes2send -= sent;
} }
printf("[Sender#%02d] bytes sent: %d\n", s_idx, pkt_s); printf("[Sender#%02d] bytes sent: %d\n", s_idx, pkt_s);
#if defined(MBED_NW_STATS_ENABLED)
count = fetch_stats(); count = fetch_stats();
for (j = 0; j < count; j++) { for (j = 0; j < count; j++) {
if ((tcp_stats[j].state == SOCK_OPEN) && (tcp_stats[j].proto == NSAPI_TCP)) { if ((tcp_stats[j].state == SOCK_OPEN) && (tcp_stats[j].proto == NSAPI_TCP)) {
@ -173,7 +175,7 @@ void TCPSOCKET_ECHOTEST_NONBLOCK()
} }
} }
TEST_ASSERT_EQUAL(bytes2send, tcp_stats[j].sent_bytes); TEST_ASSERT_EQUAL(bytes2send, tcp_stats[j].sent_bytes);
#endif
tx_sem.wait(split2half_rmng_tcp_test_time()); tx_sem.wait(split2half_rmng_tcp_test_time());
thread->join(); thread->join();
delete thread; delete thread;

View File

@ -26,11 +26,12 @@ using namespace utest::v1;
void TCPSOCKET_OPEN_CLOSE_REPEAT() void TCPSOCKET_OPEN_CLOSE_REPEAT()
{ {
#if defined(MBED_NW_STATS_ENABLED)
int count = fetch_stats(); int count = fetch_stats();
for (int j = 0; j < count; j++) { for (int j = 0; j < count; j++) {
TEST_ASSERT_EQUAL(SOCK_CLOSED, tcp_stats[j].state); TEST_ASSERT_EQUAL(SOCK_CLOSED, tcp_stats[j].state);
} }
#endif
TCPSocket *sock = new TCPSocket; TCPSocket *sock = new TCPSocket;
if (!sock) { if (!sock) {
TEST_FAIL(); TEST_FAIL();
@ -41,9 +42,10 @@ void TCPSOCKET_OPEN_CLOSE_REPEAT()
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->close()); TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->close());
} }
delete sock; delete sock;
#if defined(MBED_NW_STATS_ENABLED)
count = fetch_stats(); count = fetch_stats();
for (int j = 0; j < count; j++) { for (int j = 0; j < count; j++) {
TEST_ASSERT_EQUAL(SOCK_CLOSED, tcp_stats[j].state); TEST_ASSERT_EQUAL(SOCK_CLOSED, tcp_stats[j].state);
} }
#endif
} }

View File

@ -70,6 +70,7 @@ void TCPSOCKET_OPEN_LIMIT()
break; break;
} }
#if defined(MBED_NW_STATS_ENABLED)
int count = fetch_stats(); int count = fetch_stats();
int open_count = 0; int open_count = 0;
for (int j = 0; j < count; j++) { for (int j = 0; j < count; j++) {
@ -78,6 +79,7 @@ void TCPSOCKET_OPEN_LIMIT()
} }
} }
TEST_ASSERT(open_count >= 4); TEST_ASSERT(open_count >= 4);
#endif
TCPSocketItem *tmp; TCPSocketItem *tmp;
for (TCPSocketItem *it = socket_list_head; it;) { for (TCPSocketItem *it = socket_list_head; it;) {

View File

@ -80,14 +80,12 @@ void fill_tx_buffer_ascii(char *buff, size_t len)
} }
} }
#if defined(MBED_NW_STATS_ENABLED)
int fetch_stats() int fetch_stats()
{ {
#if defined(MBED_NW_STATS_ENABLED)
return SocketStats::mbed_stats_socket_get_each(&udp_stats[0], MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT); return SocketStats::mbed_stats_socket_get_each(&udp_stats[0], MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT);
#else
return 0;
#endif
} }
#endif
// Test setup // Test setup
utest::v1::status_t greentea_setup(const size_t number_of_cases) utest::v1::status_t greentea_setup(const size_t number_of_cases)

View File

@ -21,10 +21,10 @@
NetworkInterface *get_interface(); NetworkInterface *get_interface();
void drop_bad_packets(UDPSocket &sock, int orig_timeout); void drop_bad_packets(UDPSocket &sock, int orig_timeout);
void fill_tx_buffer_ascii(char *buff, size_t len); void fill_tx_buffer_ascii(char *buff, size_t len);
int fetch_stats(void);
#if defined(MBED_NW_STATS_ENABLED) #if defined(MBED_NW_STATS_ENABLED)
extern mbed_stats_socket_t udp_stats[MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT]; extern mbed_stats_socket_t udp_stats[MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT];
int fetch_stats(void);
#endif #endif
/* /*

View File

@ -26,11 +26,12 @@ using namespace utest::v1;
void UDPSOCKET_OPEN_CLOSE_REPEAT() void UDPSOCKET_OPEN_CLOSE_REPEAT()
{ {
#if defined(MBED_NW_STATS_ENABLED)
int count = fetch_stats(); int count = fetch_stats();
for (int j = 0; j < count; j++) { for (int j = 0; j < count; j++) {
TEST_ASSERT_EQUAL(SOCK_CLOSED, udp_stats[j].state); TEST_ASSERT_EQUAL(SOCK_CLOSED, udp_stats[j].state);
} }
#endif
UDPSocket *sock = new UDPSocket; UDPSocket *sock = new UDPSocket;
if (!sock) { if (!sock) {
TEST_FAIL(); TEST_FAIL();
@ -41,9 +42,10 @@ void UDPSOCKET_OPEN_CLOSE_REPEAT()
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->close()); TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->close());
} }
delete sock; delete sock;
#if defined(MBED_NW_STATS_ENABLED)
count = fetch_stats(); count = fetch_stats();
for (int j = 0; j < count; j++) { for (int j = 0; j < count; j++) {
TEST_ASSERT_EQUAL(SOCK_CLOSED, udp_stats[j].state); TEST_ASSERT_EQUAL(SOCK_CLOSED, udp_stats[j].state);
} }
#endif
} }

View File

@ -70,7 +70,7 @@ void UDPSOCKET_OPEN_LIMIT()
if (!socket_list_head) { if (!socket_list_head) {
break; break;
} }
#if defined(MBED_NW_STATS_ENABLED)
int count = fetch_stats(); int count = fetch_stats();
int open_count = 0; int open_count = 0;
for (int j = 0; j < count; j++) { for (int j = 0; j < count; j++) {
@ -79,7 +79,7 @@ void UDPSOCKET_OPEN_LIMIT()
} }
} }
TEST_ASSERT(open_count >= 3); TEST_ASSERT(open_count >= 3);
#endif
UDPSocketItem *tmp; UDPSocketItem *tmp;
for (UDPSocketItem *it = socket_list_head; it;) { for (UDPSocketItem *it = socket_list_head; it;) {
++open_sockets[i]; ++open_sockets[i];

View File

@ -31,7 +31,6 @@ uint32_t SocketStats::_size = 0;
int SocketStats::get_entry_position(const Socket *const reference_id) int SocketStats::get_entry_position(const Socket *const reference_id)
{ {
_mutex->lock();
for (uint32_t j = 0; j < _size; j++) { for (uint32_t j = 0; j < _size; j++) {
if (_stats[j].reference_id == reference_id) { if (_stats[j].reference_id == reference_id) {
return j; return j;
@ -70,7 +69,7 @@ void SocketStats::stats_new_socket_entry(const Socket *const reference_id)
if (get_entry_position(reference_id) >= 0) { if (get_entry_position(reference_id) >= 0) {
// Duplicate entry // Duplicate entry
MBED_WARNING1(MBED_MAKE_ERROR(MBED_MODULE_NETWORK_STATS, MBED_ERROR_CODE_INVALID_INDEX), "Duplicate socket Reference ID ", reference_id); MBED_WARNING1(MBED_MAKE_ERROR(MBED_MODULE_NETWORK_STATS, MBED_ERROR_CODE_INVALID_INDEX), "Duplicate socket Reference ID ", reference_id);
} else if (_size <= MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT) { } else if (_size < MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT) {
// Add new entry // Add new entry
_stats[_size].reference_id = (Socket *)reference_id; _stats[_size].reference_id = (Socket *)reference_id;
_size++; _size++;
@ -87,7 +86,7 @@ void SocketStats::stats_new_socket_entry(const Socket *const reference_id)
} }
} }
if (-1 == position) { if (-1 == position) {
MBED_ERROR(MBED_MAKE_ERROR(MBED_MODULE_NETWORK_STATS, MBED_ERROR_CODE_OUT_OF_RESOURCES), "List full with all open sockets"); MBED_ERROR(MBED_MAKE_ERROR(MBED_MODULE_NETWORK_STATS, MBED_ERROR_CODE_OUT_OF_RESOURCES), "List full with all open sockets");
} }
memset(&_stats[position], 0, sizeof(mbed_stats_socket_t)); memset(&_stats[position], 0, sizeof(mbed_stats_socket_t));
_stats[position].reference_id = (Socket *)reference_id; _stats[position].reference_id = (Socket *)reference_id;
@ -117,10 +116,8 @@ void SocketStats::stats_update_peer(const Socket *const reference_id, const Sock
#if defined(MBED_NW_STATS_ENABLED) #if defined(MBED_NW_STATS_ENABLED)
_mutex->lock(); _mutex->lock();
int position = get_entry_position(reference_id); int position = get_entry_position(reference_id);
if (position >= 0) { if ((position >= 0) && (!_stats[position].peer)) {
if (!_stats[position].peer) { _stats[position].peer = peer;
_stats[position].peer = peer;
}
} }
_mutex->unlock(); _mutex->unlock();
#endif #endif
@ -143,7 +140,7 @@ void SocketStats::stats_update_sent_bytes(const Socket *const reference_id, size
#if defined(MBED_NW_STATS_ENABLED) #if defined(MBED_NW_STATS_ENABLED)
_mutex->lock(); _mutex->lock();
int position = get_entry_position(reference_id); int position = get_entry_position(reference_id);
if (position >= 0) { if ((position >= 0) && (sent_bytes > 0)) {
_stats[position].sent_bytes += sent_bytes; _stats[position].sent_bytes += sent_bytes;
} }
_mutex->unlock(); _mutex->unlock();
@ -155,7 +152,7 @@ void SocketStats::stats_update_recv_bytes(const Socket *const reference_id, size
#if defined(MBED_NW_STATS_ENABLED) #if defined(MBED_NW_STATS_ENABLED)
_mutex->lock(); _mutex->lock();
int position = get_entry_position(reference_id); int position = get_entry_position(reference_id);
if (position >= 0) { if ((position >= 0) && (recv_bytes > 0)) {
_stats[position].recv_bytes += recv_bytes; _stats[position].recv_bytes += recv_bytes;
} }
_mutex->unlock(); _mutex->unlock();

View File

@ -33,7 +33,7 @@ typedef enum {
SOCK_OPEN, /**< Socket is open, but not associated to any peer address */ SOCK_OPEN, /**< Socket is open, but not associated to any peer address */
SOCK_CONNECTED, /**< Socket is associated to peer address, either by connect() or sendto()/recvfrom() calls */ SOCK_CONNECTED, /**< Socket is associated to peer address, either by connect() or sendto()/recvfrom() calls */
SOCK_LISTEN, /**< Socket is listening for incoming connections */ SOCK_LISTEN, /**< Socket is listening for incoming connections */
}socket_state; } socket_state;
typedef struct { typedef struct {
Socket *reference_id; /**< Used for identifying socket */ Socket *reference_id; /**< Used for identifying socket */
@ -47,11 +47,18 @@ typedef struct {
/** SocketStats class /** SocketStats class
* *
* * Class to get the network socket statistics
*/ */
class SocketStats class SocketStats {
{
public: public:
/** Create an socket statictics object
*
* Application users must not create class objects.
* The class object will be created by entities reporting network statistics.
* Application can fetch network statistics using static `mbed_stats_socket_get_each` API
* without creating an object.
*/
SocketStats(); SocketStats();
virtual ~SocketStats() virtual ~SocketStats()
{ {
@ -72,8 +79,8 @@ public:
/** Add entry of newly created socket in statistics array. /** Add entry of newly created socket in statistics array.
@Note: Entry in the array will be maintained even after socket is closed. @Note: Entry in the array will be maintained even after socket is closed.
It will be over-written for closed sockets when socket entries in Entry will be over-written for sockets which were closed first, in case
`MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT` exceed. we socket creation count exceeds `MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT`.
*/ */
void stats_new_socket_entry(const Socket *const reference_id); void stats_new_socket_entry(const Socket *const reference_id);
@ -99,7 +106,7 @@ private:
static uint32_t _size; static uint32_t _size;
/** Internal function to scan the array and get position of element in the list. /** Internal function to scan the array and get position of element in the list.
This API locks the mutex and next API updating the entry in array This API locks the mutex and the next API call updating the entry in the array
should release the lock */ should release the lock */
int get_entry_position(const Socket *const reference_id); int get_entry_position(const Socket *const reference_id);
#endif #endif

View File

@ -106,7 +106,7 @@ nsapi_error_t TCPSocket::connect(const SocketAddress &address)
if (ret == NSAPI_ERROR_OK || ret == NSAPI_ERROR_IN_PROGRESS) { if (ret == NSAPI_ERROR_OK || ret == NSAPI_ERROR_IN_PROGRESS) {
_remote_peer = address; _remote_peer = address;
_socket_stats.stats_update_peer(this, _remote_peer); _socket_stats.stats_update_peer(this, _remote_peer);
} }
_lock.unlock(); _lock.unlock();

View File

@ -146,13 +146,12 @@ nsapi_size_or_error_t UDPSocket::recvfrom(SocketAddress *address, void *buffer,
if (recv >= 0 && _remote_peer && _remote_peer != *address) { if (recv >= 0 && _remote_peer && _remote_peer != *address) {
continue; continue;
} }
if (recv > 0) {
_socket_stats.stats_update_recv_bytes(this, recv);
}
_socket_stats.stats_update_peer(this, _remote_peer); _socket_stats.stats_update_peer(this, _remote_peer);
// Non-blocking sockets always return. Blocking only returns when success or errors other than WOULD_BLOCK // Non-blocking sockets always return. Blocking only returns when success or errors other than WOULD_BLOCK
if ((0 == _timeout) || (NSAPI_ERROR_WOULD_BLOCK != recv)) { if ((0 == _timeout) || (NSAPI_ERROR_WOULD_BLOCK != recv)) {
ret = recv; ret = recv;
_socket_stats.stats_update_recv_bytes(this, recv);
break; break;
} else { } else {
uint32_t flag; uint32_t flag;