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 <[email protected]> >> Reviewed-by: Parav Pandit <[email protected]> >> Reviewed-by: Shahar Shitrit <[email protected]> >> Reviewed-by: Xuan Zhuo <[email protected]> >> --- >> 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 >
