On Thu, Jul 09, 2026 at 04:37:11PM +0300, Shahar Shitrit wrote:
>
>
> On 08/02/2026 13:51, Michael S. Tsirkin wrote:
> > On Thu, Feb 05, 2026 at 04:47:00PM -0600, Daniel Jurgens wrote:
> >> When probing a virtnet device, attempt to read the flow filter
> >> capabilities. In order to use the feature the caps must also
> >> be set. For now setting what was read is sufficient.
> >>
> >> This patch adds uapi definitions virtio_net flow filters define in
> >> version 1.4 of the VirtIO spec.
> >>
> >> Signed-off-by: Daniel Jurgens <[email protected]>
> >> Reviewed-by: Parav Pandit <[email protected]>
> >> Reviewed-by: Shahar Shitrit <[email protected]>
> >>
> >> ---
> >> v4:
> >> - Validate the length in the selector caps
> >> - Removed __free usage.
> >> - Removed for(int.
> >> v5:
> >> - Remove unneed () after MAX_SEL_LEN macro (test bot)
> >> v6:
> >> - Fix sparse warning "array of flexible structures" Jakub K/Simon H
> >> - Use new variable and validate ff_mask_size before set_cap. MST
> >> v7:
> >> - Set ff->ff_{caps, mask, actions} NULL in error path. Paolo Abeni
> >> - Return errors from virtnet_ff_init, -ENOTSUPP is not fatal. Xuan
> >>
> >> v8:
> >> - Use real_ff_mask_size when setting the selector caps. Jason Wang
> >>
> >> v9:
> >> - Set err after failed memory allocations. Simon Horman
> >>
> >> v10:
> >> - Return -EOPNOTSUPP in virnet_ff_init before allocing any memory.
> >> Jason/Paolo.
> >>
> >> v11:
> >> - Return -EINVAL if any resource limit is 0. Simon Horman
> >> - Ensure we don't overrun alloced space of ff->ff_mask by moving the
> >> real_ff_mask_size > ff_mask_size check into the loop. Simon Horman
> >>
> >> v12:
> >> - Move uapi includes to virtio_net.c vs header file. MST
> >> - Remove kernel.h header in virtio_net_ff uapi. MST
> >> - WARN_ON_ONCE in error paths validating selectors. MST
> >> - Move includes from .h to .c files. MST
> >> - Add WARN_ON_ONCE if obj_destroy fails. MST
> >> - Comment cleanup in virito_net_ff.h uapi. MST
> >> - Add 2 byte pad to the end of virtio_net_ff_cap_data.
> >>
> >> https://lore.kernel.org/virtio-comment/[email protected]/T/#m930988a5d3db316c68546d8b61f4b94f6ebda030
> >> - Cleanup and reinit in the freeze/restore path. MST
> >>
> >> v13:
> >> - Added /* private: */ comment before reserved field. Jakub
> >> - Change ff_mask validation to break at unkonwn selector type. This
> >> will allow compatability with newer controllers if the types of
> >> selectors is expanded. MST
> >>
> >> v14:
> >> - Handle err from virtnet_ff_init in virtnet_restore_up. MST
> >>
> >> v15:
> >> - In virtnet_restore_up only call virtnet_close in err path if
> >> netif_runnig. AI
> >>
> >> v16:
> >> - Return 0 from virtnet_restore_up if virtnet_init_ff return not
> >> supported. AI
> >>
> >> v17:
> >> - During restore freeze_down on error during ff_init. AI
> >>
> >> v18:
> >> - Changed selector cap validation to verify size for each type
> >> instead of just checking they weren't bigger than max size. AI
> >> - Added __count_by attribute to flexible members in uapi. Paolo A
> >>
> >> v19:
> >> - Fixed ;; and incorrect plural in comment. AI
> >>
> >> v20:
> >> - include uapi/linux/stddef.h for __counted_by. AI
> >
> > AI has led you astray, sadly (
> will fix in v21
> >
> >
> >
> >
> >> ---
> >> drivers/net/virtio_net.c | 231 ++++++++++++++++++++++++++++-
> >> include/uapi/linux/virtio_net_ff.h | 91 ++++++++++++
> >> 2 files changed, 321 insertions(+), 1 deletion(-)
> >> create mode 100644 include/uapi/linux/virtio_net_ff.h
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index db88dcaefb20..2cfa37e2f83f 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -26,6 +26,11 @@
> >> #include <net/netdev_rx_queue.h>
> >> #include <net/netdev_queues.h>
> >> #include <net/xdp_sock_drv.h>
> >> +#include <linux/virtio_admin.h>
> >> +#include <net/ipv6.h>
> >> +#include <net/ip.h>
> >> +#include <uapi/linux/virtio_pci.h>
> >> +#include <uapi/linux/virtio_net_ff.h>
> >>
> >> static int napi_weight = NAPI_POLL_WEIGHT;
> >> module_param(napi_weight, int, 0444);
> >> @@ -281,6 +286,14 @@ static const struct virtnet_stat_desc
> >> virtnet_stats_tx_speed_desc_qstat[] = {
> >> VIRTNET_STATS_DESC_TX_QSTAT(speed, ratelimit_packets,
> >> hw_drop_ratelimits),
> >> };
> >>
> >> +struct virtnet_ff {
> >> + struct virtio_device *vdev;
> >> + bool ff_supported;
> >> + struct virtio_net_ff_cap_data *ff_caps;
> >> + struct virtio_net_ff_cap_mask_data *ff_mask;
> >> + struct virtio_net_ff_actions *ff_actions;
> >> +};
> >> +
> >> #define VIRTNET_Q_TYPE_RX 0
> >> #define VIRTNET_Q_TYPE_TX 1
> >> #define VIRTNET_Q_TYPE_CQ 2
> >> @@ -488,6 +501,7 @@ struct virtnet_info {
> >> TRAILING_OVERLAP(struct virtio_net_rss_config_trailer, rss_trailer,
> >> hash_key_data,
> >> u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> >> );
> >> + struct virtnet_ff ff;
> >> };
> >> static_assert(offsetof(struct virtnet_info, rss_trailer.hash_key_data) ==
> >> offsetof(struct virtnet_info, rss_hash_key_data));
> >> @@ -526,6 +540,7 @@ static struct sk_buff *virtnet_skb_append_frag(struct
> >> sk_buff *head_skb,
> >> struct page *page, void *buf,
> >> int len, int truesize);
> >> static void virtnet_xsk_completed(struct send_queue *sq, int num);
> >> +static void remove_vq_common(struct virtnet_info *vi);
> >>
> >> enum virtnet_xmit_type {
> >> VIRTNET_XMIT_TYPE_SKB,
> >> @@ -5684,6 +5699,192 @@ static const struct netdev_stat_ops
> >> virtnet_stat_ops = {
> >> .get_base_stats = virtnet_get_base_stats,
> >> };
> >>
> >> +static size_t get_mask_size(u16 type)
> >> +{
> >> + switch (type) {
> >> + case VIRTIO_NET_FF_MASK_TYPE_ETH:
> >> + return sizeof(struct ethhdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_IPV4:
> >> + return sizeof(struct iphdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_IPV6:
> >> + return sizeof(struct ipv6hdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_TCP:
> >> + return sizeof(struct tcphdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_UDP:
> >> + return sizeof(struct udphdr);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +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_admin_cmd_query_cap_id_result *cap_id_list;
> >> + struct virtio_net_ff_selector *sel;
> >> + unsigned long sel_types = 0;
> >> + size_t real_ff_mask_size;
> >> + int err;
> >> + int i;
> >> +
> >> + if (!vdev->config->admin_cmd_exec)
> >> + return -EOPNOTSUPP;
> >> +
> >> + cap_id_list = kzalloc(sizeof(*cap_id_list), GFP_KERNEL);
> >> + if (!cap_id_list)
> >> + return -ENOMEM;
> >> +
> >> + err = virtio_admin_cap_id_list_query(vdev, cap_id_list);
> >> + if (err)
> >> + goto err_cap_list;
> >> +
> >> + if (!(VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_RESOURCE_CAP) &&
> >> + VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_SELECTOR_CAP) &&
> >> + VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_ACTION_CAP))) {
> >> + err = -EOPNOTSUPP;
> >> + goto err_cap_list;
> >> + }
> >> +
> >> + ff->ff_caps = kzalloc(sizeof(*ff->ff_caps), GFP_KERNEL);
> >> + if (!ff->ff_caps) {
> >> + err = -ENOMEM;
> >> + goto err_cap_list;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_RESOURCE_CAP,
> >> + ff->ff_caps,
> >> + sizeof(*ff->ff_caps));
> >> +
> >> + if (err)
> >> + goto err_ff;
> >> +
> >> + if (!ff->ff_caps->groups_limit ||
> >> + !ff->ff_caps->classifiers_limit ||
> >> + !ff->ff_caps->rules_limit ||
> >> + !ff->ff_caps->rules_per_group_limit) {
> >> + err = -EINVAL;
> >> + goto err_ff;
> >> + }
> >> +
> >> + /* 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);
> >> +
> >> + ff->ff_mask = kzalloc(ff_mask_size, GFP_KERNEL);
> >> + if (!ff->ff_mask) {
> >> + err = -ENOMEM;
> >> + goto err_ff;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_SELECTOR_CAP,
> >> + ff->ff_mask,
> >> + ff_mask_size);
> >
> > So ff_actions is from device and ff_actions->count does not seem to be
> > checked.
> >
> > If device somehow gains a larger mask down the road, can it not then
> > overflow?
> > or malicious?
> see below
> >
> >
> >> +
> >> + if (err)
> >> + goto err_ff_mask;
> >> +
> >> + ff->ff_actions = kzalloc(sizeof(*ff->ff_actions) +
> >> + VIRTIO_NET_FF_ACTION_MAX,
> >> + GFP_KERNEL);
> >> + if (!ff->ff_actions) {
> >> + err = -ENOMEM;
> >> + goto err_ff_mask;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_ACTION_CAP,
> >> + ff->ff_actions,
> >> + sizeof(*ff->ff_actions) +
> >> VIRTIO_NET_FF_ACTION_MAX);
> >
> > So ff_actions is from device and ff_actions->count is not checked.
> >
> > If device gains a ton of actions down the road, can it not then overflow?
> > or malicious?
>
> it can't overflow, as it can write up to sizeof(*ff->ff_actions) +
> VIRTIO_NET_FF_ACTION_MAX bytes. But it is a valid concern in case of
> someone in the future iterates on count. Better to handle it now so it
> won't be forgotten. Checking that ff_actions->count doesn't exceed
> VIRTIO_NET_FF_ACTION_MAX can break backward compatibility.
These are admin commands right? They are truncated silently.
All you have to do is just check the length returned.
At least that's my vague understanding, don't have the
time to re-review now. HTH
> If this max
> value is extended in a new spec/controller then this driver would fail
> to load. Instead, I will assign ff->ff_actions->count with the minimum
> of what we got and the max.
> >
> >> +
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_RESOURCE_CAP,
> >> + ff->ff_caps,
> >> + sizeof(*ff->ff_caps));
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + real_ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
> >> + sel = (void *)&ff->ff_mask->selectors;
> >> +
> >> + for (i = 0; i < ff->ff_mask->count; i++) {
> >> + /* If the selector type is unknown it may indicate the spec
> >> + * has been revised to include new types of selectors
> >> + */
> >> + if (sel->type > VIRTIO_NET_FF_MASK_TYPE_MAX)
> >
> > do you want to check sel->type 0 too?
> yes, will add in v21.
> >
> >> + break;
> >
> > but count remains unchanged? should we not to reduce count here
> > so device knows what driver can drive?
>
> we should. will add it in v21.
> >
> >
> >> +
> >> + if (sel->length != get_mask_size(sel->type) ||
> >> + test_and_set_bit(sel->type, &sel_types)) {
> >> + WARN_ON_ONCE(true);
> >> + err = -EINVAL;
> >> + goto err_ff_action;
> >> + }
> >> + real_ff_mask_size += sizeof(struct virtio_net_ff_selector) +
> >> sel->length;
> >> + if (real_ff_mask_size > ff_mask_size) {
> >> + WARN_ON_ONCE(true);
> >> + err = -EINVAL;
> >> + goto err_ff_action;
> >> + }
> >> + sel = (void *)sel + sizeof(*sel) + sel->length;
> >> + }
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_SELECTOR_CAP,
> >> + ff->ff_mask,
> >> + real_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 0;
> >> +
> >> +err_ff_action:
> >> + kfree(ff->ff_actions);
> >> + ff->ff_actions = NULL;
> >> +err_ff_mask:
> >> + kfree(ff->ff_mask);
> >> + ff->ff_mask = NULL;
> >> +err_ff:
> >> + kfree(ff->ff_caps);
> >> + ff->ff_caps = NULL;
> >> +err_cap_list:
> >> + kfree(cap_id_list);
> >> +
> >> + return err;
> >> +}
> >> +
> >> +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);
> >> + ff->ff_supported = false;
> >> +}
> >> +
> >> static void virtnet_freeze_down(struct virtio_device *vdev)
> >> {
> >> struct virtnet_info *vi = vdev->priv;
> >> @@ -5702,6 +5903,10 @@ static void virtnet_freeze_down(struct
> >> virtio_device *vdev)
> >> netif_tx_lock_bh(vi->dev);
> >> netif_device_detach(vi->dev);
> >> netif_tx_unlock_bh(vi->dev);
> >> +
> >> + rtnl_lock();
> >> + virtnet_ff_cleanup(&vi->ff);
> >> + rtnl_unlock();
> >> }
> >>
> >> static int init_vqs(struct virtnet_info *vi);
> >> @@ -5727,10 +5932,23 @@ static int virtnet_restore_up(struct virtio_device
> >> *vdev)
> >> return err;
> >> }
> >>
> >> + /* Initialize flow filters. Not supported is an acceptable and common
> >> + * return code
> >> + */
> >> + rtnl_lock();
> >> + err = virtnet_ff_init(&vi->ff, vi->vdev);
> >> + if (err && err != -EOPNOTSUPP) {
> >> + rtnl_unlock();
> >> + virtnet_freeze_down(vi->vdev);
> >> + remove_vq_common(vi);
> >> + return err;
> >> + }
> >> + rtnl_unlock();
> >> +
> >> netif_tx_lock_bh(vi->dev);
> >> netif_device_attach(vi->dev);
> >> netif_tx_unlock_bh(vi->dev);
> >> - return err;
> >> + return 0;
> >> }
> >>
> >> static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64
> >> offloads)
> >> @@ -7058,6 +7276,15 @@ static int virtnet_probe(struct virtio_device *vdev)
> >> }
> >> vi->guest_offloads_capable = vi->guest_offloads;
> >>
> >> + /* Initialize flow filters. Not supported is an acceptable and common
> >> + * return code
> >> + */
> >> + err = virtnet_ff_init(&vi->ff, vi->vdev);
> >> + if (err && err != -EOPNOTSUPP) {
> >> + rtnl_unlock();
> >> + goto free_unregister_netdev;
> >> + }
> >> +
> >> rtnl_unlock();
> >>
> >> err = virtnet_cpu_notif_add(vi);
> >> @@ -7073,6 +7300,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:
> >> @@ -7121,6 +7349,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/uapi/linux/virtio_net_ff.h
> >> b/include/uapi/linux/virtio_net_ff.h
> >> new file mode 100644
> >> index 000000000000..552a6b3a8a91
> >> --- /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 <uapi/linux/stddef.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
> >> + */
> >> +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;
> >> + /* private: */
> >> + __u8 reserved[2];
> >> +};
> >> +
> >> +/**
> >> + * 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[] __counted_by(length);
> >> +};
> >> +
> >> +#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: packed array of struct virtio_net_ff_selector.
> >> + */
> >> +struct virtio_net_ff_cap_mask_data {
> >> + __u8 count;
> >> + __u8 reserved[7];
> >> + __u8 selectors[] __counted_by(count);
> >
> > This looks wrong to me. count is # of selectors (packed entries) not
> > bytes.
> right, will remove it.
> > >
> >
> >
> >> +};
> >> +
> >> +#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[] __counted_by(count);
> >
> >
> > this too.
> count directly represents the number of __u8 elements in actions[], so
> the size of the flexible array is count * sizeof(__u8). Therefore, here
> it is correct.
> >
> >> +};
> >> +#endif
> >> --
> >> 2.50.1
> >
> >
