On 11/19/25 12:35 AM, Michael S. Tsirkin wrote:
> On Wed, Nov 19, 2025 at 12:26:23AM -0600, Dan Jurgens wrote:
>> On 11/18/25 3:55 PM, Michael S. Tsirkin wrote:
>>> On Tue, Nov 18, 2025 at 08:38:58AM -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.
>>
>>>> + 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->classifier, &(*c)->classifier, tmp->size)) {
>>>
>>> note that classifier has padding bytes.
>>> comparing these with memcmp is not safe, is it?
>>
>> The reserved bytes are set to 0, this is fine.
>
> I mean the compiler padding. set to 0 where?
There's no compiler padding in virtio_net_ff_selector. There are
reserved fields between the count and selector array.
>
>>>
>>>
>>>> + 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)
>>>
>>> what kind of locking prevents two threads racing in this code?
>>
>> The ethtool calls happen under rtnl_lock.
>>
>>>
>>>
>>>> @@ -6932,29 +6945,30 @@ static int setup_classifier(struct virtnet_ff *ff,
>>>> struct virtnet_classifier *c)
>>>> (*c)->size);
>>>> if (err)
>>>> goto err_xarray;
>>>>
>>>> + refcount_set(&(*c)->refcount, 1);
>>>
>>>
>>> so you insert uninitialized refcount? can't another thread find it
>>> meanwhile?
>>
>> Again, rtnl_lock.
>>
>>
>>>>
>>>> err = insert_rule(ff, eth_rule, c->id, key, key_size);
>>>> if (err) {
>>>> /* destroy_classifier will free the classifier */
>>>
>>> will free is no longer correct, is it?
>>
>> Clarified the comment.
>>
>>>
>>>> - destroy_classifier(ff, c->id);
>>>> + try_destroy_classifier(ff, c->id);
>>>> goto err_key;
>>>> }
>>>>
>>>> --
>>>> 2.50.1
>>>
>
