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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <94a3e428-dfea-4910-998d-eb2e26824014@nvidia.com>
Date: Mon, 24 Nov 2025 18:10:10 -0600
From: Dan Jurgens <danielj@...dia.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: netdev@...r.kernel.org, jasowang@...hat.com, pabeni@...hat.com,
 virtualization@...ts.linux.dev, parav@...dia.com, shshitrit@...dia.com,
 yohadt@...dia.com, xuanzhuo@...ux.alibaba.com, eperezma@...hat.com,
 jgg@...pe.ca, kevin.tian@...el.com, kuba@...nel.org, andrew+netdev@...n.ch,
 edumazet@...gle.com
Subject: Re: [PATCH net-next v12 10/12] virtio_net: Add support for IPv6
 ethtool steering

On 11/24/25 5:12 PM, Michael S. Tsirkin wrote:
> 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.
> 

Agreed, will fix that.

>>>
>>>> +	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)?
> 

1. You told me to move it to the TCP patch last review.

2. None of this code is really reachable until the get ops are added in
the last patch. ethtool needs to do gets to know if/how it can set.

Bisecting would be a strange way to try to debug this series, since
functionality is added by flow type.

> 
>>>
>>>> +}
>>>> +
>>>>  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
>>>
>