[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c88f7798-996e-402c-97fe-1f8cc8ec172c@nvidia.com>
Date: Wed, 19 Nov 2025 09:45:15 -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 v11 08/12] virtio_net: Use existing classifier if
possible
On 11/19/25 1:41 AM, Michael S. Tsirkin wrote:
> On Wed, Nov 19, 2025 at 01:33:31AM -0600, Dan Jurgens wrote:
>> On 11/19/25 1:23 AM, Michael S. Tsirkin wrote:
>>> On Wed, Nov 19, 2025 at 01:18:56AM -0600, Dan Jurgens wrote:
>>>> 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.
>>>
>>> I might be missing something here, but are not the
>>> structures this code compares of the type struct virtnet_classifier
>>> not virtio_net_ff_selector ?
>>>
>>> and that one is:
>>>
>>> struct virtnet_classifier {
>>> size_t size;
>>> + refcount_t refcount;
>>> u32 id;
>>> struct virtio_net_resource_obj_ff_classifier classifier;
>>> };
>>>
>>>
>>> which seems to have some padding depending on the architecture.
>>
>> We're only comparing the ->classifier part of that, which is pad free.
>
> Oh I see a classifier has a classifer inside :(
>
> Should be something else, e.g. ff_classifier to avoid confusion I think.
>
> Or resource_obj since it's the resource object. Or even obj.
>
> But
>
Did you have more to say after that "But"? I did this, also updated the
commit messages and included refcount.h.
>
>>>
>>>
>>>
>
Powered by blists - more mailing lists