sys/log: Add number of entries support in log

[vrahane]

Please check email on the dev list for more information:

• This adds support for logging number of entries since a given index of the log, it is optional, so only used when enabled
• The MYNEWT_VAL for the same is LOG_FLAGS_TLV_NUM_ENTRIES and the TLV support syscfg is LOG_FLAGS_TLV_SUPPORT
• No LOG_VERSION: 4 needed
• Backwards and forwards compatible with earlier log header since the TLV is added at the end of the log entry
• CLI command support is also added to help read the number of entries since a particular index on the CLI
• Added selftests for num_entries CLI command:

compat> log -ne reboot_log -i 50

043837 entries: 5

Example log entries:

7143966 [ih=0x92ea3333][ne=0] [1577940571820322]  [ix=16] {_ "rsn": "OTHER: 0x0", "cnt": 238, "img": "24.3.19.131805", "hash": "92ea3333ffe0c6eaa01b842e2a0f8692f6b4a748de10378423b7abc8887993d2", "flags": "active bootable confirmed"}
7143973 [ih=0x92ea3333][ne=1] [1577949942585905]  [ix=33] {_ "rsn": "SOFT", "cnt": 239, "img": "24.3.19.131805", "hash": "92ea3333ffe0c6eaa01b842e2a0f8692f6b4a748de10378423b7abc8887993d2", "flags": "active bootable confirmed"}
7143978 [ih=0x92ea3333][ne=2] [1577951411585993]  [ix=50] {_ "rsn": "SOFT", "cnt": 240, "img": "24.3.19.131805", "hash": "92ea3333ffe0c6eaa01b842e2a0f8692f6b4a748de10378423b7abc8887993d2", "flags": "active bootable confirmed"}
7143984 [ih=0x92ea3333][ne=3] [1577953577609410]  [ix=67] {_ "rsn": "SOFT", "cnt": 241, "img": "24.3.19.131805", "hash": "92ea3333ffe0c6eaa01b842e2a0f8692f6b4a748de10378423b7abc8887993d2", "flags": "active bootable confirmed"}
7143989 [ih=0x92ea3333][ne=4] [1577956085609419]  [ix=84] {_ "rsn": "WDOG", "cnt": 242, "img": "24.3.19.131805", "hash": "92ea3333ffe0c6eaa01b842e2a0f8692f6b4a748de10378423b7abc8887993d2", "flags": "active bootable confirmed"}

• mcumgr PR: https://github.com/apache/mynewt-mcumgr/pull/172
• newtmgr PR: https://github.com/apache/mynewt-newtmgr/pull/205

Tests performed

• [x] NRF52840 slinky logs fine as it is
• [x] NRF52840 slinky logs fine with LOG_FLAGS_TLV_SUPPORT and LOG_TLV_NUM_ENTRIES support
• [x] NRF52840 slinky logs fine with downgrading firmware with no LOG_FLAGS_TLV_SUPPORT and LOG_TLV_NUM_ENTRIES support

[andrzej-kaczmarek]

Would be good to explain what's the purpose of logging number of entries in a log itself.

Since you introduce new log version which is not backwards compatible, why no just revamp this so we don't need to add new log version and bunch of ifdefs every time a new field is added, e.g. by using tlv.

[vrahane]

The purpose of logging number of entries in the entry itself is that we do not have to walk all the entries to count the number of entries. For a large number of log entries, walking logs serially to find the end counting number of entries is little slow. So, this gives us a way to persist number of entries and retrieve number of entries whenever needed without having to walk the log.

This change is supposed to be backwards compatible since the base header is the same 15 bytes but somehow entries with log version 3 are not getting read correctly. I am still figuring out why that is happening. We wanted to open a PR with changes sooner than later and hence I thought the backwards compatibility fix could be added later on in a follow on PR. It does not require any format/log header changes as such.

The ifdefs can actually be removed to be similar to image hash. The only reason I kept them is to save us some stack space and code space.

TLV would be a good idea but wouldn't that require too many changes to the logging code ? I wanted to keep the change backwards compatible.

[vrahane]

If we want to make this change such that it can log various entries of different types, flags could be utilized but this header in itself won't be enough to make the log format generic enough, perhaps we could agree upon a log header/trailer format and define its functional aspect. I am open to making some changes to the way it works but do keep in mind we want to keep this change backwards compatible with log version 3.

[andrzej-kaczmarek]

If the base header is the same, you don't need the log version 4 in the first place. The new struct for v4 is redundant and also invalid since counter doesn't need to follow img hash - this depends on flags.

[vrahane]

Yes, that’s what I have been thinking as well. I think the log version 3 was added at the time to add support for flags. So, I should be able to just add a field to the same header. Haven’t gotten to this yet again. Will let you know after I try it again and will update this PR accordingly.

[vrahane]

@andrzej-kaczmarek I did try this out, keeping the same log version works fine with upgrades but downgrades do not work as the header now has an extra field which the old firmware does not know about which is fine in a way. I am thinking of adding the num_entries field after the log entry body, that way downgrades will work fine as well. Another option which we spoke about internally was to read the logs at bootup and populate number of entries and keep incrementing in RAM which is doable but then there are corner cases to worry about. What are your thoughts on it ?