Hi Ihar,
I have some comments below.
On 11/17/23 23:28, Ihar Hrachyshka wrote:
The daemon will now log to scapy.log file.
Log messages include stats on request processing time as well as any
errors that may happen during processing.
If you'd like to see even more logs (e.g. for debugging purposes), just
pass --verbose to scapy-server.
Signed-off-by: Ihar Hrachyshka <[email protected]>
---
tests/ovn-macros.at | 5 ++++-
tests/scapy-server.py | 20 ++++++++++++++++++--
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 0b15bcc80..21c314c7c 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -882,7 +882,10 @@ fmt_pkt() {
start_scapy_server() {
pidfile=$ovs_base/scapy.pid
ctlfile=$ovs_base/scapy.ctl
- "$top_srcdir"/tests/scapy-server.py --pidfile=$pidfile --unixctl=$ctlfile
--detach
+ logfile=$ovs_base/scapy.log
+
+ "$top_srcdir"/tests/scapy-server.py \
+ --pidfile=$pidfile --unixctl=$ctlfile --log-file=$logfile --detach
on_exit "test -e \"$pidfile\" && ovs-appctl -t $ctlfile exit"
}
diff --git a/tests/scapy-server.py b/tests/scapy-server.py
index a7255c84d..52def4b5c 100755
--- a/tests/scapy-server.py
+++ b/tests/scapy-server.py
@@ -1,6 +1,7 @@
#!/usr/bin/env python3
import argparse
+import datetime
import ovs.daemon
import ovs.unixctl
@@ -23,15 +24,28 @@ def exit(conn, argv, aux):
def process(data):
+ start_time = datetime.now()
+ vlog.info("received payload request")
I suggest outputting the payload request here. i.e.
vlog.info(f"received payload request: '{data}'")
It makes the log message more useful since we'll know exactly which
payload is being processed. It also allows us to see all processed
packets, not just the ones that cause failures.
try:
data = data.replace('\n', '')
return binascii.hexlify(raw(eval(data))).decode()
- except Exception:
+ except Exception as e:
+ vlog.exception(
+ f"failed to process payload request: {e}\n"
+ f" Request was: {data}")
At this point, "data" has been transformed to remove newlines.
Therefore, if you print it here, it's not an accurate representation of
the input to the program. That's another reason to print it at the
beginning of process(), and don't print it here.
return ""
+ finally:
+ total_time = (datetime.now() - start_time).microseconds / 1000
+ vlog.info(f"took {total_time:.2f}ms to process payload request")
Instead of using datetime.now() (both here and at the beginning of the
function), I suggest using time.perf_counter() for this measurement.
There are a few reasons:
1) perf_counter() uses a monotonic clock, so oddball things like leap
seconds won't affect its output.
2) perf_counter() returns a number of seconds as a float, rather than a
datetime object. Since we only care about the elapsed time, we don't
need the complexities of a datetime object (or the timedelta object you
get as a result of subtracting two datetimes).
3) The final log message has an error. We only account for the
sub-second value when printing. The log message will always make it seem
like it took less than a second, when the operation may have taken
seconds or minutes to complete. The best way to ensure you print the
difference correctly would be to call total_time.total_seconds() to
convert the timedelta object into a float, then print that. However,
since it was mentioned in point (2) already that perf_counter() returns
such a float, you can save some complexity and just use perf_counter().
def payload(conn, argv, aux):
- conn.reply(process(argv[0]))
+ try:
+ conn.reply(process(argv[0]))
+ except Exception as e:
+ vlog.exception(
+ f"failed to reply to payload request: {e}\n"
+ f" Request was: {argv[0]}")
Similarly, you won't have to print `argv[0]` here if you print the
payload at the beginning of process().
def main():
@@ -55,6 +69,8 @@ def main():
ovs.unixctl.command_register("payload", "", 1, 1, payload, None)
ovs.daemon.daemonize_complete()
+ vlog.info("scapy server ready")
+
poller = ovs.poller.Poller()
while not exiting:
server.run()
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
