| Message ID | 761374d3-1c76-4dc2-a4cc-7bd693deb453@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: gro: remove network_header use, move p->{flush/flush_id} calculations to L4 | expand |
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> > --- > tools/testing/selftests/net/gro.c | 147 ++++++++++++++++++++++++++++++ > 1 file changed, 147 insertions(+) > > diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c > index 353e1e867fbb..5dc7b539ccbf 100644 > --- a/tools/testing/selftests/net/gro.c > +++ b/tools/testing/selftests/net/gro.c > @@ -617,6 +617,123 @@ 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) > +{ > + static char buf1[MAX_HDR_LEN + PAYLOAD_LEN]; > + static char buf2[MAX_HDR_LEN + PAYLOAD_LEN]; > + static char buf3[MAX_HDR_LEN + PAYLOAD_LEN]; > + bool send_three = false; > + struct iphdr *iph1; > + struct iphdr *iph2; > + struct iphdr *iph3; > + > + iph1 = (struct iphdr *)(buf1 + ETH_HLEN); > + iph2 = (struct iphdr *)(buf2 + ETH_HLEN); > + iph3 = (struct iphdr *)(buf3 + ETH_HLEN); > + > + 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); > + > + 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; > + } Consider moving the fix_ip4_checksum calls out of the switch to reduce duplication. > + > + 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++) { Please avoid unnamed magic constants. Something like const int num_flush_id_cases = 6; /* See switch in send_flush_id_case */ Or even define an enum with named tests and and _MAX val. It's verbose, but helpful to readers. > + sleep(1); > + send_flush_id_case(fd, daddr, i); > + sleep(1); > + write_packet(fd, fin_pkt, total_hdr_len, daddr); > + } > +} > +
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> >> --- >> tools/testing/selftests/net/gro.c | 147 ++++++++++++++++++++++++++++++ >> 1 file changed, 147 insertions(+) >> >> diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c >> index 353e1e867fbb..5dc7b539ccbf 100644 >> --- a/tools/testing/selftests/net/gro.c >> +++ b/tools/testing/selftests/net/gro.c >> @@ -617,6 +617,123 @@ 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) >> +{ >> + static char buf1[MAX_HDR_LEN + PAYLOAD_LEN]; >> + static char buf2[MAX_HDR_LEN + PAYLOAD_LEN]; >> + static char buf3[MAX_HDR_LEN + PAYLOAD_LEN]; >> + bool send_three = false; >> + struct iphdr *iph1; >> + struct iphdr *iph2; >> + struct iphdr *iph3; >> + >> + iph1 = (struct iphdr *)(buf1 + ETH_HLEN); >> + iph2 = (struct iphdr *)(buf2 + ETH_HLEN); >> + iph3 = (struct iphdr *)(buf3 + ETH_HLEN); >> + >> + 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); >> + >> + 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; >> + } > > Consider moving the fix_ip4_checksum calls out of the switch to reduce > duplication. > >> + >> + 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++) { > > Please avoid unnamed magic constants. Something like > > const int num_flush_id_cases = 6; /* See switch in send_flush_id_case */ > Will do that, thanks for the review! > Or even define an enum with named tests and and _MAX val. It's > verbose, but helpful to readers. > >> + sleep(1); >> + send_flush_id_case(fd, daddr, i); >> + sleep(1); >> + write_packet(fd, fin_pkt, total_hdr_len, daddr); >> + } >> +} >> +
diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c index 353e1e867fbb..5dc7b539ccbf 100644 --- a/tools/testing/selftests/net/gro.c +++ b/tools/testing/selftests/net/gro.c @@ -617,6 +617,123 @@ 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) +{ + static char buf1[MAX_HDR_LEN + PAYLOAD_LEN]; + static char buf2[MAX_HDR_LEN + PAYLOAD_LEN]; + static char buf3[MAX_HDR_LEN + PAYLOAD_LEN]; + bool send_three = false; + struct iphdr *iph1; + struct iphdr *iph2; + struct iphdr *iph3; + + iph1 = (struct iphdr *)(buf1 + ETH_HLEN); + iph2 = (struct iphdr *)(buf2 + ETH_HLEN); + iph3 = (struct iphdr *)(buf3 + ETH_HLEN); + + 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); + + 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 +1052,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 +1180,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.
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 | 147 ++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+)