Message-ID: <CADvbK_fzsXeqmayPkR4BDnkrgKDJcRd5bUXp0JNXSu8rfj-F-A@mail.gmail.com>
Date: Fri, 10 Nov 2023 09:50:15 -0500
From: Xin Long <lucien.xin@...il.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: David Ahern <dsahern@...nel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: tcpdump and Big TCP

On Mon, Oct 2, 2023 at 2:59 PM Xin Long <lucien.xin@...il.com> wrote:
>
> On Mon, Oct 2, 2023 at 1:26 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > On Mon, Oct 2, 2023 at 7:19 PM Xin Long <lucien.xin@...il.com> wrote:
> > >
> > > On Mon, Oct 2, 2023 at 12:25 PM Eric Dumazet <edumazet@...gle.com> wrote:
> > > >
> > > > On Mon, Oct 2, 2023 at 6:20 PM David Ahern <dsahern@...nel.org> wrote:
> > > > >
> > > > > Eric:
> > > > >
> > > > > Looking at the tcpdump source code, it has a GUESS_TSO define that can
> > > > > be enabled to dump IPv4 packets with tot_len = 0:
> > > > >
> > > > >         if (len < hlen) {
> > > > > #ifdef GUESS_TSO
> > > > >             if (len) {
> > > > >                 ND_PRINT("bad-len %u", len);
> > > > >                 return;
> > > > >             }
> > > > >             else {
> > > > >                 /* we guess that it is a TSO send */
> > > > >                 len = length;
> > > > >             }
> > > > > #else
> > > > >             ND_PRINT("bad-len %u", len);
> > > > >             return;
> > > > > #endif /* GUESS_TSO */
> > > > >         }
> > > > >
> > > > >
> > > > > The IPv6 version has a similar check but no compile change needed:
> > > > >         /*
> > > > >          * RFC 1883 says:
> > > > >          *
> > > > >          * The Payload Length field in the IPv6 header must be set to zero
> > > > >          * in every packet that carries the Jumbo Payload option.  If a
> > > > >          * packet is received with a valid Jumbo Payload option present and
> > > > >          * a non-zero IPv6 Payload Length field, an ICMP Parameter Problem
> > > > >          * message, Code 0, should be sent to the packet's source, pointing
> > > > >          * to the Option Type field of the Jumbo Payload option.
> > > > >          *
> > > > >          * Later versions of the IPv6 spec don't discuss the Jumbo Payload
> > > > >          * option.
> > > > >          *
> > > > >          * If the payload length is 0, we temporarily just set the total
> > > > >          * length to the remaining data in the packet (which, for Ethernet,
> > > > >          * could include frame padding, but if it's a Jumbo Payload frame,
> > > > >          * it shouldn't even be sendable over Ethernet, so we don't worry
> > > > >          * about that), so we can process the extension headers in order
> > > > >          * to *find* a Jumbo Payload hop-by-hop option and, when we've
> > > > >          * processed all the extension headers, check whether we found
> > > > >          * a Jumbo Payload option, and fail if we haven't.
> > > > >          */
> > > > >         if (payload_len != 0) {
> > > > >                 len = payload_len + sizeof(struct ip6_hdr);
> > > > >                 if (length < len)
> > > > >                         ND_PRINT("truncated-ip6 - %u bytes missing!",
> > > > >                                 len - length);
> > > > >         } else
> > > > >                 len = length + sizeof(struct ip6_hdr);
> > > > >
> > > > >
> > > > > Maybe I am missing something, but it appears that no code change to
> > > > > tcpdump is needed for Linux Big TCP packets other than enabling that
> > > > > macro when building. I did that in a local build and the large packets
> > > > > were dumped just fine.
> > > > >
> > > Right, wireshark/tshark currently has no problem parsing BIG TCP IPv4 packets.
> > > I think it enables GUESS_TSO by default.
> > >
> > > We also enabled GUESS_TSO in tcpdump for RHEL-9 when BIG TCP IPv4 was
> > > backported in it.
> >
> > Make sure to enable this in tcpdump source, so that other distros do
> > not have to 'guess'.
> Looks the tcpdump maintainer has posted one:
>
A couple of updates:

> https://github.com/the-tcpdump-group/tcpdump/pull/1085
In tcpdump, this one has been Merged into master and tcpdump-4.99 branch.
It means tcpdump has officially supported BIG TCP parsing on upstream and
its next release version.

For wireshark, according to the maintainer Guy Harris,
Code in Wireshark to deal with the total length being 0 in the IPv4 header
dates back to at least 2012.

For the TP_STATUS from packet socket including our TP_STATUS_GSO_TCP,
it has been merged into the pcapng IETF draft doc:

  https://github.com/IETF-OPSAWG-WG/draft-ietf-opsawg-pcap/pull/144

and the next step is to implement it in both tcpdump and wireshark.

But in my opinion, this doesn't block the IPv6 jumbo headers removal,
as both tcpdump and wireshark now have no issue to parse BIG TCP packets.

Thanks.

>
> >
> > >
> > > >
> > > > My point is that tcpdump should not guess, but look at TP_STATUS_GSO_TCP
> > > > (and TP_STATUS_CSUM_VALID would also be nice)
> > > >
> > > > Otherwise, why add TP_STATUS_GSO_TCP in the first place ?
> > > That's for more reliable parsing in the future.
> >
> > We want this. I thought this was obvious.
> >
> > >
> > > As currently in libpcap, it doesn't save meta_data(like
> > > TP_STATUS_CSUM_VALID/GSO_TCP)
> > > to 'pcap' files, and it requires libpcap APIs change and uses the
> > > 'pcap-ng' file format.
> > > I think it will take quite some time to implement in userspace.
> >
> > Great. Until this is implemented as discussed last year, we will not remove
> > IPv6 jumbo headers.
> I will get back to this libpcap APIs and pcap-ng things, and let you
> know when it's done.
>
> Thanks.

Powered by blists - more mailing lists