[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b37d8e3e-ce0d-4778-8b1a-022cbd854cbd@nvidia.com>
Date: Tue, 21 Oct 2025 08:38:47 -0500
From: Dan Jurgens <danielj@...dia.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: netdev@...r.kernel.org, jasowang@...hat.com, alex.williamson@...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, shameerali.kolothum.thodi@...wei.com, jgg@...pe.ca,
kevin.tian@...el.com, kuba@...nel.org, andrew+netdev@...n.ch,
edumazet@...gle.com
Subject: Re: [PATCH net-next v5 05/12] virtio_net: Query and set flow filter
caps
On 10/21/25 8:18 AM, Michael S. Tsirkin wrote:
> On Thu, Oct 16, 2025 at 12:00:48AM -0500, Daniel Jurgens wrote:
>> + /* VIRTIO_NET_FF_MASK_TYPE start at 1 */
>> + for (i = 1; i <= VIRTIO_NET_FF_MASK_TYPE_MAX; i++)
>> + ff_mask_size += get_mask_size(i);
>> +
>> +
>> + err = virtio_admin_cap_set(vdev,
>> + VIRTIO_NET_FF_RESOURCE_CAP,
>> + ff->ff_caps,
>> + sizeof(*ff->ff_caps));
>> + if (err)
>> + goto err_ff_action;
>> +
>> + ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
>
> overriding ff_mask_size seems unncessarily funky.
> use a variable reflecting what this is?
>
I can use another variable, and check that it matches the original.
The code below is just validating the mask data is sane.
>
>> + sel = &ff->ff_mask->selectors[0];
>> +
>> + for (i = 0; i < ff->ff_mask->count; i++) {
>> + if (sel->length > MAX_SEL_LEN) {
>> + err = -EINVAL;
>> + goto err_ff_action;
>> + }
>
> You also need to validate this is all within allocated size.
>
>> + ff_mask_size += sizeof(struct virtio_net_ff_selector) + sel->length;
>> + sel = (void *)sel + sizeof(*sel) + sel->length;
>
>> + }
>> +
>> + err = virtio_admin_cap_set(vdev,
>> + VIRTIO_NET_FF_SELECTOR_CAP,
>> + ff->ff_mask,
>> + ff_mask_size);
>> + if (err)
>> + goto err_ff_action;
>> +
>> + err = virtio_admin_cap_set(vdev,
>> + VIRTIO_NET_FF_ACTION_CAP,
>> + ff->ff_actions,
>> + sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
>> + if (err)
>> + goto err_ff_action;
>> +
>> + ff->vdev = vdev;
>> + ff->ff_supported = true;
>> +
>> + kfree(cap_id_list);
>> +
>> + return;
>> +
>> +err_ff_action:
>> + kfree(ff->ff_actions);
>> +err_ff_mask:
>> + kfree(ff->ff_mask);
>> +err_ff:
>> + kfree(ff->ff_caps);
>> +err_cap_list:
>> + kfree(cap_id_list);
>> +}
>> +
>> +static void virtnet_ff_cleanup(struct virtnet_ff *ff)
>> +{
>> + if (!ff->ff_supported)
>> + return;
>> +
>> + kfree(ff->ff_actions);
>> + kfree(ff->ff_mask);
>> + kfree(ff->ff_caps);
>> +}
>> +
>> static int virtnet_probe(struct virtio_device *vdev)
>> {
>> int i, err = -ENOMEM;
>> @@ -7116,6 +7277,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>> }
>> vi->guest_offloads_capable = vi->guest_offloads;
>>
>> + virtnet_ff_init(&vi->ff, vi->vdev);
>> +
>> rtnl_unlock();
>>
>> err = virtnet_cpu_notif_add(vi);
>> @@ -7131,6 +7294,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>>
>> free_unregister_netdev:
>> unregister_netdev(dev);
>> + virtnet_ff_cleanup(&vi->ff);
>> free_failover:
>> net_failover_destroy(vi->failover);
>> free_vqs:
>> @@ -7180,6 +7344,7 @@ static void virtnet_remove(struct virtio_device *vdev)
>> virtnet_free_irq_moder(vi);
>>
>> unregister_netdev(vi->dev);
>> + virtnet_ff_cleanup(&vi->ff);
>>
>> net_failover_destroy(vi->failover);
>>
>> diff --git a/include/linux/virtio_admin.h b/include/linux/virtio_admin.h
>> index 039b996f73ec..db0f42346ca9 100644
>> --- a/include/linux/virtio_admin.h
>> +++ b/include/linux/virtio_admin.h
>> @@ -3,6 +3,7 @@
>> * Header file for virtio admin operations
>> */
>> #include <uapi/linux/virtio_pci.h>
>> +#include <uapi/linux/virtio_net_ff.h>
>>
>> #ifndef _LINUX_VIRTIO_ADMIN_H
>> #define _LINUX_VIRTIO_ADMIN_H
>> diff --git a/include/uapi/linux/virtio_net_ff.h b/include/uapi/linux/virtio_net_ff.h
>> new file mode 100644
>> index 000000000000..1a4738889403
>> --- /dev/null
>> +++ b/include/uapi/linux/virtio_net_ff.h
>> @@ -0,0 +1,91 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>> + *
>> + * Header file for virtio_net flow filters
>> + */
>> +#ifndef _LINUX_VIRTIO_NET_FF_H
>> +#define _LINUX_VIRTIO_NET_FF_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/kernel.h>
>> +
>> +#define VIRTIO_NET_FF_RESOURCE_CAP 0x800
>> +#define VIRTIO_NET_FF_SELECTOR_CAP 0x801
>> +#define VIRTIO_NET_FF_ACTION_CAP 0x802
>> +
>> +/**
>> + * struct virtio_net_ff_cap_data - Flow filter resource capability limits
>> + * @groups_limit: maximum number of flow filter groups supported by the device
>> + * @classifiers_limit: maximum number of classifiers supported by the device
>> + * @rules_limit: maximum number of rules supported device-wide across all groups
>> + * @rules_per_group_limit: maximum number of rules allowed in a single group
>> + * @last_rule_priority: priority value associated with the lowest-priority rule
>> + * @selectors_per_classifier_limit: maximum selectors allowed in one classifier
>> + *
>> + * The limits are reported by the device and describe resource capacities for
>> + * flow filters. Multi-byte fields are little-endian.
>> + */
>> +struct virtio_net_ff_cap_data {
>> + __le32 groups_limit;
>> + __le32 classifiers_limit;
>> + __le32 rules_limit;
>> + __le32 rules_per_group_limit;
>> + __u8 last_rule_priority;
>> + __u8 selectors_per_classifier_limit;
>> +};
>> +
>> +/**
>> + * struct virtio_net_ff_selector - Selector mask descriptor
>> + * @type: selector type, one of VIRTIO_NET_FF_MASK_TYPE_* constants
>> + * @flags: selector flags, see VIRTIO_NET_FF_MASK_F_* constants
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @length: size in bytes of @mask
>> + * @reserved1: must be set to 0 by the driver and ignored by the device
>> + * @mask: variable-length mask payload for @type, length given by @length
>> + *
>> + * A selector describes a header mask that a classifier can apply. The format
>> + * of @mask depends on @type.
>> + */
>> +struct virtio_net_ff_selector {
>> + __u8 type;
>> + __u8 flags;
>> + __u8 reserved[2];
>> + __u8 length;
>> + __u8 reserved1[3];
>> + __u8 mask[];
>> +};
>> +
>> +#define VIRTIO_NET_FF_MASK_TYPE_ETH 1
>> +#define VIRTIO_NET_FF_MASK_TYPE_IPV4 2
>> +#define VIRTIO_NET_FF_MASK_TYPE_IPV6 3
>> +#define VIRTIO_NET_FF_MASK_TYPE_TCP 4
>> +#define VIRTIO_NET_FF_MASK_TYPE_UDP 5
>> +#define VIRTIO_NET_FF_MASK_TYPE_MAX VIRTIO_NET_FF_MASK_TYPE_UDP
>> +
>> +/**
>> + * struct virtio_net_ff_cap_mask_data - Supported selector mask formats
>> + * @count: number of entries in @selectors
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @selectors: array of supported selector descriptors
>> + */
>> +struct virtio_net_ff_cap_mask_data {
>> + __u8 count;
>> + __u8 reserved[7];
>> + struct virtio_net_ff_selector selectors[];
>> +};
>> +#define VIRTIO_NET_FF_MASK_F_PARTIAL_MASK (1 << 0)
>> +
>> +#define VIRTIO_NET_FF_ACTION_DROP 1
>> +#define VIRTIO_NET_FF_ACTION_RX_VQ 2
>> +#define VIRTIO_NET_FF_ACTION_MAX VIRTIO_NET_FF_ACTION_RX_VQ
>> +/**
>> + * struct virtio_net_ff_actions - Supported flow actions
>> + * @count: number of supported actions in @actions
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @actions: array of action identifiers (VIRTIO_NET_FF_ACTION_*)
>> + */
>> +struct virtio_net_ff_actions {
>> + __u8 count;
>> + __u8 reserved[7];
>> + __u8 actions[];
>> +};
>> +#endif
>> --
>> 2.50.1
>
Powered by blists - more mailing lists