Re: [PATCH net-next v12 10/12] virtio_net: Add support for IPv6 ethtool steering

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Nov 24, 2025 at 05:04:30PM -0600, Dan Jurgens wrote:
> On 11/24/25 3:59 PM, Michael S. Tsirkin wrote:
> > On Wed, Nov 19, 2025 at 01:15:21PM -0600, Daniel Jurgens wrote:
> >> Implement support for IPV6_USER_FLOW type rules.
> >>
> 
> >>  	return false;
> >> @@ -5958,11 +5989,33 @@ static void parse_ip4(struct iphdr *mask, struct iphdr *key,
> >>  	}
> >>  }
> >>  
> >> +static void parse_ip6(struct ipv6hdr *mask, struct ipv6hdr *key,
> >> +		      const struct ethtool_rx_flow_spec *fs)
> >> +{
> > 
> > I note logic wise it is different from ipv4, it is looking at the fs.
> 
> I'm not following you here. They both get the l3_mask and l3_val from
> the flow spec.

yes but ipv4 is buggy in your patch.

> > 
> >> +	const struct ethtool_usrip6_spec *l3_mask = &fs->m_u.usr_ip6_spec;
> >> +	const struct ethtool_usrip6_spec *l3_val  = &fs->h_u.usr_ip6_spec;
> >> +
> >> +	if (!ipv6_addr_any((struct in6_addr *)l3_mask->ip6src)) {
> >> +		memcpy(&mask->saddr, l3_mask->ip6src, sizeof(mask->saddr));
> >> +		memcpy(&key->saddr, l3_val->ip6src, sizeof(key->saddr));
> >> +	}
> >> +
> >> +	if (!ipv6_addr_any((struct in6_addr *)l3_mask->ip6dst)) {
> >> +		memcpy(&mask->daddr, l3_mask->ip6dst, sizeof(mask->daddr));
> >> +		memcpy(&key->daddr, l3_val->ip6dst, sizeof(key->daddr));
> >> +	}
> > 
> > Is this enough?
> > For example, what if user tries to set up a filter by l4_proto ?
> > 
> 
> That's in the next patch.

yes but if just this one is applied (e.g. by bisect)?


> > 
> >> +}
> >> +
> >>  static bool has_ipv4(u32 flow_type)
> >>  {
> >>  	return flow_type == IP_USER_FLOW;
> >>  }
> >>  
> >> +static bool has_ipv6(u32 flow_type)
> >> +{
> >> +	return flow_type == IPV6_USER_FLOW;
> >> +}
> >> +
> dr);
> >>  
> >> -	if (fs->h_u.usr_ip4_spec.l4_4_bytes ||
> >> -	    fs->h_u.usr_ip4_spec.ip_ver != ETH_RX_NFC_IP4 ||
> >> -	    fs->m_u.usr_ip4_spec.l4_4_bytes ||
> >> -	    fs->m_u.usr_ip4_spec.ip_ver ||
> >> -	    fs->m_u.usr_ip4_spec.proto)
> >> -		return -EINVAL;
> >> +		if (fs->h_u.usr_ip6_spec.l4_4_bytes ||
> >> +		    fs->m_u.usr_ip6_spec.l4_4_bytes)
> >> +			return -EINVAL;
> >>  
> >> -	parse_ip4(v4_m, v4_k, fs);
> >> +		parse_ip6(v6_m, v6_k, fs);
> > 
> > 
> > why does ipv6 not check unsupported fields unlike ipv4?
> 
> The UAPI for user_ip6 doesn't make the same assertions:
> 
> /**
> 
>  * struct ethtool_usrip6_spec - general flow specification for IPv6
> 
>  * @ip6src: Source host
> 
>  * @ip6dst: Destination host
> 
>  * @l4_4_bytes: First 4 bytes of transport (layer 4) header
> 
>  * @tclass: Traffic Class
> 
>  * @l4_proto: Transport protocol number (nexthdr after any Extension
> Headers)                                          ]
>  */
> 
> /**
>  * struct ethtool_usrip4_spec - general flow specification for IPv4
>  * @ip4src: Source host
>  * @ip4dst: Destination host
>  * @l4_4_bytes: First 4 bytes of transport (layer 4) header
>  * @tos: Type-of-service
>  * @ip_ver: Value must be %ETH_RX_NFC_IP4; mask must be 0
>  * @proto: Transport protocol number; mask must be 0
>  */
> 
> A check of l4_proto is probably reasonable though, since this is adding
> filter by IP only, so l4_proto should be unset.


maybe run this by relevant maintainers.
> 
> > 
> >> +	} else {
> >> +		selector->type = VIRTIO_NET_FF_MASK_TYPE_IPV4;
> >> +		selector->length = sizeof(struct iphdr);
> >> +
> >> +		if (fs->h_u.usr_ip4_spec.l4_4_bytes ||
> >> +		    fs->h_u.usr_ip4_spec.ip_ver != ETH_RX_NFC_IP4 ||
> >> +		    fs->m_u.usr_ip4_spec.l4_4_bytes ||
> >> +		    fs->m_u.usr_ip4_spec.ip_ver ||
> >> +		    fs->m_u.usr_ip4_spec.proto)
> >> +			return -EINVAL;
> >> +
> >> +		parse_ip4(v4_m, v4_k, fs);
> >> +	}
> >>  
> >>  	return 0;
> >>  }
> >> -- 
> >> 2.50.1
> >