diff mbox series

[net-next,v7,3/3] selftests/net: add flush id selftests

Message ID 20240412155533.115507-4-richardbgobert@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: gro: move p->{flush/flush_id} calculations to L4 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/apply fail Patch does not apply to net-next-1

Commit Message

Richard Gobert April 12, 2024, 3:55 p.m. UTC 
Added flush id selftests to test different cases where DF flag is set or
unset and id value changes in the following packets. All cases where the
packets should coalesce or should not coalesce are tested.

Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
---
 tools/testing/selftests/net/gro.c | 144 ++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)

Comments

Willem de Bruijn April 14, 2024, 12:55 a.m. UTC | #1 
Richard Gobert wrote:
> Added flush id selftests to test different cases where DF flag is set or
> unset and id value changes in the following packets. All cases where the
> packets should coalesce or should not coalesce are tested.
> 
> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>

Thanks for adding tests. Minor point below only. The tests pass both
before and after your series, right? Then immediately a nice
validation that the optimization has no unintended side-effects.

> ---
>  tools/testing/selftests/net/gro.c | 144 ++++++++++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
> index 353e1e867fbb..74ab06953c38 100644
> --- a/tools/testing/selftests/net/gro.c
> +++ b/tools/testing/selftests/net/gro.c
> @@ -617,6 +617,120 @@ static void add_ipv6_exthdr(void *buf, void *optpkt, __u8 exthdr_type, char *ext
>  	iph->payload_len = htons(ntohs(iph->payload_len) + MIN_EXTHDR_SIZE);
>  }
>  
> +static void fix_ip4_checksum(struct iphdr *iph)
> +{
> +	iph->check = 0;
> +	iph->check = checksum_fold(iph, sizeof(struct iphdr), 0);
> +}
> +
> +static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
> +{
> +	bool send_three = false;
> +	static char buf1[MAX_HDR_LEN + PAYLOAD_LEN];
> +	static char buf2[MAX_HDR_LEN + PAYLOAD_LEN];
> +	static char buf3[MAX_HDR_LEN + PAYLOAD_LEN];
> +
> +	create_packet(buf1, 0, 0, PAYLOAD_LEN, 0);
> +	create_packet(buf2, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
> +	create_packet(buf3, PAYLOAD_LEN * 2, 0, PAYLOAD_LEN, 0);
> +
> +	struct iphdr *iph1 = (struct iphdr *)(buf1 + ETH_HLEN);
> +	struct iphdr *iph2 = (struct iphdr *)(buf2 + ETH_HLEN);
> +	struct iphdr *iph3 = (struct iphdr *)(buf3 + ETH_HLEN);
> +

minor: variable defintions before code, and reverse chrismas tree.
Richard Gobert April 17, 2024, 2:01 p.m. UTC | #2 
Willem de Bruijn wrote:
> Richard Gobert wrote:
>> Added flush id selftests to test different cases where DF flag is set or
>> unset and id value changes in the following packets. All cases where the
>> packets should coalesce or should not coalesce are tested.
>>
>> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
> 
> Thanks for adding tests. Minor point below only. The tests pass both
> before and after your series, right? Then immediately a nice
> validation that the optimization has no unintended side-effects.
> 

Yes, the logic is preserved - tests pass both in net-next and after
applying the patch :)

>> ---
>>  tools/testing/selftests/net/gro.c | 144 ++++++++++++++++++++++++++++++
>>  1 file changed, 144 insertions(+)
>>
>> diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
>> index 353e1e867fbb..74ab06953c38 100644
>> --- a/tools/testing/selftests/net/gro.c
>> +++ b/tools/testing/selftests/net/gro.c
>> @@ -617,6 +617,120 @@ static void add_ipv6_exthdr(void *buf, void *optpkt, __u8 exthdr_type, char *ext
>>  	iph->payload_len = htons(ntohs(iph->payload_len) + MIN_EXTHDR_SIZE);
>>  }
>>  
>> +static void fix_ip4_checksum(struct iphdr *iph)
>> +{
>> +	iph->check = 0;
>> +	iph->check = checksum_fold(iph, sizeof(struct iphdr), 0);
>> +}
>> +
>> +static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
>> +{
>> +	bool send_three = false;
>> +	static char buf1[MAX_HDR_LEN + PAYLOAD_LEN];
>> +	static char buf2[MAX_HDR_LEN + PAYLOAD_LEN];
>> +	static char buf3[MAX_HDR_LEN + PAYLOAD_LEN];
>> +
>> +	create_packet(buf1, 0, 0, PAYLOAD_LEN, 0);
>> +	create_packet(buf2, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
>> +	create_packet(buf3, PAYLOAD_LEN * 2, 0, PAYLOAD_LEN, 0);
>> +
>> +	struct iphdr *iph1 = (struct iphdr *)(buf1 + ETH_HLEN);
>> +	struct iphdr *iph2 = (struct iphdr *)(buf2 + ETH_HLEN);
>> +	struct iphdr *iph3 = (struct iphdr *)(buf3 + ETH_HLEN);
>> +
> 
> minor: variable defintions before code, and reverse chrismas tree.

Good catch, I'll apply these changes and push v8 when the relevant series
for net will be merged. Thanks!
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
index 353e1e867fbb..74ab06953c38 100644
--- a/tools/testing/selftests/net/gro.c
+++ b/tools/testing/selftests/net/gro.c
@@ -617,6 +617,120 @@  static void add_ipv6_exthdr(void *buf, void *optpkt, __u8 exthdr_type, char *ext
 	iph->payload_len = htons(ntohs(iph->payload_len) + MIN_EXTHDR_SIZE);
 }
 
+static void fix_ip4_checksum(struct iphdr *iph)
+{
+	iph->check = 0;
+	iph->check = checksum_fold(iph, sizeof(struct iphdr), 0);
+}
+
+static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
+{
+	bool send_three = false;
+	static char buf1[MAX_HDR_LEN + PAYLOAD_LEN];
+	static char buf2[MAX_HDR_LEN + PAYLOAD_LEN];
+	static char buf3[MAX_HDR_LEN + PAYLOAD_LEN];
+
+	create_packet(buf1, 0, 0, PAYLOAD_LEN, 0);
+	create_packet(buf2, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
+	create_packet(buf3, PAYLOAD_LEN * 2, 0, PAYLOAD_LEN, 0);
+
+	struct iphdr *iph1 = (struct iphdr *)(buf1 + ETH_HLEN);
+	struct iphdr *iph2 = (struct iphdr *)(buf2 + ETH_HLEN);
+	struct iphdr *iph3 = (struct iphdr *)(buf3 + ETH_HLEN);
+
+	switch (tcase) {
+	case 0: /* DF=1, Incrementing - should coalesce */
+		iph1->frag_off |= htons(IP_DF);
+		iph1->id = htons(8);
+		fix_ip4_checksum(iph1);
+
+		iph2->frag_off |= htons(IP_DF);
+		iph2->id = htons(9);
+		fix_ip4_checksum(iph2);
+		break;
+
+	case 1: /* DF=1, Fixed - should coalesce */
+		iph1->frag_off |= htons(IP_DF);
+		iph1->id = htons(8);
+		fix_ip4_checksum(iph1);
+
+		iph2->frag_off |= htons(IP_DF);
+		iph2->id = htons(8);
+		fix_ip4_checksum(iph2);
+		break;
+
+	case 2: /* DF=0, Incrementing - should coalesce */
+		iph1->frag_off &= ~htons(IP_DF);
+		iph1->id = htons(8);
+		fix_ip4_checksum(iph1);
+
+		iph2->frag_off &= ~htons(IP_DF);
+		iph2->id = htons(9);
+		fix_ip4_checksum(iph2);
+		break;
+
+	case 3: /* DF=0, Fixed - should not coalesce */
+		iph1->frag_off &= ~htons(IP_DF);
+		iph1->id = htons(8);
+		fix_ip4_checksum(iph1);
+
+		iph2->frag_off &= ~htons(IP_DF);
+		iph2->id = htons(8);
+		fix_ip4_checksum(iph2);
+		break;
+
+	case 4: /* DF=1, two packets incrementing, and one fixed - should
+		 * coalesce only the first two packets
+		 */
+		iph1->frag_off |= htons(IP_DF);
+		iph1->id = htons(8);
+		fix_ip4_checksum(iph1);
+
+		iph2->frag_off |= htons(IP_DF);
+		iph2->id = htons(9);
+		fix_ip4_checksum(iph2);
+
+		iph3->frag_off |= htons(IP_DF);
+		iph3->id = htons(9);
+		fix_ip4_checksum(iph3);
+		send_three = true;
+		break;
+
+	case 5: /* DF=1, two packets fixed, and one incrementing - should
+		 * coalesce only the first two packets
+		 */
+		iph1->frag_off |= htons(IP_DF);
+		iph1->id = htons(8);
+		fix_ip4_checksum(iph1);
+
+		iph2->frag_off |= htons(IP_DF);
+		iph2->id = htons(8);
+		fix_ip4_checksum(iph2);
+
+		iph3->frag_off |= htons(IP_DF);
+		iph3->id = htons(9);
+		fix_ip4_checksum(iph3);
+		send_three = true;
+		break;
+	}
+
+	write_packet(fd, buf1, total_hdr_len + PAYLOAD_LEN, daddr);
+	write_packet(fd, buf2, total_hdr_len + PAYLOAD_LEN, daddr);
+
+	if (send_three)
+		write_packet(fd, buf3, total_hdr_len + PAYLOAD_LEN, daddr);
+}
+
+static void test_flush_id(int fd, struct sockaddr_ll *daddr, char *fin_pkt)
+{
+	for (int i = 0; i < 6; i++) {
+		sleep(1);
+		send_flush_id_case(fd, daddr, i);
+		sleep(1);
+		write_packet(fd, fin_pkt, total_hdr_len, daddr);
+	}
+}
+
 static void send_ipv6_exthdr(int fd, struct sockaddr_ll *daddr, char *ext_data1, char *ext_data2)
 {
 	static char buf[MAX_HDR_LEN + PAYLOAD_LEN];
@@ -935,6 +1049,8 @@  static void gro_sender(void)
 			send_fragment4(txfd, &daddr);
 			sleep(1);
 			write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
+
+			test_flush_id(txfd, &daddr, fin_pkt);
 		} else if (proto == PF_INET6) {
 			sleep(1);
 			send_fragment6(txfd, &daddr);
@@ -1061,6 +1177,34 @@  static void gro_receiver(void)
 
 			printf("fragmented ip4 doesn't coalesce: ");
 			check_recv_pkts(rxfd, correct_payload, 2);
+
+			/* is_atomic checks */
+			printf("DF=1, Incrementing - should coalesce: ");
+			correct_payload[0] = PAYLOAD_LEN * 2;
+			check_recv_pkts(rxfd, correct_payload, 1);
+
+			printf("DF=1, Fixed - should coalesce: ");
+			correct_payload[0] = PAYLOAD_LEN * 2;
+			check_recv_pkts(rxfd, correct_payload, 1);
+
+			printf("DF=0, Incrementing - should coalesce: ");
+			correct_payload[0] = PAYLOAD_LEN * 2;
+			check_recv_pkts(rxfd, correct_payload, 1);
+
+			printf("DF=0, Fixed - should not coalesce: ");
+			correct_payload[0] = PAYLOAD_LEN;
+			correct_payload[1] = PAYLOAD_LEN;
+			check_recv_pkts(rxfd, correct_payload, 2);
+
+			printf("DF=1, 2 Incrementing and one fixed - should coalesce only first 2 packets: ");
+			correct_payload[0] = PAYLOAD_LEN * 2;
+			correct_payload[1] = PAYLOAD_LEN;
+			check_recv_pkts(rxfd, correct_payload, 2);
+
+			printf("DF=1, 2 Fixed and one incrementing - should coalesce only first 2 packets: ");
+			correct_payload[0] = PAYLOAD_LEN * 2;
+			correct_payload[1] = PAYLOAD_LEN;
+			check_recv_pkts(rxfd, correct_payload, 2);
 		} else if (proto == PF_INET6) {
 			/* GRO doesn't check for ipv6 hop limit when flushing.
 			 * Hence no corresponding test to the ipv4 case.