netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Daniel Jurgens <danielj@nvidia.com>
Cc: netdev@vger.kernel.org, mst@redhat.com, jasowang@redhat.com,
	pabeni@redhat.com, virtualization@lists.linux.dev,
	parav@nvidia.com, shshitrit@nvidia.com, yohadt@nvidia.com,
	xuanzhuo@linux.alibaba.com, eperezma@redhat.com, jgg@ziepe.ca,
	kevin.tian@intel.com, kuba@kernel.org, andrew+netdev@lunn.ch,
	edumazet@google.com
Subject: Re: [PATCH net-next v13 11/12] virtio_net: Add support for TCP and UDP ethtool rules
Date: Tue, 2 Dec 2025 15:55:39 +0000	[thread overview]
Message-ID: <aS8L--z0ezhkywT_@horms.kernel.org> (raw)
In-Reply-To: <20251126193539.7791-12-danielj@nvidia.com>

On Wed, Nov 26, 2025 at 01:35:38PM -0600, Daniel Jurgens wrote:

...

> @@ -6005,6 +6085,11 @@ static void parse_ip4(struct iphdr *mask, struct iphdr *key,
>  		mask->tos = l3_mask->tos;
>  		key->tos = l3_val->tos;
>  	}
> +
> +	if (l3_mask->proto) {
> +		mask->protocol = l3_mask->proto;
> +		key->protocol = l3_val->proto;
> +	}
>  }

Hi Daniel,

Claude Code with review-prompts flags an issue here,
which I can't convince myself is not the case.

If parse_ip4() is called for a IP_USER_FLOW, which use ethtool_usrip4_spec,
as does this function, then all is well.

However, it seems that it may also be called for TCP_V4_FLOW and UDP_V4_FLOW
flows, in which case accessing .proto will overrun the mask and key which
are actually struct ethtool_tcpip4_spec.

https://netdev-ai.bots.linux.dev/ai-review.html?id=51d97b85-5ca3-4cb8-a96a-0d6eab5e7196#patch-10

>  
>  static void parse_ip6(struct ipv6hdr *mask, struct ipv6hdr *key,
> @@ -6022,16 +6107,35 @@ static void parse_ip6(struct ipv6hdr *mask, struct ipv6hdr *key,
>  		memcpy(&mask->daddr, l3_mask->ip6dst, sizeof(mask->daddr));
>  		memcpy(&key->daddr, l3_val->ip6dst, sizeof(key->daddr));
>  	}
> +
> +	if (l3_mask->l4_proto) {
> +		mask->nexthdr = l3_mask->l4_proto;
> +		key->nexthdr = l3_val->l4_proto;
> +	}

Likewise here.

>  }

...

  parent reply	other threads:[~2025-12-02 15:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-26 19:35 [PATCH net-next v13 00/12] virtio_net: Add ethtool flow rules support Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 01/12] virtio_pci: Remove supported_cap size build assert Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 02/12] virtio: Add config_op for admin commands Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 03/12] virtio: Expose generic device capability operations Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 04/12] virtio: Expose object create and destroy API Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 05/12] virtio_net: Query and set flow filter caps Daniel Jurgens
2025-11-26 23:31   ` Michael S. Tsirkin
2025-12-01 15:15     ` Dan Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 06/12] virtio_net: Create a FF group for ethtool steering Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 07/12] virtio_net: Implement layer 2 ethtool flow rules Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 08/12] virtio_net: Use existing classifier if possible Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 09/12] virtio_net: Implement IPv4 ethtool flow rules Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 10/12] virtio_net: Add support for IPv6 ethtool steering Daniel Jurgens
2025-11-26 19:35 ` [PATCH net-next v13 11/12] virtio_net: Add support for TCP and UDP ethtool rules Daniel Jurgens
2025-12-01 15:26   ` Michael S. Tsirkin
2025-12-01 17:12     ` Dan Jurgens
2025-12-02 15:55   ` Simon Horman [this message]
2025-12-02 22:45     ` Dan Jurgens
2025-12-03 13:33     ` Michael S. Tsirkin
2025-12-03 16:02       ` Chris Mason
2025-12-04  7:16         ` Michael S. Tsirkin
2025-12-04 11:55           ` Chris Mason
2025-12-03 20:09       ` Simon Horman
2025-11-26 19:35 ` [PATCH net-next v13 12/12] virtio_net: Add get ethtool flow rules ops Daniel Jurgens

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aS8L--z0ezhkywT_@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=danielj@nvidia.com \
    --cc=edumazet@google.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=kevin.tian@intel.com \
    --cc=kuba@kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=parav@nvidia.com \
    --cc=shshitrit@nvidia.com \
    --cc=virtualization@lists.linux.dev \
    --cc=xuanzhuo@linux.alibaba.com \
    --cc=yohadt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).