DT case sensitivity

DT case sensitivity

[Rob Herring]

The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)

What PPC systems are case insensitive? Can we limit that to certain systems?

AFAICT, dtc at least (if not anything FDT based) has always been case
sensitive at least for node and property names. I'm not sure about
compatible strings?

Anyone see potential issues with switching all platforms except PPC
and Sparc to case sensitive comparisons?

Rob

Re: DT case sensitivity

[Benjamin Herrenschmidt]

On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> The default DT string handling in the kernel is node names and
> compatibles are case insensitive and property names are case sensitive
> (Sparc is the the only variation and is opposite). It seems only PPC
> (and perhaps only Power Macs?) needs to support case insensitive
> comparisons. It was probably a mistake to follow PPC for new arches
> and we should have made everything case sensitive from the start. So I
> have a few questions for the DT historians. :)

Open Firmware itself is insensitive.

> What PPC systems are case insensitive? Can we limit that to certain systems?

All PowerMacs at least, the problem is that I don't have DT images or
access to all the historical systems (and yes some people occasionally
still use them) to properly test a change in that area.

> AFAICT, dtc at least (if not anything FDT based) has always been case
> sensitive at least for node and property names. I'm not sure about
> compatible strings?
> 
> Anyone see potential issues with switching all platforms except PPC
> and Sparc to case sensitive comparisons?

Cheers,
Ben.

Re: DT case sensitivity

[Rob Herring]

On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> > The default DT string handling in the kernel is node names and
> > compatibles are case insensitive and property names are case sensitive
> > (Sparc is the the only variation and is opposite). It seems only PPC
> > (and perhaps only Power Macs?) needs to support case insensitive
> > comparisons. It was probably a mistake to follow PPC for new arches
> > and we should have made everything case sensitive from the start. So I
> > have a few questions for the DT historians. :)
>
> Open Firmware itself is insensitive.

Doesn't it depend on the implementation? Otherwise, how is Sparc different?

> > What PPC systems are case insensitive? Can we limit that to certain systems?
>
> All PowerMacs at least, the problem is that I don't have DT images or
> access to all the historical systems (and yes some people occasionally
> still use them) to properly test a change in that area.

I'm temped to break them so I can find folks to provide me with DT dumps. :)

Rob

Re: DT case sensitivity

[Benjamin Herrenschmidt]

On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
> On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > 
> > On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> > > The default DT string handling in the kernel is node names and
> > > compatibles are case insensitive and property names are case sensitive
> > > (Sparc is the the only variation and is opposite). It seems only PPC
> > > (and perhaps only Power Macs?) needs to support case insensitive
> > > comparisons. It was probably a mistake to follow PPC for new arches
> > > and we should have made everything case sensitive from the start. So I
> > > have a few questions for the DT historians. :)
> > 
> > Open Firmware itself is insensitive.
> 
> Doesn't it depend on the implementation? Otherwise, how is Sparc different?

Not sure ... Forth itself is insensitive for words but maybe not for
string comparisons.

> 
> > > What PPC systems are case insensitive? Can we limit that to certain systems?
> > 
> > All PowerMacs at least, the problem is that I don't have DT images or
> > access to all the historical systems (and yes some people occasionally
> > still use them) to properly test a change in that area.
> 
> I'm temped to break them so I can find folks to provide me with DT dumps. :)

I have a collection of DT dumps but I'm not sure about the legality of
publishing them...

Cheers,
Ben.

Re: DT case sensitivity

[Grant Likely]

On 23/08/2018 02:29, Benjamin Herrenschmidt wrote:
> On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
>> On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
>> <benh@kernel.crashing.org> wrote:
>>>
>>> On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
>>>> The default DT string handling in the kernel is node names and
>>>> compatibles are case insensitive and property names are case sensitive
>>>> (Sparc is the the only variation and is opposite). It seems only PPC
>>>> (and perhaps only Power Macs?) needs to support case insensitive
>>>> comparisons. It was probably a mistake to follow PPC for new arches
>>>> and we should have made everything case sensitive from the start. So I
>>>> have a few questions for the DT historians. :)
>>>
>>> Open Firmware itself is insensitive.
>>
>> Doesn't it depend on the implementation? Otherwise, how is Sparc different?
>
> Not sure ... Forth itself is insensitive for words but maybe not for
> string comparisons.

What problem are you trying to solve? I would think making everything
case insensitive would be the direction to go if you do anything. Least
possibility of breaking existing platforms in that scenario.

g.

>
>>
>>>> What PPC systems are case insensitive? Can we limit that to certain systems?
>>>
>>> All PowerMacs at least, the problem is that I don't have DT images or
>>> access to all the historical systems (and yes some people occasionally
>>> still use them) to properly test a change in that area.
>>
>> I'm temped to break them so I can find folks to provide me with DT dumps. :)
>
> I have a collection of DT dumps but I'm not sure about the legality of
> publishing them...
>
> Cheers,
> Ben.
>
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: DT case sensitivity

[Rob Herring]

On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
>
> On 23/08/2018 02:29, Benjamin Herrenschmidt wrote:
> > On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
> >> On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
> >> <benh@kernel.crashing.org> wrote:
> >>>
> >>> On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> >>>> The default DT string handling in the kernel is node names and
> >>>> compatibles are case insensitive and property names are case sensitive
> >>>> (Sparc is the the only variation and is opposite). It seems only PPC
> >>>> (and perhaps only Power Macs?) needs to support case insensitive
> >>>> comparisons. It was probably a mistake to follow PPC for new arches
> >>>> and we should have made everything case sensitive from the start. So I
> >>>> have a few questions for the DT historians. :)
> >>>
> >>> Open Firmware itself is insensitive.
> >>
> >> Doesn't it depend on the implementation? Otherwise, how is Sparc different?
> >
> > Not sure ... Forth itself is insensitive for words but maybe not for
> > string comparisons.
>
> What problem are you trying to solve?

I'm looking at removing device_node.name and using full_name instead
(which now is only the local node name plus unit-address). This means
replacing of_node_cmp() (and still some strcmp) calls in a lot of
places. I need to use either strncmp or strncasecmp instead.

> I would think making everything
> case insensitive would be the direction to go if you do anything. Least
> possibility of breaking existing platforms in that scenario.

Really? Even if all the "new" arches are effectively case sensitive?
Anything using dtc and libfdt are (and json-schema certainly will be).
But I frequently say the kernel's job is not DT validation, so you
pass crap in, you get undefined results.

Rob

Re: DT case sensitivity

[Benjamin Herrenschmidt]

On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
> On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
> > 
> > 
> > What problem are you trying to solve?
> 
> I'm looking at removing device_node.name and using full_name instead
> (which now is only the local node name plus unit-address). This means
> replacing of_node_cmp() (and still some strcmp) calls in a lot of
> places. I need to use either strncmp or strncasecmp instead.
> 
> > I would think making everything
> > case insensitive would be the direction to go if you do anything. Least
> > possibility of breaking existing platforms in that scenario.
> 
> Really? Even if all the "new" arches are effectively case sensitive?
> Anything using dtc and libfdt are (and json-schema certainly will be).
> But I frequently say the kernel's job is not DT validation, so you
> pass crap in, you get undefined results.

I tend to agree with Grant. Let's put it this way:

What is the drawback of being case insensitive ?

Do we expect that there exist a case where we will want to distinguish
between nodes that have the same name with a different case ?

If not, I don't see the point of being strict about it.

Cheers,
Ben.

Re: DT case sensitivity

[Grant Likely]

On 23/08/2018 12:47, Benjamin Herrenschmidt wrote:
> On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
>> On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
>>>
>>>
>>> What problem are you trying to solve?
>>
>> I'm looking at removing device_node.name and using full_name instead
>> (which now is only the local node name plus unit-address). This means
>> replacing of_node_cmp() (and still some strcmp) calls in a lot of
>> places. I need to use either strncmp or strncasecmp instead.
Makes sense. Simplifies the code.

>>
>>> I would think making everything
>>> case insensitive would be the direction to go if you do anything. Least
>>> possibility of breaking existing platforms in that scenario.
>>
>> Really? Even if all the "new" arches are effectively case sensitive?
>> Anything using dtc and libfdt are (and json-schema certainly will be).
>> But I frequently say the kernel's job is not DT validation, so you
>> pass crap in, you get undefined results.
>
> I tend to agree with Grant. Let's put it this way:
>
> What is the drawback of being case insensitive ?
>
> Do we expect that there exist a case where we will want to distinguish
> between nodes that have the same name with a different case ? >
> If not, I don't see the point of being strict about it.

I'd also add that any place where there are two nodes or props with the
same name, but different case, then it is probably a bug (or a really
bad design). Going the direction of case-insensitive eliminates the
possibility.

I do understand that that you'd like the kernel to be strict about what
it accepts, but that strictness probably makes more sense back in DTC
where it can also be checked against schema.

g.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: DT case sensitivity

[Rob Herring]

On Thu, Aug 23, 2018 at 6:48 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
> > On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
> > >
> > >
> > > What problem are you trying to solve?
> >
> > I'm looking at removing device_node.name and using full_name instead
> > (which now is only the local node name plus unit-address). This means
> > replacing of_node_cmp() (and still some strcmp) calls in a lot of
> > places. I need to use either strncmp or strncasecmp instead.
> >
> > > I would think making everything
> > > case insensitive would be the direction to go if you do anything. Least
> > > possibility of breaking existing platforms in that scenario.
> >
> > Really? Even if all the "new" arches are effectively case sensitive?
> > Anything using dtc and libfdt are (and json-schema certainly will be).
> > But I frequently say the kernel's job is not DT validation, so you
> > pass crap in, you get undefined results.
>
> I tend to agree with Grant. Let's put it this way:
>
> What is the drawback of being case insensitive ?

It's a more expensive comparison. I don't think it's a hot path, but
we do do a lot of string compares.

Property names are case sensitive already. It would be nice if
everything was treated the same way.

If we're case sensitive then it is well defined which node we'll match
and which one will be ignored. Otherwise, it is whichever one we
happen to match first.

> Do we expect that there exist a case where we will want to distinguish
> between nodes that have the same name with a different case ?

If someone has a DT with a node in the wrong case (as defined in the
DT spec or a binding doc) and the kernel accepts it, then it's an ABI.
Yes, as Grant says, validation is the place that should catch it, but
we're not there yet and it's cheap (cheaper in fact) for the kernel to
do.

Rob

Re: DT case sensitivity

[Grant Likely]

On 23/08/2018 13:08, Rob Herring wrote:
> On Thu, Aug 23, 2018 at 6:48 AM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>>
>> On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
>>> On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
>>>>
>>>>
>>>> What problem are you trying to solve?
>>>
>>> I'm looking at removing device_node.name and using full_name instead
>>> (which now is only the local node name plus unit-address). This means
>>> replacing of_node_cmp() (and still some strcmp) calls in a lot of
>>> places. I need to use either strncmp or strncasecmp instead.
>>>
>>>> I would think making everything
>>>> case insensitive would be the direction to go if you do anything. Least
>>>> possibility of breaking existing platforms in that scenario.
>>>
>>> Really? Even if all the "new" arches are effectively case sensitive?
>>> Anything using dtc and libfdt are (and json-schema certainly will be).
>>> But I frequently say the kernel's job is not DT validation, so you
>>> pass crap in, you get undefined results.
>>
>> I tend to agree with Grant. Let's put it this way:
>>
>> What is the drawback of being case insensitive ?
>
> It's a more expensive comparison. I don't think it's a hot path, but
> we do do a lot of string compares.

I'd hazard to argue that the cost of the string compare will not be a
source of performance problems when compared to doing a linear search
everytime a DT lookup is performed. The search algorithm is far more
problematic.

> Property names are case sensitive already. It would be nice if
> everything was treated the same way.
>
> If we're case sensitive then it is well defined which node we'll match
> and which one will be ignored. Otherwise, it is whichever one we
> happen to match first.

If case-insensitive-always is chosen, then the kernel should probably
complain at add node time if another matching node name already exists.
That's a relatively easy thing to test.

You are right however that in the FDT world we're case-sensitive now and
if it is relaxed then we're never going back. You could try switching
everyone over to case-sensitive and see if anything falls out in
linux-next for a few cycles. I would not touch the PPC or SPARC
behaviour. I don't think it is worth the risk to make the kernel more
strict when we cannot test the result. Only if everything was changed to
case-insensitive would it make sense to touch PPC and SPARC at the same
time because it reduces the number of platform variations.

If you do change compatible matches to be case sensitive, then you
should be prepared to fix bugs where the driver uses a different case
from the DTS. In which case drivers will need to be modified to accept
the deviant property names.

g.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: DT case sensitivity

[Segher Boessenkool]

On Thu, Aug 23, 2018 at 11:29:01AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
> > On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
> > <benh@kernel.crashing.org> wrote:
> > > 
> > > On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> > > > The default DT string handling in the kernel is node names and
> > > > compatibles are case insensitive and property names are case sensitive
> > > > (Sparc is the the only variation and is opposite). It seems only PPC
> > > > (and perhaps only Power Macs?) needs to support case insensitive
> > > > comparisons. It was probably a mistake to follow PPC for new arches
> > > > and we should have made everything case sensitive from the start. So I
> > > > have a few questions for the DT historians. :)
> > > 
> > > Open Firmware itself is insensitive.
> > 
> > Doesn't it depend on the implementation? Otherwise, how is Sparc different?
> 
> Not sure ...

The standard requires case-sensitive.

> Forth itself is insensitive for words

Not even.   http://forth.sourceforge.net/std/dpans/dpans3.htm#3.3.1.2

(Most non-ancient implementations are though).

> but maybe not for string comparisons.

Only COMPARE is standardised, and that is case-sensitive comparison.  Many
systems have other words to do case-insensitive comparisons, or words where
some runtime flag determines the case-sensitivity.

Btw.  A node name in Open Firmware is generically
  driver-name@unit-address:device-arguments
where driver-name is the part that is in the "name" property; this whole
case-sensitivity business is even worse for FDT, where you also treat the
unit address as part of the name.  In real Open Firmware the address is
compared *as a number* (or as a few numbers), so it is naturally case-
insensitive (it does not care if you write 01a0 or 01A0, or 1a0 or 000001a0
etc.)


Segher

Re: DT case sensitivity

[Rob Herring]

On Thu, Aug 23, 2018 at 7:37 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Thu, Aug 23, 2018 at 11:29:01AM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
> > > On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
> > > <benh@kernel.crashing.org> wrote:
> > > >
> > > > On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> > > > > The default DT string handling in the kernel is node names and
> > > > > compatibles are case insensitive and property names are case sensitive
> > > > > (Sparc is the the only variation and is opposite). It seems only PPC
> > > > > (and perhaps only Power Macs?) needs to support case insensitive
> > > > > comparisons. It was probably a mistake to follow PPC for new arches
> > > > > and we should have made everything case sensitive from the start. So I
> > > > > have a few questions for the DT historians. :)
> > > >
> > > > Open Firmware itself is insensitive.
> > >
> > > Doesn't it depend on the implementation? Otherwise, how is Sparc different?
> >
> > Not sure ...
>
> The standard requires case-sensitive.
>
> > Forth itself is insensitive for words
>
> Not even.   http://forth.sourceforge.net/std/dpans/dpans3.htm#3.3.1.2
>
> (Most non-ancient implementations are though).
>
> > but maybe not for string comparisons.
>
> Only COMPARE is standardised, and that is case-sensitive comparison.  Many
> systems have other words to do case-insensitive comparisons, or words where
> some runtime flag determines the case-sensitivity.
>
> Btw.  A node name in Open Firmware is generically
>   driver-name@unit-address:device-arguments
> where driver-name is the part that is in the "name" property; this whole
> case-sensitivity business is even worse for FDT, where you also treat the
> unit address as part of the name.  In real Open Firmware the address is
> compared *as a number* (or as a few numbers), so it is naturally case-
> insensitive (it does not care if you write 01a0 or 01A0, or 1a0 or 000001a0
> etc.)

True, but it is very rare at least in Linux to look at the
unit-addresses. dtc now does some checking on unit-addresses too, so
we should at least be consistent.

Another question: Is there ever a case where the node name in the path
aka 'driver-name' doesn't match the 'name' property? I think this
generally can't happen on FDT as the 'name' property is generated when
we unflatten it though I suppose one could craft an FDT with name
properties.

There's also various places in the kernel that check for a NULL name
which doesn't seem like it could happen either other than the root
node.

Rob

Re: DT case sensitivity

[Segher Boessenkool]

On Fri, Aug 24, 2018 at 10:14:01AM -0500, Rob Herring wrote:
> Another question: Is there ever a case where the node name in the path
> aka 'driver-name' doesn't match the 'name' property? I think this
> generally can't happen on FDT as the 'name' property is generated when
> we unflatten it though I suppose one could craft an FDT with name
> properties.

In Open Firmware, it *is* the "name" property :-)

> There's also various places in the kernel that check for a NULL name
> which doesn't seem like it could happen either other than the root
> node.

In Open Firmware the root node is required to have a "name" property, too.
There are systems that violate that rule (as with most rules...)


Segher

Re: DT case sensitivity

[Segher Boessenkool]

On Thu, Aug 23, 2018 at 11:03:28AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> > The default DT string handling in the kernel is node names and
> > compatibles are case insensitive and property names are case sensitive
> > (Sparc is the the only variation and is opposite). It seems only PPC
> > (and perhaps only Power Macs?) needs to support case insensitive
> > comparisons. It was probably a mistake to follow PPC for new arches
> > and we should have made everything case sensitive from the start. So I
> > have a few questions for the DT historians. :)
> 
> Open Firmware itself is insensitive.

Some implementations are, yes.  But those are technically broken.

> > What PPC systems are case insensitive? Can we limit that to certain systems?
> 
> All PowerMacs at least, the problem is that I don't have DT images or
> access to all the historical systems (and yes some people occasionally
> still use them) to properly test a change in that area.

If one implementation does case insensitive, it will most likely just work,
because people do not make insane names differing only in case on purpose.
Now people write other things that they only test against that implementation,
and those things now only work with case-insensitive.  And you do not know
without testing if anything breaks.  (But the laws of big numbers are against
you here).  Since there isn't really a drawback to doing case-insensitive
always, that is a much safer way forward, much less work for everyone, even
if technically the wrong thing to do :-)


Segher

Re: DT case sensitivity

[Benjamin Herrenschmidt]

On Thu, 2018-08-23 at 07:19 -0500, Segher Boessenkool wrote:
> If one implementation does case insensitive, it will most likely just work,
> because people do not make insane names differing only in case on purpose.

Apple did :-)

ide, vs IDE, ata vs ATA, I've seen all sort of crap there, esp. on old
machines.

> Now people write other things that they only test against that implementation,
> and those things now only work with case-insensitive.  And you do not know
> without testing if anything breaks.  (But the laws of big numbers are against
> you here).  Since there isn't really a drawback to doing case-insensitive
> always, that is a much safer way forward, much less work for everyone, even
> if technically the wrong thing to do :-)
> 
> 
> Segher

Re: DT case sensitivity

[Michael Ellerman]

Rob Herring <robh@kernel.org> writes:

> The default DT string handling in the kernel is node names and
> compatibles are case insensitive and property names are case sensitive
> (Sparc is the the only variation and is opposite). It seems only PPC
> (and perhaps only Power Macs?) needs to support case insensitive
> comparisons. It was probably a mistake to follow PPC for new arches
> and we should have made everything case sensitive from the start. So I
> have a few questions for the DT historians. :)
>
> What PPC systems are case insensitive? Can we limit that to certain systems?
>
> AFAICT, dtc at least (if not anything FDT based) has always been case
> sensitive at least for node and property names. I'm not sure about
> compatible strings?
>
> Anyone see potential issues with switching all platforms except PPC
> and Sparc to case sensitive comparisons?

Looking at some old device trees, on ppc we do definitely have mixed
case compatible strings, and I see a few hundred of those appear in the
kernel source. So I think for us it's too late.

But I don't see why that prevents other arches from switching to case
sensitive, I don't think it really hurts us in any way.

cheers