On 6 Feb 2024, at 13:46, Ilya Maximets wrote:
> On 2/4/24 15:40, Timothy Redaelli wrote: >> A log message like this one: >> >> 2024-01-09T06:45:17.201Z|00071|bfd(handler2)|INFO|ovn-0af536-0: BFD state >> change: down->up "Neighbor Signaled Session Down"->"Neighbor Signaled Session >> Down". >> >> can be hard to read since '->' usually represents a status change, but >> in this case the diagnostic code stays constant. Update the log message to >> avoid such ambiguity. The log message for the above event become: >> >> 2024-01-09T06:45:16.211Z|00026|bfd(handler3)|INFO|ovn-0af536-0: BFD state >> change: up->down, previous failure: "Neighbor Signaled Session Down", >> current failure: "Neighbor Signaled Session Down". > > Hi, Timothy. Thanks for the patch! > > Though I'm not sure that the new version is fully correct either. > Since this message is BFD-specific, we should use BFD-specific language. > Diagnostic is not a failure in a general case, so logging them as a failure > is not correct, IMO. > > We may also want to specify that it is a local diagnostic and not a remote > one to avoid any further ambiguity. > > However, if the confusion is caused by the fact that it doesn't change, while > the arrow suggests that it did, maybe we can just re-structure the message > in a following way: > > BFD state change: > (bfd.SessionState: up, bfd.LocalDiag: Neighbor Signaled Session Down) -> > (bfd.SessionState: down, bfd.LocalDiag: Neighbor Signaled Session Down) > > (no need to split lines, I did that only for ease of reading) > > Since at least one part will change, the arrow should not be confusing > anymore. > > Spelling out bfd.Xx names as in the RFC is probably not necessary since we're > printing out simple words (state, diagnostic) in other places, so can do that > here as well. Good idea, I do like the RFC naming of the events. Maybe to save space, we could remove the “bfd.” prefix, but keep the SessionState, etc. Maybe a follow-up patch to change this in all BFD-related messages to make them more uniform? > Thoughts? > > Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
