[PATCH mptcp-next v2 5/6] selftests: mptcp: explicitly tests aggregate counters

Paolo Abeni posted 6 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH mptcp-next v2 5/6] selftests: mptcp: explicitly tests aggregate counters
Posted by Paolo Abeni 2 years, 7 months ago
Update the existing sockopt test-case to do some some
basic checks on the newly added counters.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 .../selftests/net/mptcp/mptcp_sockopt.c       | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index ae61f39556ca..869c041c34f3 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -51,6 +51,11 @@ struct mptcp_info {
 	__u8	mptcpi_local_addr_used;
 	__u8	mptcpi_local_addr_max;
 	__u8	mptcpi_csum_enabled;
+	__u32	mptcpi_retransmits;
+	__u64	mptcpi_bytes_retrans;
+	__u64	mptcpi_bytes_sent;
+	__u64	mptcpi_bytes_received;
+	__u64	mptcpi_bytes_acked;
 };
 
 struct mptcp_subflow_data {
@@ -83,8 +88,10 @@ struct mptcp_subflow_addrs {
 
 struct so_state {
 	struct mptcp_info mi;
+	struct mptcp_info last_sample;
 	uint64_t mptcpi_rcv_delta;
 	uint64_t tcpi_rcv_delta;
+	bool pkt_stats_avail;
 };
 
 static void die_perror(const char *msg)
@@ -320,6 +327,9 @@ static void do_getsockopt_mptcp_info(struct so_state *s, int fd, size_t w)
 
 	assert(olen == sizeof(i));
 
+	s->pkt_stats_avail = olen == sizeof(i);
+
+	s->last_sample = i;
 	if (s->mi.mptcpi_write_seq == 0)
 		s->mi = i;
 
@@ -556,6 +566,23 @@ static void process_one_client(int fd, int pipefd)
 	do_getsockopts(&s, fd, ret, ret2);
 	if (s.mptcpi_rcv_delta != (uint64_t)ret + 1)
 		xerror("mptcpi_rcv_delta %" PRIu64 ", expect %" PRIu64, s.mptcpi_rcv_delta, ret + 1, s.mptcpi_rcv_delta - ret);
+
+	/* be nice when running on top of older kernel */
+	if (s.pkt_stats_avail) {
+		if (s.last_sample.mptcpi_bytes_sent != ret2)
+			xerror("mptcpi_bytes_sent %" PRIu64 ", expect %" PRIu64,
+			       s.last_sample.mptcpi_bytes_sent, ret2,
+			       s.last_sample.mptcpi_bytes_sent - ret2);
+		if (s.last_sample.mptcpi_bytes_received != ret)
+			xerror("mptcpi_bytes_received %" PRIu64 ", expect %" PRIu64,
+			       s.last_sample.mptcpi_bytes_received, ret,
+			       s.last_sample.mptcpi_bytes_received - ret);
+		if (s.last_sample.mptcpi_bytes_acked != ret)
+			xerror("mptcpi_bytes_acked %" PRIu64 ", expect %" PRIu64,
+			       s.last_sample.mptcpi_bytes_acked, ret2,
+			       s.last_sample.mptcpi_bytes_acked - ret2);
+	}
+
 	close(fd);
 }
 
-- 
2.40.1
Re: [PATCH mptcp-next v2 5/6] selftests: mptcp: explicitly tests aggregate counters
Posted by Matthieu Baerts 2 years, 7 months ago

On 22/05/2023 15:56, Paolo Abeni wrote:
> Update the existing sockopt test-case to do some some
> basic checks on the newly added counters.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  .../selftests/net/mptcp/mptcp_sockopt.c       | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> index ae61f39556ca..869c041c34f3 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> @@ -51,6 +51,11 @@ struct mptcp_info {
>  	__u8	mptcpi_local_addr_used;
>  	__u8	mptcpi_local_addr_max;
>  	__u8	mptcpi_csum_enabled;
> +	__u32	mptcpi_retransmits;
> +	__u64	mptcpi_bytes_retrans;
> +	__u64	mptcpi_bytes_sent;
> +	__u64	mptcpi_bytes_received;
> +	__u64	mptcpi_bytes_acked;
>  };
>  
>  struct mptcp_subflow_data {
> @@ -83,8 +88,10 @@ struct mptcp_subflow_addrs {
>  
>  struct so_state {
>  	struct mptcp_info mi;
> +	struct mptcp_info last_sample;
>  	uint64_t mptcpi_rcv_delta;
>  	uint64_t tcpi_rcv_delta;
> +	bool pkt_stats_avail;
>  };
>  
>  static void die_perror(const char *msg)
> @@ -320,6 +327,9 @@ static void do_getsockopt_mptcp_info(struct so_state *s, int fd, size_t w)
>  
>  	assert(olen == sizeof(i));

(see below)

> +	s->pkt_stats_avail = olen == sizeof(i);

Good idea, it is simple! But then we should remove the assert just above.

And maybe we should have here: olen >= sizeof(i), no?
Maybe we don't need to support newer kernels but it is just in case we
add more info later even if I guess we would modify mptcp_info if we does.

Also, maybe "struct mptcp_info" will be defined somewhere else and we
will not use the one defined here above?

> +
> +	s->last_sample = i;
>  	if (s->mi.mptcpi_write_seq == 0)
>  		s->mi = i;
>  
> @@ -556,6 +566,23 @@ static void process_one_client(int fd, int pipefd)
>  	do_getsockopts(&s, fd, ret, ret2);
>  	if (s.mptcpi_rcv_delta != (uint64_t)ret + 1)
>  		xerror("mptcpi_rcv_delta %" PRIu64 ", expect %" PRIu64, s.mptcpi_rcv_delta, ret + 1, s.mptcpi_rcv_delta - ret);
> +
> +	/* be nice when running on top of older kernel */
> +	if (s.pkt_stats_avail) {
> +		if (s.last_sample.mptcpi_bytes_sent != ret2)
> +			xerror("mptcpi_bytes_sent %" PRIu64 ", expect %" PRIu64,
> +			       s.last_sample.mptcpi_bytes_sent, ret2,
> +			       s.last_sample.mptcpi_bytes_sent - ret2);
> +		if (s.last_sample.mptcpi_bytes_received != ret)
> +			xerror("mptcpi_bytes_received %" PRIu64 ", expect %" PRIu64,
> +			       s.last_sample.mptcpi_bytes_received, ret,
> +			       s.last_sample.mptcpi_bytes_received - ret);
> +		if (s.last_sample.mptcpi_bytes_acked != ret)
> +			xerror("mptcpi_bytes_acked %" PRIu64 ", expect %" PRIu64,
> +			       s.last_sample.mptcpi_bytes_acked, ret2,
> +			       s.last_sample.mptcpi_bytes_acked - ret2);
> +	}
> +
>  	close(fd);
>  }
>  

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-next v2 5/6] selftests: mptcp: explicitly tests aggregate counters
Posted by Paolo Abeni 2 years, 7 months ago
On Tue, 2023-05-23 at 17:29 +0200, Matthieu Baerts wrote:
> 
> On 22/05/2023 15:56, Paolo Abeni wrote:
> > Update the existing sockopt test-case to do some some
> > basic checks on the newly added counters.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> >  .../selftests/net/mptcp/mptcp_sockopt.c       | 27 +++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> > index ae61f39556ca..869c041c34f3 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> > @@ -51,6 +51,11 @@ struct mptcp_info {
> >  	__u8	mptcpi_local_addr_used;
> >  	__u8	mptcpi_local_addr_max;
> >  	__u8	mptcpi_csum_enabled;
> > +	__u32	mptcpi_retransmits;
> > +	__u64	mptcpi_bytes_retrans;
> > +	__u64	mptcpi_bytes_sent;
> > +	__u64	mptcpi_bytes_received;
> > +	__u64	mptcpi_bytes_acked;
> >  };
> >  
> >  struct mptcp_subflow_data {
> > @@ -83,8 +88,10 @@ struct mptcp_subflow_addrs {
> >  
> >  struct so_state {
> >  	struct mptcp_info mi;
> > +	struct mptcp_info last_sample;
> >  	uint64_t mptcpi_rcv_delta;
> >  	uint64_t tcpi_rcv_delta;
> > +	bool pkt_stats_avail;
> >  };
> >  
> >  static void die_perror(const char *msg)
> > @@ -320,6 +327,9 @@ static void do_getsockopt_mptcp_info(struct so_state *s, int fd, size_t w)
> >  
> >  	assert(olen == sizeof(i));
> 
> (see below)
> 
> > +	s->pkt_stats_avail = olen == sizeof(i);
> 
> Good idea, it is simple! But then we should remove the assert just above.
> 
> And maybe we should have here: olen >= sizeof(i), no?
> Maybe we don't need to support newer kernels but it is just in case we
> add more info later even if I guess we would modify mptcp_info if we does.
> 
> Also, maybe "struct mptcp_info" will be defined somewhere else and we
> will not use the one defined here above?

Yep, I'll use:

	olen >= sizeof(i)

thanks!

/P