Re: [PATCH net-next v11 08/12] virtio_net: Use existing classifier if possible

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Nov 19, 2025 at 09:45:15AM -0600, Dan Jurgens wrote:
> 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"?

Ugh ... donnu how this got here.

> I did this, also updated the
> commit messages and included refcount.h.

thanks.

> > 
> >>>
> >>>
> >>>
> >