On 10/19/23 13:27, Simon Horman wrote: > On Wed, Oct 18, 2023 at 10:46:10PM +0200, Ilya Maximets wrote: >> Currently we log the 980-ish byte long tc_action structure as a >> single long hex string. That is very hard to read and hard to >> spot the difference between two. And most of the fields are zero. >> >> Use the sparse hex dump instead as we do for keys already. >> >> Ex.: >> >> Action 1 mismatch: >> - Expected Action: >> 00000000 f0 3c 00 00 01 00 00 00-00 00 00 00 00 00 00 00 >> 000003d0 00 00 00 00 ff ff ff ff- >> - Received Action: >> 00000000 f0 3c 00 00 01 01 00 00-00 00 00 00 00 00 00 00 >> 000003d0 00 00 00 00 ff ff ff ff- >> >> Without the change, each action would be a 1900+ characters >> long string of mostly zeroes. >> >> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org> > > Hi Ilya, > > I agree this is is a nice improvement. And I'm fine with it as is. > But I do feel it is worth mentioning that when reading the > patch I was surprised that it had no impact on test cases. > Perhaps we could consider adding one at some point?
Maybe, but I'm not really sure how to make a reliable test for this without introducing extra test-only code. The main problem is that we're printing the bare hex dump of the structure and the layout of that structure may change depending on a system or a compiler. Also, the only way to trigger this code today is by having an actual bug in the kernel or in OVS. So, we will not be able to test without extra instrumentation. For the time being, it seems like an unnecessary thing for a such low level debug logging. I'd say this should be on even lower level than DBG, because only developers can actually understand what is going on in these logs. > > In any case, > > Acked-by: Simon Horman <ho...@ovn.org> > >> --- >> >> We may want to backport this down to 2.17, since it's >> a debug-only change. > > FWIIW, I am fine either way. Thanks, Eelco and Simon! For now, applied and backported down to 2.17 as is. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
