[dpdk-dev] [EXT] Re: [PATCH v10 1/4] devtools: add exception for reserved fields

[Stephen Hemminger]

On Thu, 15 Apr 2021 08:31:30 +0000
Akhil Goyal <gakhil at marvell.com> wrote:

> > > > >  ; Ignore fields inserted in cacheline boundary of rte_cryptodev
> > > > >  [suppress_type]
> > > > >          name = rte_cryptodev
> > > > > -        has_data_member_inserted_between = {offset_after(attached),  
> > end}  
> > > > > \ No newline at end of file
> > > > > +        has_data_member_inserted_between = {offset_after(attached),  
> > end}  
> > > > > +
> > > > > +; Ignore changes in reserved fields
> > > > > +[suppress_variable]
> > > > > +       name_regexp = reserved  
> > > > Mm, this rule is a bit scary, as it matches anything with "reserved" in it.  
> > >
> > > Why do you feel it is scary? Reserved is something which may change at  
> > any time  
> > > Just like experimental. Hence creating a generic exception rule for it make  
> > sense  
> > > And it is done intentionally in this patch.  
> > 
> > The reserved regexp on the name of a variable / struct field is too lax.
> > Anything could be named with reserved in it.
> > If we have clear patterns, they must be preferred, like (untested)
> > name_regexp = ^reserved_(64|ptr)s$
> > 
> > 
> > Experimental is different.
> > This is a symbol version tag, which has a clear meaning and can't be
> > used for anything else.
> > 
> >   
> > >  
> > > >
> > > > You need an exception anyway to insert the new fields (like in patch 2).
> > > > Can you test your series dropping this patch 1 ?  
> > > It will not work, as there are 2 changes,
> > > 1. addition of ca_enqueue after attached. This is taken care by the  
> > exception set in patch 2  
> > > 2. change in the reserved_ptr[4] -> reserved_ptr[3]. For this change we  
> > need exception for reserved.
> > 
> > In the eventdev struct, reserved fields are all in the range between
> > the attached field and the end of the struct.
> > I pushed your series without patch 1 to a branch of mine, and it
> > passes the check fine:
> > https://github.com/david-marchand/dpdk/runs/2350324578?check_suite_focus=true#step:15:8549> 
> >   
> Yes it will work, I actually put the new field after reserved and
> it was creating issues, so I added it.
> But later I decided to move it above reserved fields and missed
> that it will work without reserved exception.
> 
> Hence we can drop the first patch for now.
> 
> Regards,
> Akhil

Is there a check that size didn't change?
For example if a reserved field was removed.