Re: [RFC][PATCH 03/12] digest_lists: Basic definitions

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jun 28, 2021 at 09:27:05AM +0000, Roberto Sassu wrote:
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, June 28, 2021 10:46 AM
> > On Mon, Jun 28, 2021 at 08:30:32AM +0000, Roberto Sassu wrote:
> > > > > +struct compact_list_hdr {
> > > > > +	__u8 version;
> > > >
> > > > You should never need a version, that way lies madness.
> > >
> > > We wanted to have a way to switch to a new format, if necessary.
> > 
> > Then just add a new ioctl if you need that in the future, no need to try
> > to cram it into this one.
> 
> Given that digest lists are generated elsewhere, it would be still
> unclear when the ioctl() would be issued. Maybe the kernel needs
> to parse both v1 and v2 digest lists (I expect that v1 cannot be easily
> converted to v2, if they are signed).
> 
>  It would be also unpractical if digest lists are loaded at kernel
> initialization time (I didn't send the patch yet).

Then that is up to your api design, I do not know.  But note that
"version" fields almost always never work, so be careful about assuming
that this will solve any future issues.

> > > > > +	__le16 type;
> > > > > +	__le16 modifiers;
> > > > > +	__le16 algo;
> > > > > +	__le32 count;
> > > > > +	__le32 datalen;
> > > >
> > > > Why are user/kernel apis specified in little endian format?  Why would
> > > > that matter?  Shouldn't they just be "native" endian?
> > >
> > > I thought this would make it clear that the kernel always expects the
> > > digest lists to be in little endian.
> > 
> > Why would a big endian system expect the data from userspace to be in
> > little endian?  Shouldn't this always just be "native" endian given that
> > this is not something that is being sent to hardware?
> 
> The digest list might come from a system with different endianness.

Ok, I have no idea what digests really are used for then.  So stick with
little endian and be sure to properly convert within the kernel as
needed.

thanks,

greg k-h