Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gnsi.accounting is_truncated flags? #96

Closed
haussli opened this issue Jul 14, 2023 · 2 comments · Fixed by #106
Closed

Add gnsi.accounting is_truncated flags? #96

haussli opened this issue Jul 14, 2023 · 2 comments · Fixed by #106

Comments

@haussli
Copy link
Contributor

haussli commented Jul 14, 2023

It is conceivable that an implementation may lack buffer space for some accounting messages, and we have seen this in practice. This might be more likely in embedded and IoT environments.

In theory, gnsi.accounting.CommandService.cmd_args (cmd too?) and GrpcService.payloads (and config_metadata) are potentially affected.
https://github.com/openconfig/gnsi/blob/main/accounting/acct.proto#L147-L151
https://github.com/openconfig/gnsi/blob/main/accounting/acct.proto#L174-L180

@morrowc
Copy link
Contributor

morrowc commented Jul 21, 2023

Seems like a good addition.
would we expect that a truncated message would instead be split over multiple messages?

haussli added a commit to haussli/gnsi that referenced this issue Jul 27, 2023
accounting.CommandService.cmd{_args} and
accounting.GrpcService.{payloads,config_metadata} to indicate that the
fields were truncates due to implementation limitations.

fixes openconfig#96
@haussli
Copy link
Contributor Author

haussli commented Jul 27, 2023

The particular case that brought this to my attention is a subsystem that lacks a buffer large enough to hold a large accounting message (>8k IIRC). Purely an implementation limitation, but truncation is how it handled the issue. That was seen when the NMS pushed a large initial configuration to the device. This suggestion is to simply indicate that it occurred, whether it is by the originating system or some proxy service or even the destination service.

MR for that here #106

WRT splitting messages, I have not found any suggestion in the docs about how that should be done nor why GrpcService.payloads is a "repeated" field. There is no guidance about how payloads should be populated. Should the payload be split somehow? If it is split, how does a NMS correlate and sequence multiple messages? We discussed about 2 weeks ago in chat; I do not think we arrived at a conclusion. :) Should we attempt to define these processes/behaviors separately?

morrowc pushed a commit that referenced this issue Aug 1, 2023
accounting.CommandService.cmd{_args} and
accounting.GrpcService.{payloads,config_metadata} to indicate that the
fields were truncates due to implementation limitations.

fixes #96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants