On Mon, Nov 24, 2025 at 04:31:54PM -0600, Dan Jurgens wrote: > 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.
ah I got it. you mean refcount - the reference count - not "the reference". And in the context of refcount_t there's decrement and increment, not "release". acquire/release in fact refer to memory ordering. > > > >> + * and free it if needed. > >> + */ > >> + try_destroy_classifier(ff, c->id); > >> goto err_key; > >> } > >> > >> -- > >> 2.50.1 > >
