diff mbox series

[net,v3] gve: ignore nonrelevant GSO type bits when processing TSO headers

Message ID 20240610225729.2985343-1-joshwash@google.com (mailing list archive)
State Accepted
Commit 1b9f756344416e02b41439bf2324b26aa25e141c
Delegated to: Netdev Maintainers
Headers show
Series [net,v3] gve: ignore nonrelevant GSO type bits when processing TSO headers | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 864 this patch: 864
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 868 this patch: 868
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 868 this patch: 868
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 33 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-06-11--09-00 (tests: 643)

Commit Message

Joshua Washington June 10, 2024, 10:57 p.m. UTC 
From: Joshua Washington <joshwash@google.com>

TSO currently fails when the skb's gso_type field has more than one bit
set.

TSO packets can be passed from userspace using PF_PACKET, TUNTAP and a
few others, using virtio_net_hdr (e.g., PACKET_VNET_HDR). This includes
virtualization, such as QEMU, a real use-case.

The gso_type and gso_size fields as passed from userspace in
virtio_net_hdr are not trusted blindly by the kernel. It adds gso_type
|= SKB_GSO_DODGY to force the packet to enter the software GSO stack
for verification.

This issue might similarly come up when the CWR bit is set in the TCP
header for congestion control, causing the SKB_GSO_TCP_ECN gso_type bit
to be set.

Fixes: a57e5de476be ("gve: DQO: Add TX path")
Signed-off-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Suggested-by: Eric Dumazet <edumazet@google.com>
Acked-by: Andrei Vagin <avagin@gmail.com>

v2 - Remove unnecessary comments, remove line break between fixes tag
and signoffs.

v3 - Add back unrelated empty line removal.
---
 drivers/net/ethernet/google/gve/gve_tx_dqo.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

Comments

Jakub Kicinski June 11, 2024, 12:27 a.m. UTC | #1 
On Mon, 10 Jun 2024 15:57:18 -0700 joshwash@google.com wrote:
> v2 - Remove unnecessary comments, remove line break between fixes tag
> and signoffs.
> 
> v3 - Add back unrelated empty line removal.

Read the maintainer info again, please:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
we prefer no in-reply to postings.
Joshua Washington June 11, 2024, 2:26 a.m. UTC | #2 
My apologies. I'll send an updated patch tomorrow without --in-reply-to.

On Mon, Jun 10, 2024 at 5:27 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 10 Jun 2024 15:57:18 -0700 joshwash@google.com wrote:
> > v2 - Remove unnecessary comments, remove line break between fixes tag
> > and signoffs.
> >
> > v3 - Add back unrelated empty line removal.
>
> Read the maintainer info again, please:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
> we prefer no in-reply to postings.
Jakub Kicinski June 11, 2024, 2:35 a.m. UTC | #3 
On Mon, 10 Jun 2024 19:26:32 -0700 Joshua Washington wrote:
> My apologies. I'll send an updated patch tomorrow without --in-reply-to.

No need, it's still in patchwork, it was just a note for the future.
I should have made that more clear, I realized that after hitting send.
patchwork-bot+netdevbpf@kernel.org June 12, 2024, 3 a.m. UTC | #4 
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 10 Jun 2024 15:57:18 -0700 you wrote:
> From: Joshua Washington <joshwash@google.com>
> 
> TSO currently fails when the skb's gso_type field has more than one bit
> set.
> 
> TSO packets can be passed from userspace using PF_PACKET, TUNTAP and a
> few others, using virtio_net_hdr (e.g., PACKET_VNET_HDR). This includes
> virtualization, such as QEMU, a real use-case.
> 
> [...]

Here is the summary with links:
  - [net,v3] gve: ignore nonrelevant GSO type bits when processing TSO headers
    https://git.kernel.org/netdev/net/c/1b9f75634441

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/google/gve/gve_tx_dqo.c b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
index fe1b26a4d736..0b3cca3fc792 100644
--- a/drivers/net/ethernet/google/gve/gve_tx_dqo.c
+++ b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
@@ -555,28 +555,18 @@  static int gve_prep_tso(struct sk_buff *skb)
 	if (unlikely(skb_shinfo(skb)->gso_size < GVE_TX_MIN_TSO_MSS_DQO))
 		return -1;
 
+	if (!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)))
+		return -EINVAL;
+
 	/* Needed because we will modify header. */
 	err = skb_cow_head(skb, 0);
 	if (err < 0)
 		return err;
 
 	tcp = tcp_hdr(skb);
-
-	/* Remove payload length from checksum. */
 	paylen = skb->len - skb_transport_offset(skb);
-
-	switch (skb_shinfo(skb)->gso_type) {
-	case SKB_GSO_TCPV4:
-	case SKB_GSO_TCPV6:
-		csum_replace_by_diff(&tcp->check,
-				     (__force __wsum)htonl(paylen));
-
-		/* Compute length of segmentation header. */
-		header_len = skb_tcp_all_headers(skb);
-		break;
-	default:
-		return -EINVAL;
-	}
+	csum_replace_by_diff(&tcp->check, (__force __wsum)htonl(paylen));
+	header_len = skb_tcp_all_headers(skb);
 
 	if (unlikely(header_len > GVE_TX_MAX_HDR_SIZE_DQO))
 		return -EINVAL;