Re: [PATCH net-next v21 06/13] virtio_net: Create a FF group for ethtool steering

On 03/08/2026 19:27, Michael S. Tsirkin wrote:
> On Mon, Aug 03, 2026 at 05:07:14PM +0300, Shahar Shitrit wrote:
>> From: Daniel Jurgens <[email protected]>
>>
>> All ethtool steering rules will go in one group, create it during
>> initialization.
>>
>> Signed-off-by: Daniel Jurgens <[email protected]>
>> Reviewed-by: Parav Pandit <[email protected]>
>> Reviewed-by: Xuan Zhuo <[email protected]>
>> Signed-off-by: Shahar Shitrit <[email protected]>
>> ---
>>  drivers/net/virtio_net.c           | 29 +++++++++++++++++++++++++++++
>>  include/uapi/linux/virtio_net_ff.h | 15 +++++++++++++++
>>  2 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 0d1f5a6cfd12..e11e42583243 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -287,6 +287,9 @@ static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc_qstat[] = {
>>  	VIRTNET_STATS_DESC_TX_QSTAT(speed, ratelimit_packets, hw_drop_ratelimits),
>>  };
>>  
>> +#define VIRTNET_FF_ETHTOOL_GROUP_PRIORITY 1
> 
> why 1 and not 0?
We left 0 open for ARFS. We want ethtool rules to have higher priority.
But since VIRTNET_FF_ETHTOOL_GROUP_PRIORITY is not part of the UAPI
header file, I will change it to 0 now and later when ARFS is
implemented, it could be changed to 1.
> 
>> +#define VIRTNET_FF_MAX_GROUPS 1
>> +
>>  struct virtnet_ff {
>>  	struct virtio_device *vdev;
>>  	bool ff_supported;
>> @@ -5749,6 +5752,7 @@ static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
>>  	size_t ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data) +
>>  			      sizeof(struct virtio_net_ff_selector) *
>>  			      VIRTIO_NET_FF_MASK_TYPE_MAX;
>> +	struct virtio_net_resource_obj_ff_group ethtool_group = {};
>>  	struct virtio_admin_cmd_query_cap_id_result *cap_id_list;
>>  	struct virtio_net_ff_selector *sel;
>>  	unsigned long sel_types = 0;
>> @@ -5839,6 +5843,12 @@ static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
>>  	ff->ff_actions->count = min_t(u8, ff->ff_actions->count,
>>  				      VIRTIO_NET_FF_ACTION_MAX);
>>  
>> +	if (le32_to_cpu(ff->ff_caps->groups_limit) < VIRTNET_FF_MAX_GROUPS) {
>> +		err = -ENOSPC;
>> +		goto err_ff_action;
>> +	}
>> +	ff->ff_caps->groups_limit = cpu_to_le32(VIRTNET_FF_MAX_GROUPS);
>> +
>>  	err = virtio_admin_cap_set(vdev,
>>  				   VIRTIO_NET_FF_RESOURCE_CAP,
>>  				   ff->ff_caps,
>> @@ -5886,6 +5896,19 @@ static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
>>  	if (err)
>>  		goto err_ff_action;
>>  
>> +	ethtool_group.group_priority = cpu_to_le16(VIRTNET_FF_ETHTOOL_GROUP_PRIORITY);
>> +
>> +	/* Use priority for the object ID. */
> 
> and how is this ok?
> spec says:
>  Each resource object has a unique resource object ID - a driver-assigned number
>   in the range of 0 to \field{limit - 1}, where the \field{limit} is the maximum
>   number set by the driver for this resource object type.
> 
> and limit here is 1, right?
right. it is fixed by changing VIRTNET_FF_ETHTOOL_GROUP_PRIORITY to 0.
> 
>> +	err = virtio_admin_obj_create(vdev,
>> +				      VIRTIO_NET_RESOURCE_OBJ_FF_GROUP,
>> +				      VIRTNET_FF_ETHTOOL_GROUP_PRIORITY,
>> +				      VIRTIO_ADMIN_GROUP_TYPE_SELF,
>> +				      0,
>> +				      &ethtool_group,
>> +				      sizeof(ethtool_group));
>> +	if (err)
>> +		goto err_ff_action;
>> +
>>  	ff->vdev = vdev;
>>  	ff->ff_supported = true;
>>  
>> @@ -5913,6 +5936,12 @@ static void virtnet_ff_cleanup(struct virtnet_ff *ff)
>>  	if (!ff->ff_supported)
>>  		return;
>>  
>> +	virtio_admin_obj_destroy(ff->vdev,
>> +				 VIRTIO_NET_RESOURCE_OBJ_FF_GROUP,
>> +				 VIRTNET_FF_ETHTOOL_GROUP_PRIORITY,
>> +				 VIRTIO_ADMIN_GROUP_TYPE_SELF,
>> +				 0);
>> +
>>  	kfree(ff->ff_actions);
>>  	kfree(ff->ff_mask);
>>  	kfree(ff->ff_caps);
>> diff --git a/include/uapi/linux/virtio_net_ff.h b/include/uapi/linux/virtio_net_ff.h
>> index f9e771a0baff..b0780e49df9f 100644
>> --- a/include/uapi/linux/virtio_net_ff.h
>> +++ b/include/uapi/linux/virtio_net_ff.h
>> @@ -12,6 +12,8 @@
>>  #define VIRTIO_NET_FF_SELECTOR_CAP 0x801
>>  #define VIRTIO_NET_FF_ACTION_CAP 0x802
>>  
>> +#define VIRTIO_NET_RESOURCE_OBJ_FF_GROUP 0x0200
>> +
>>  /**
>>   * struct virtio_net_ff_cap_data - Flow filter resource capability limits
>>   * @groups_limit: maximum number of flow filter groups supported by the device
>> @@ -88,4 +90,17 @@ struct virtio_net_ff_actions {
>>  	__u8 reserved[7];
>>  	__u8 actions[] __counted_by(count);
>>  };
>> +
>> +/**
>> + * struct virtio_net_resource_obj_ff_group - Flow filter group object
>> + * @group_priority: priority of the group used to order evaluation
>> + *
>> + * This structure is the payload for the VIRTIO_NET_RESOURCE_OBJ_FF_GROUP
>> + * administrative object. Devices use @group_priority to order flow filter
>> + * groups. Multi-byte fields are little-endian.
>> + */
>> +struct virtio_net_resource_obj_ff_group {
>> +	__le16 group_priority;
>> +};
>> +
>>  #endif
>> -- 
>> 2.49.0
>