I am sorry for the late reply. I am not sure anymore about is it a bug I found or the author of rte_ipv4_fragment_packet() realy wanted to constraint the size of mtu writing lines:
frag_size = (uint16_t)(mtu_size - sizeof(struct ipv4_hdr)); /* Fragment size should be a multiply of 8. */ assert((frag_size & IPV4_HDR_FO_MASK) == 0); So, if we assume that any mtu size is valid then it's a bug and the function must be rewriten. Otherwise, since mtu_size is an input parameter of the function, validation should be stronger than just a assertion or it should be noted in the documentation that not all values for the paremater mtu_size are valid. I can write a patch. I just need a confirmation since I am not sure about the networking background regarding MTU. I tried to find anything about MTU in the RFC, but so far nothing. According to RFC all mtu sizes are valid. >2016-08-22 14:31 GMT+03:00 Thomas Monjalon <thomas.monjalon at 6wind.com>: >Hi, > 2016-08-15 20:30 GMT+03:00 ????????? ??????? <kiselev99 at gmail.com>: > > While playing with function rte_ipv4_fragment_packet I found that it > > incorrectly fragments packets. > > For example if the function takes 1200 bytes packet and mtu size 1000 it > > will produces two fragments. And when those fragments are reassembled back > > the resulting packet will be 4 bytes shorter than it should be. > > > > I played with linux ping program and it reports that a reply is truncated. > > 1204 bytes from 192.168.125.1: icmp_seq=1 ttl=64 (truncated) > > > > Looking at the source of rte_ipv4_fragment_packet I discovered the cause > > of the above behavior. > > > > Function makes the following assumption and the whole calculations are > > bases on that assumption. > > > > /* Fragment size should be a multiply of 8. */ > > IP_FRAG_ASSERT((frag_size & IPV4_HDR_FO_MASK) == 0); > > > > The problem is that this assert doesn?t make any sense. It's true that > > fragment size should be a multiply of 8, but what this line real checks is > > that > > the size of mtu minus 20 bytes should be multiply of 8. In other words > > it constrains the size of the mtu. So, if I take valid mtu value, say > > 1504, > > it will produce incorrect fragments when asserts are off. >Thanks for reporting. > >2016-08-15 20:48, ????????? ???????: >> I'am sorry. Looks like having an mtu value multiply of 8 is a good practice. >> >> But mtu value 1504 is also widely used in qinq linux interfaces. >Please, would like to write a patch for master branch? >Or do you prefer to delegate it to someone reading this thread? -- Alexander Kiselev
