I've done a bit of commit message grammar ("chacking that the peed_is")
and actually tested this one.  Without the check, my DCO test server
has quite a lot of these...

Jan  7 07:24:06 ubuntu2004 tun-udp-p2mp-topology-subnet[2441339]: Received 
packet for peer-id unknown to OpenVPN: -1, type 0, reason 2
Jan  7 07:24:06 ubuntu2004 tun-tcp-p2mp[2441317]: Received packet for peer-id 
unknown to OpenVPN: -1, type 0, reason 1

with the patch these are gone.  Very welcome de-noisification of my logs :-)

We do still have a bit of "logging redundancy" here... for every client
connect to instance A, p2p instance B logs 3 lines (on D_DCO_DEBUG)

Jan  7 18:35:35 ubuntu2004 tun-udp-p2p-tls-sha256[2463894]: dco_do_read
Jan  7 18:35:35 ubuntu2004 tun-udp-p2p-tls-sha256[2463894]: ovpn-dco: ignoring 
message (type=3) for foreign ifindex 34074
Jan  7 18:35:35 ubuntu2004 tun-udp-p2p-tls-sha256[2463894]: 
process_incoming_dco: received message of type 0 - ignoring

.. this is coming from forward.c::process_incoming_dco(), which I assume
is only called in the p2p case, and the first message is coming from
ovpn_handle_msg() (which is not using the __func__ paradigm... why?) - so
if we can assert that "there is no message, there is nothing to see
(and nothing to log!)" in the multi_process_incoming_dco() case - maybe
we can apply this (unwritten) function contract here as well...

Stare-at-code also agrees that if the function contract says
"if (peer_id < 0) there is no valid packet, ever", early exit is a
good way out - see my note about un-initializing the dco-> fields at 
the *end* of the function for 1/3, though - in this case, no un-init...
and also Arne's comment about "is this documented anywhere?")

Your patch has been applied to the master and release/2.6 branch.

commit 388e032019ec3674b8294c856039b96fe35e5f32 (master)
commit b0dee39c353ae9479fd19e66ae07cb336d57eef8 (release/2.6)
Author: Antonio Quartulli
Date:   Tue Jan 3 21:23:29 2023 +0100

     dco: bail out when no peer-specific message is delivered

     Signed-off-by: Antonio Quartulli <a...@unstable.cc>
     Acked-by: Arne Schwabe <a...@rfc2549.org>
     Message-Id: <20230103202330.1835-...@unstable.cc>
     Signed-off-by: Gert Doering <g...@greenie.muc.de>

kind regards,

Gert Doering

Openvpn-devel mailing list

Reply via email to