* tcpdump and Big TCP
@ 2023-10-02 16:20 David Ahern
2023-10-02 16:25 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2023-10-02 16:20 UTC (permalink / raw)
To: Eric Dumazet, Xin Long; +Cc: netdev@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: tcpdump and Big TCP 2023-10-02 16:20 tcpdump and Big TCP David Ahern @ 2023-10-02 16:25 ` Eric Dumazet 2023-10-02 17:19 ` Xin Long 0 siblings, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2023-10-02 16:25 UTC (permalink / raw) To: David Ahern; +Cc: Xin Long, netdev@vger.kernel.org On Mon, Oct 2, 2023 at 6:20 PM David Ahern <dsahern@kernel.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. > 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 ? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tcpdump and Big TCP 2023-10-02 16:25 ` Eric Dumazet @ 2023-10-02 17:19 ` Xin Long 2023-10-02 17:26 ` Eric Dumazet 0 siblings, 1 reply; 6+ messages in thread From: Xin Long @ 2023-10-02 17:19 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Ahern, netdev@vger.kernel.org On Mon, Oct 2, 2023 at 12:25 PM Eric Dumazet <edumazet@google.com> wrote: > > On Mon, Oct 2, 2023 at 6:20 PM David Ahern <dsahern@kernel.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. > > 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. 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. Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tcpdump and Big TCP 2023-10-02 17:19 ` Xin Long @ 2023-10-02 17:26 ` Eric Dumazet 2023-10-02 18:59 ` Xin Long 0 siblings, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2023-10-02 17:26 UTC (permalink / raw) To: Xin Long; +Cc: David Ahern, netdev@vger.kernel.org On Mon, Oct 2, 2023 at 7:19 PM Xin Long <lucien.xin@gmail.com> wrote: > > On Mon, Oct 2, 2023 at 12:25 PM Eric Dumazet <edumazet@google.com> wrote: > > > > On Mon, Oct 2, 2023 at 6:20 PM David Ahern <dsahern@kernel.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'. > > > > > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tcpdump and Big TCP 2023-10-02 17:26 ` Eric Dumazet @ 2023-10-02 18:59 ` Xin Long 2023-11-10 14:50 ` Xin Long 0 siblings, 1 reply; 6+ messages in thread From: Xin Long @ 2023-10-02 18:59 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Ahern, netdev@vger.kernel.org On Mon, Oct 2, 2023 at 1:26 PM Eric Dumazet <edumazet@google.com> wrote: > > On Mon, Oct 2, 2023 at 7:19 PM Xin Long <lucien.xin@gmail.com> wrote: > > > > On Mon, Oct 2, 2023 at 12:25 PM Eric Dumazet <edumazet@google.com> wrote: > > > > > > On Mon, Oct 2, 2023 at 6:20 PM David Ahern <dsahern@kernel.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: https://github.com/the-tcpdump-group/tcpdump/pull/1085 > > > > > > > > > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tcpdump and Big TCP 2023-10-02 18:59 ` Xin Long @ 2023-11-10 14:50 ` Xin Long 0 siblings, 0 replies; 6+ messages in thread From: Xin Long @ 2023-11-10 14:50 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Ahern, netdev@vger.kernel.org On Mon, Oct 2, 2023 at 2:59 PM Xin Long <lucien.xin@gmail.com> wrote: > > On Mon, Oct 2, 2023 at 1:26 PM Eric Dumazet <edumazet@google.com> wrote: > > > > On Mon, Oct 2, 2023 at 7:19 PM Xin Long <lucien.xin@gmail.com> wrote: > > > > > > On Mon, Oct 2, 2023 at 12:25 PM Eric Dumazet <edumazet@google.com> wrote: > > > > > > > > On Mon, Oct 2, 2023 at 6:20 PM David Ahern <dsahern@kernel.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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-10 14:50 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-02 16:20 tcpdump and Big TCP David Ahern 2023-10-02 16:25 ` Eric Dumazet 2023-10-02 17:19 ` Xin Long 2023-10-02 17:26 ` Eric Dumazet 2023-10-02 18:59 ` Xin Long 2023-11-10 14:50 ` Xin Long
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).