Re: [PATCH] tap: always set the no_frag flag in IPv4 headers.

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 14 Feb 2025 17:46:21 -0500
Jon Maloy <jmaloy@redhat.com> wrote:

> On 2025-02-14 06:00, Stefano Brivio wrote:
> > On Wed, 12 Feb 2025 18:50:23 -0500
> > Jon Maloy <jmaloy@redhat.com> wrote:
> >   
> >> When studying the Linux source code and Wireshark dumps it seems like
> >> the no_frag flag in the IPv4 header is always set. Following discussions
> >> in the Internet on this subject indicates that modern routers never
> >> fragment packets, and that it isn't even supported in many cases.
> >>
> >> Adding to this that incoming messages forwarded on the tap interface
> >> never even pass through a router it seems safe to always set this flag.
> >>
> >> This makes the IPv4 headers of forwarded messages identical to those
> >> sent by the external sockets, something we must consider desirable.
> >>
> >> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> >> ---
> >>   tap.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tap.c b/tap.c
> >> index d0673e5..44b0fc0 100644
> >> --- a/tap.c
> >> +++ b/tap.c
> >> @@ -153,7 +153,7 @@ static void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src,
> >>   	ip4h->tos = 0;
> >>   	ip4h->tot_len = htons(l3len);
> >>   	ip4h->id = 0;
> >> -	ip4h->frag_off = 0;
> >> +	ip4h->frag_off = htons(IP_DF);  
> > 
> > $ tshark -r test/test_logs/pasta.pcap -V -Y frame.number==9 | grep "Header Checksum"
> >      Header Checksum: 0x07d4 incorrect, should be 0xc7d3(may be caused by "IP checksum offload"?)
> > 
> > See L2_BUF_IP4_PSUM().
> 
> Not sure what to do about this. I don't even see we calculate the 
> checksum in our code

We precalculate that part, see L2_BUF_IP4_PSUM() (and also
L2_BUF_IP4_INIT()).

> so does it matter?

Well, I think it matters that we send out valid IPv4 packets. Try this
change and see for yourself.

-- 
Stefano