On Thu, 15 Apr 2021 08:31:30 +0000
Akhil Goyal <gak...@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.
