| 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: <20251124170321-mutt-send-email-mst@kernel.org>
Date: Mon, 24 Nov 2025 17:04:38 -0500
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Daniel Jurgens <danielj@...dia.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 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"
> + * and free it if needed.
> + */
> + try_destroy_classifier(ff, c->id);
> goto err_key;
> }
>
> --
> 2.50.1
Powered by blists - more mailing lists