| 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
| ||
|
Message-ID: <d793f3da-17f8-489c-b48e-d1917412b87f@nvidia.com>
Date: Mon, 24 Nov 2025 16:31:54 -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 08/12] virtio_net: Use existing classifier if
possible
On 11/24/25 4:04 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 19, 2025 at 01:15:19PM -0600, Daniel Jurgens wrote:
>> Classifiers can be used by more than one rule. If there is an existing
>> classifier, use it instead of creating a new one. If duplicate
>> classifiers are created it would artifically limit the number of rules
>> to the classifier limit, which is likely less than the rules limit.
>>
>> Signed-off-by: Daniel Jurgens <danielj@...dia.com>
>> Reviewed-by: Parav Pandit <parav@...dia.com>
>> Reviewed-by: Shahar Shitrit <shshitrit@...dia.com>
>> Reviewed-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
>> ---
>> v4:
>> - Fixed typo in commit message
>> - for (int -> for (
>>
>> v8:
>> - Removed unused num_classifiers. Jason Wang
>>
>> v12:
>> - Clarified comment about destroy_classifier freeing. MST
>> - Renamed the classifier field of virtnet_classifier to obj. MST
>> - Explained why in commit message. MST
>> ---
>> ---
>> drivers/net/virtio_net.c | 51 ++++++++++++++++++++++++++--------------
>> 1 file changed, 34 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 7600e2383a72..5e49cd78904f 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -32,6 +32,7 @@
>> #include <uapi/linux/virtio_pci.h>
>> #include <uapi/linux/virtio_net_ff.h>
>> #include <linux/xarray.h>
>> +#include <linux/refcount.h>
>>
>> static int napi_weight = NAPI_POLL_WEIGHT;
>> module_param(napi_weight, int, 0444);
>> @@ -302,7 +303,6 @@ struct virtnet_ff {
>> struct virtio_net_ff_cap_mask_data *ff_mask;
>> struct virtio_net_ff_actions *ff_actions;
>> struct xarray classifiers;
>> - int num_classifiers;
>> struct virtnet_ethtool_ff ethtool;
>> };
>>
>> @@ -5816,12 +5816,13 @@ struct virtnet_ethtool_rule {
>> /* The classifier struct must be the last field in this struct */
>> struct virtnet_classifier {
>> size_t size;
>> + refcount_t refcount;
>> u32 id;
>> - struct virtio_net_resource_obj_ff_classifier classifier;
>> + struct virtio_net_resource_obj_ff_classifier obj;
>> };
>>
>> static_assert(sizeof(struct virtnet_classifier) ==
>> - ALIGN(offsetofend(struct virtnet_classifier, classifier),
>> + ALIGN(offsetofend(struct virtnet_classifier, obj),
>> __alignof__(struct virtnet_classifier)),
>> "virtnet_classifier: classifier must be the last member");
>>
>> @@ -5909,11 +5910,24 @@ static bool validate_mask(const struct virtnet_ff *ff,
>> return false;
>> }
>>
>> -static int setup_classifier(struct virtnet_ff *ff, struct virtnet_classifier *c)
>> +static int setup_classifier(struct virtnet_ff *ff,
>> + struct virtnet_classifier **c)
>> {
>> + struct virtnet_classifier *tmp;
>> + unsigned long i;
>> int err;
>>
>> - err = xa_alloc(&ff->classifiers, &c->id, c,
>> + xa_for_each(&ff->classifiers, i, tmp) {
>> + if ((*c)->size == tmp->size &&
>> + !memcmp(&tmp->obj, &(*c)->obj, tmp->size)) {
>> + refcount_inc(&tmp->refcount);
>> + kfree(*c);
>> + *c = tmp;
>> + goto out;
>> + }
>> + }
>> +
>> + err = xa_alloc(&ff->classifiers, &(*c)->id, *c,
>> XA_LIMIT(0, le32_to_cpu(ff->ff_caps->classifiers_limit) - 1),
>> GFP_KERNEL);
>> if (err)
>> @@ -5921,29 +5935,30 @@ static int setup_classifier(struct virtnet_ff *ff, struct virtnet_classifier *c)
>>
>> err = virtio_admin_obj_create(ff->vdev,
>> VIRTIO_NET_RESOURCE_OBJ_FF_CLASSIFIER,
>> - c->id,
>> + (*c)->id,
>> VIRTIO_ADMIN_GROUP_TYPE_SELF,
>> 0,
>> - &c->classifier,
>> - c->size);
>> + &(*c)->obj,
>> + (*c)->size);
>> if (err)
>> goto err_xarray;
>>
>> + refcount_set(&(*c)->refcount, 1);
>> +out:
>> return 0;
>>
>> err_xarray:
>> - xa_erase(&ff->classifiers, c->id);
>> + xa_erase(&ff->classifiers, (*c)->id);
>>
>> return err;
>> }
>>
>> -static void destroy_classifier(struct virtnet_ff *ff,
>> - u32 classifier_id)
>> +static void try_destroy_classifier(struct virtnet_ff *ff, u32 classifier_id)
>> {
>> struct virtnet_classifier *c;
>>
>> c = xa_load(&ff->classifiers, classifier_id);
>> - if (c) {
>> + if (c && refcount_dec_and_test(&c->refcount)) {
>> virtio_admin_obj_destroy(ff->vdev,
>> VIRTIO_NET_RESOURCE_OBJ_FF_CLASSIFIER,
>> c->id,
>> @@ -5967,7 +5982,7 @@ static void destroy_ethtool_rule(struct virtnet_ff *ff,
>> 0);
>>
>> xa_erase(&ff->ethtool.rules, eth_rule->flow_spec.location);
>> - destroy_classifier(ff, eth_rule->classifier_id);
>> + try_destroy_classifier(ff, eth_rule->classifier_id);
>> kfree(eth_rule);
>> }
>>
>> @@ -6139,7 +6154,7 @@ static int build_and_insert(struct virtnet_ff *ff,
>> }
>>
>> c->size = classifier_size;
>> - classifier = &c->classifier;
>> + classifier = &c->obj;
>> classifier->count = num_hdrs;
>> selector = (void *)&classifier->selectors[0];
>>
>> @@ -6149,14 +6164,16 @@ static int build_and_insert(struct virtnet_ff *ff,
>> if (err)
>> goto err_key;
>>
>> - err = setup_classifier(ff, c);
>> + err = setup_classifier(ff, &c);
>> if (err)
>> goto err_classifier;
>>
>> err = insert_rule(ff, eth_rule, c->id, key, key_size);
>> if (err) {
>> - /* destroy_classifier will free the classifier */
>> - destroy_classifier(ff, c->id);
>> + /* destroy_classifier release the reference on the classifier
>
>
> try_destroy_classifier ? and I think you mean *will* release and free.
>
> and what is "the reference"
I see the comment is munged. But classifiers are reference counted,
try_destroy_classifier will release the reference. And free if the
refcount is now 0.
See setup_classifier above.
>
>> + * and free it if needed.
>> + */
>> + try_destroy_classifier(ff, c->id);
>> goto err_key;
>> }
>>
>> --
>> 2.50.1
>
Powered by blists - more mailing lists