From 7cd7ecc941e921e02a280cb358001076d4469610 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 28 Nov 2018 19:02:23 -0800 Subject: Add the length of the buffer that is traced --- src/core/lib/iomgr/tcp_posix.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 606bfce6e7..8f3403473e 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -634,8 +634,8 @@ static bool tcp_write_with_timestamps(grpc_tcp* tcp, struct msghdr* msg, if (sending_length == static_cast(length)) { gpr_mu_lock(&tcp->tb_mu); grpc_core::TracedBuffer::AddNewEntry( - &tcp->tb_head, static_cast(tcp->bytes_counter + length), - tcp->outgoing_buffer_arg); + &tcp->tb_head, static_cast(tcp->bytes_counter + length), + static_cast(length), tcp->outgoing_buffer_arg); gpr_mu_unlock(&tcp->tb_mu); tcp->outgoing_buffer_arg = nullptr; } -- cgit v1.2.3 From af16b2c09d75f56acc9fa2c7d76ebb038e06ea3e Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 28 Nov 2018 20:16:27 -0800 Subject: Return immediately if the first message is empty --- src/core/lib/iomgr/tcp_posix.cc | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 606bfce6e7..84d593426e 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -686,11 +686,9 @@ struct cmsghdr* process_timestamp(grpc_tcp* tcp, msghdr* msg, } /** For linux platforms, reads the socket's error queue and processes error - * messages from the queue. Returns true if all the errors processed were - * timestamps. Returns false if any of the errors were not timestamps. For - * non-linux platforms, error processing is not used/enabled currently. + * messages from the queue. */ -static bool process_errors(grpc_tcp* tcp) { +static void process_errors(grpc_tcp* tcp) { while (true) { struct iovec iov; iov.iov_base = nullptr; @@ -719,10 +717,10 @@ static bool process_errors(grpc_tcp* tcp) { } while (r < 0 && saved_errno == EINTR); if (r == -1 && saved_errno == EAGAIN) { - return true; /* No more errors to process */ + return; /* No more errors to process */ } if (r == -1) { - return false; + return; } if (grpc_tcp_trace.enabled()) { if ((msg.msg_flags & MSG_CTRUNC) == 1) { @@ -732,10 +730,14 @@ static bool process_errors(grpc_tcp* tcp) { if (msg.msg_controllen == 0) { /* There was no control message found. It was probably spurious. */ - return true; + return; + } + auto cmsg = CMSG_FIRSTHDR(&msg); + if (cmsg == nullptr || cmsg->cmsg_len == 0) { + /* No control message found. */ + return; } - for (auto cmsg = CMSG_FIRSTHDR(&msg); cmsg && cmsg->cmsg_len; - cmsg = CMSG_NXTHDR(&msg, cmsg)) { + do { if (cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_TIMESTAMPING) { /* Got a control message that is not a timestamp. Don't know how to @@ -745,10 +747,10 @@ static bool process_errors(grpc_tcp* tcp) { "unknown control message cmsg_level:%d cmsg_type:%d", cmsg->cmsg_level, cmsg->cmsg_type); } - return false; + return; } - cmsg = process_timestamp(tcp, &msg, cmsg); - } + cmsg = CMSG_NXTHDR(&msg, process_timestamp(tcp, &msg, cmsg)); + } while (cmsg && cmsg->cmsg_len); } } -- cgit v1.2.3 From fe4ef31ac28f702755c67cb0d79140bc9cbaa552 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 29 Nov 2018 00:56:22 -0800 Subject: Do not add the TCP buffer length. --- src/core/lib/iomgr/buffer_list.cc | 3 +-- src/core/lib/iomgr/buffer_list.h | 2 +- src/core/lib/iomgr/tcp_posix.cc | 2 +- test/core/iomgr/buffer_list_test.cc | 5 ++--- 4 files changed, 5 insertions(+), 7 deletions(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/buffer_list.cc b/src/core/lib/iomgr/buffer_list.cc index b9e38f7dd2..e20dab15b1 100644 --- a/src/core/lib/iomgr/buffer_list.cc +++ b/src/core/lib/iomgr/buffer_list.cc @@ -30,10 +30,9 @@ namespace grpc_core { void TracedBuffer::AddNewEntry(TracedBuffer** head, uint32_t seq_no, - uint32_t length, void* arg) { + void* arg) { GPR_DEBUG_ASSERT(head != nullptr); TracedBuffer* new_elem = New(seq_no, arg); - new_elem->ts_.length = length; /* Store the current time as the sendmsg time. */ new_elem->ts_.sendmsg_time = gpr_now(GPR_CLOCK_REALTIME); if (*head == nullptr) { diff --git a/src/core/lib/iomgr/buffer_list.h b/src/core/lib/iomgr/buffer_list.h index 9e2c1dcb24..9f62d988cc 100644 --- a/src/core/lib/iomgr/buffer_list.h +++ b/src/core/lib/iomgr/buffer_list.h @@ -58,7 +58,7 @@ class TracedBuffer { /** Add a new entry in the TracedBuffer list pointed to by head. Also saves * sendmsg_time with the current timestamp. */ static void AddNewEntry(grpc_core::TracedBuffer** head, uint32_t seq_no, - uint32_t length, void* arg); + void* arg); /** Processes a received timestamp based on sock_extended_err and * scm_timestamping structures. It will invoke the timestamps callback if the diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 8f3403473e..ee339cdfc4 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -635,7 +635,7 @@ static bool tcp_write_with_timestamps(grpc_tcp* tcp, struct msghdr* msg, gpr_mu_lock(&tcp->tb_mu); grpc_core::TracedBuffer::AddNewEntry( &tcp->tb_head, static_cast(tcp->bytes_counter + length), - static_cast(length), tcp->outgoing_buffer_arg); + tcp->outgoing_buffer_arg); gpr_mu_unlock(&tcp->tb_mu); tcp->outgoing_buffer_arg = nullptr; } diff --git a/test/core/iomgr/buffer_list_test.cc b/test/core/iomgr/buffer_list_test.cc index a979fde5e4..e104e8e91a 100644 --- a/test/core/iomgr/buffer_list_test.cc +++ b/test/core/iomgr/buffer_list_test.cc @@ -48,7 +48,7 @@ static void TestShutdownFlushesList() { for (auto i = 0; i < NUM_ELEM; i++) { gpr_atm_rel_store(&verifier_called[i], static_cast(0)); grpc_core::TracedBuffer::AddNewEntry( - &list, i, 0, static_cast(&verifier_called[i])); + &list, i, static_cast(&verifier_called[i])); } grpc_core::TracedBuffer::Shutdown(&list, nullptr, GRPC_ERROR_NONE); GPR_ASSERT(list == nullptr); @@ -66,7 +66,6 @@ static void TestVerifierCalledOnAckVerifier(void* arg, GPR_ASSERT(ts->acked_time.clock_type == GPR_CLOCK_REALTIME); GPR_ASSERT(ts->acked_time.tv_sec == 123); GPR_ASSERT(ts->acked_time.tv_nsec == 456); - GPR_ASSERT(ts->length == 789); gpr_atm* done = reinterpret_cast(arg); gpr_atm_rel_store(done, static_cast(1)); } @@ -85,7 +84,7 @@ static void TestVerifierCalledOnAck() { grpc_core::TracedBuffer* list = nullptr; gpr_atm verifier_called; gpr_atm_rel_store(&verifier_called, static_cast(0)); - grpc_core::TracedBuffer::AddNewEntry(&list, 213, 789, &verifier_called); + grpc_core::TracedBuffer::AddNewEntry(&list, 213, &verifier_called); grpc_core::TracedBuffer::ProcessTimestamp(&list, &serr, &tss); GPR_ASSERT(gpr_atm_acq_load(&verifier_called) == static_cast(1)); GPR_ASSERT(list == nullptr); -- cgit v1.2.3 From ba45e77413b0475036bf5d22adabf8bd432fd443 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 29 Nov 2018 10:00:57 -0800 Subject: Revert the do while and if --- src/core/lib/iomgr/tcp_posix.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 84d593426e..68c61b1201 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -732,12 +732,9 @@ static void process_errors(grpc_tcp* tcp) { /* There was no control message found. It was probably spurious. */ return; } - auto cmsg = CMSG_FIRSTHDR(&msg); - if (cmsg == nullptr || cmsg->cmsg_len == 0) { - /* No control message found. */ - return; - } - do { + bool seen = false; + for (auto cmsg = CMSG_FIRSTHDR(&msg); cmsg && cmsg->cmsg_len; + cmsg = CMSG_NXTHDR(&msg, cmsg)) { if (cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_TIMESTAMPING) { /* Got a control message that is not a timestamp. Don't know how to @@ -749,8 +746,12 @@ static void process_errors(grpc_tcp* tcp) { } return; } - cmsg = CMSG_NXTHDR(&msg, process_timestamp(tcp, &msg, cmsg)); - } while (cmsg && cmsg->cmsg_len); + cmsg = process_timestamp(tcp, &msg, cmsg); + seen = true; + } + if (!seen) { + return; + } } } -- cgit v1.2.3