On 11/18/25 4:06 PM, Michael S. Tsirkin wrote:
> On Tue, Nov 18, 2025 at 08:38:55AM -0600, Daniel Jurgens wrote:
>> When probing a virtnet device, attempt to read the flow filter

>> +    real_ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
>> +    sel = (void *)&ff->ff_mask->selectors[0];
> 
> why the cast? discards type checks for no good reason I can see.
> 

Jakub and Simon both requested I change the type of ff_mask->selectors
to u8 [], to eliminate sparse warnings about array of flexible
structures. So a cast is needed here.

> And &...[0] is unnecessarily funky. Just plain ff->ff_mask->selectors
> will do.
> 

Done

> 
>> +
>> +    for (i = 0; i < ff->ff_mask->count; i++) {
>> +            if (sel->length > MAX_SEL_LEN) {
>> +                    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) {
>> +                    err = -EINVAL;
>> +                    goto err_ff_action;
>> +            }
>> +            sel = (void *)sel + sizeof(*sel) + sel->length;
> 
> I guess the MAX_SEL_LEN check guarantees the allocation
> is big enough? Let's add a BUG_ON just in case.

Added BUG_ON in both error cases.

> 


>>  
>>  #ifndef _LINUX_VIRTIO_ADMIN_H
>>  #define _LINUX_VIRTIO_ADMIN_H
> 
> 
> Why do it? Let net pull this header itself.

Done

> 
> 
>> diff --git a/include/uapi/linux/virtio_net_ff.h 
>> b/include/uapi/linux/virtio_net_ff.h
>> new file mode 100644
>> index 000000000000..bd7a194a9959
>> --- /dev/null
>> +++ b/include/uapi/linux/virtio_net_ff.h
> 
> 
> you should document in commit log that you are adding
> these types to UAPI, and which spec they are from.

Done

> 
>> @@ -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;
> 
> 
> Ouch this is a problem. There is a 2 byte padding here.
> 
> This is a spec bug but I don't know if it is too late to fix.
> 
> Parav what do you think?
> 
> 
> 
> 
>> +};
>> +
>> +/**
>> + * 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];
>> +    __u8 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
> 


Reply via email to