devmodelcommon.proto: Add types for CAN devices

[rene]

devmodelcommon.proto: Add types for CAN devices

Extended the API to support CAN Devices on EVE. The proposal is described
and discussed at LFEdge's EVE Wiki:

https://wiki.lfedge.org/display/EVE/CAN+Bus+support

Three new members are added to PhyIoType:

• PHY_IO_TYPE_PHY_IO_CAN
• PHY_IO_TYPE_PHY_IO_VCAN
• PHY_IO_TYPE_PHY_IO_LCAN

Signed-off-by: Renê de Souza Pinto [email protected]

[eriknordmark]

We should ignore the buflint complaints about using PHY_IO_TYPE_ prefix etc since it makes more sense to follow the style for the existing enum values.

But a question.
Would a controller (and associated UI etc) want to know whether a device understands these new enum values before sending them in the API?
If so we should also add a new value in the API Capabilities enum in the API for CAN support.
But if we don't think the controller cares then that isn't needed.

[rene]

We should ignore the buflint complaints about using PHY_IO_TYPE_ prefix etc since it makes more sense to follow the style for the existing enum values.

But a question. Would a controller (and associated UI etc) want to know whether a device understands these new enum values before sending them in the API? If so we should also add a new value in the API Capabilities enum in the API for CAN support. But if we don't think the controller cares then that isn't needed.

@eriknordmark , AFAU receiving a non supported I/O type is harmless because pillar will not perform any special handling of it... From what I see in the API we don't treat every single supported I/O type in capabilities, so I think we don't need any special treatment for CAN devices... but I don't have the full picture of the API, so I'm totally OK to accept the approach you and others point here as more adequate for this particular case...

[eriknordmark]

@eriknordmark , AFAU receiving a non supported I/O type is harmless because pillar will not perform any special handling of it... From what I see in the API we don't treat every single supported I/O type in capabilities, so I think we don't need any special treatment for CAN devices... but I don't have the full picture of the API, so I'm totally OK to accept the approach you and others point here as more adequate for this particular case...

Does the EVE code not care if the I/O type is 17 (some undefined value) and still look at the pci addresses etc? If so we don't need a API capability.

[rene]

@eriknordmark , AFAU receiving a non supported I/O type is harmless because pillar will not perform any special handling of it... From what I see in the API we don't treat every single supported I/O type in capabilities, so I think we don't need any special treatment for CAN devices... but I don't have the full picture of the API, so I'm totally OK to accept the approach you and others point here as more adequate for this particular case...

Does the EVE code not care if the I/O type is 17 (some undefined value) and still look at the pci addresses etc? If so we don't need a API capability.

I looked at domain manager's code and found basically two main functions that could potential fail due to an unknown IoType:
https://github.com/lf-edge/eve/blob/master/pkg/pillar/cmd/domainmgr/domainmgr.go#L2763 and
https://github.com/lf-edge/eve/blob/master/pkg/pillar/types/assignableadapters.go#L186

In the latter, there is this conversion ib.Type = IoType(phyAdapter.Ptype) , for unknown IoTypes, it will keep the phyAdapter.Ptype, i.e., any number above the known types, like 17, 18, etc...

There are a lot of checks for specific types, as I mentioned, but I couldn't find any returns or errors generated due to an unknown type... so I think we are safe....

[rene]

@eriknordmark , btw I've found this https://github.com/lf-edge/eve/blob/master/pkg/pillar/types/assignableadapters.go#L237 :

        IoNVMEStorage IoType = 9
	IoSATAStorage IoType = 10
	IoNetEthPF    IoType = 11
	IoNetEthVF    IoType = 12
	IoNVME        IoType = 255
	IoOther       IoType = 255

IoNVME has the same value of IoOther.... I don't think that's on purpose, it is?

[eriknordmark]

There are a lot of checks for specific types, as I mentioned, but I couldn't find any returns or errors generated due to an unknown type... so I think we are safe....

Thanks for checking.

[eriknordmark]

@eriknordmark , btw I've found this https://github.com/lf-edge/eve/blob/master/pkg/pillar/types/assignableadapters.go#L237 :

        IoNVMEStorage IoType = 9
	IoSATAStorage IoType = 10
	IoNetEthPF    IoType = 11
	IoNetEthVF    IoType = 12
	IoNVME        IoType = 255
	IoOther       IoType = 255

IoNVME has the same value of IoOther.... I don't think that's on purpose, it is?

That was the purpose initially. But now we have the unique type IoNVMEStorage so this should be removed AFAIR. However, I see code in pkg/pillar/cmd/domainmgr/domainmgr.go which refers to IoNVME and not IoNVMEStorage. Can you check with Ioannis? If so he can remove it from the API and replace the usage in domainmgr.

[eriknordmark]

LGTM