| Message ID | 20221224194253.3202231-10-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Various patches to improve DCO behaviour | expand |
Hi, On Sat, Dec 24, 2022 at 08:42:53PM +0100, Arne Schwabe wrote: > buf_printf(&out, "%s", mroute_addr_print(&mi->real, gc)); > + if (mi->context.c2.tls_multi) > + { > + buf_printf(&out, " peer-id=%d", mi->context.c2.tls_multi->peer_id); > + } Do I read this right, this will add the peer-id=%d to every single log line in p2mp mode? In DCO mode, this might be a relevant information (... for things related to DCO), but in non-DCO mode, it's mostly "uninteresting", no? I'm all for complete log information, but making the lines too long isn't that helpful either... (The other part of that patch will be looked at, when 4/9 has survived) gert
Am 25.12.22 um 00:13 schrieb Gert Doering: > Hi, > > On Sat, Dec 24, 2022 at 08:42:53PM +0100, Arne Schwabe wrote: >> buf_printf(&out, "%s", mroute_addr_print(&mi->real, gc)); >> + if (mi->context.c2.tls_multi) >> + { >> + buf_printf(&out, " peer-id=%d", mi->context.c2.tls_multi->peer_id); >> + } > > Do I read this right, this will add the peer-id=%d to every single > log line in p2mp mode? In DCO mode, this might be a relevant information > (... for things related to DCO), but in non-DCO mode, it's mostly > "uninteresting", no? > > I'm all for complete log information, but making the lines too long > isn't that helpful either... Yeah. This accidentially slipped into the patch set. I am sorry for including it. It was however really helpful for doing the debugging. Arne
Hi, On Sun, Dec 25, 2022 at 01:23:06AM +0100, Arne Schwabe wrote: > >> + if (mi->context.c2.tls_multi) > >> + { > >> + buf_printf(&out, " peer-id=%d", mi->context.c2.tls_multi->peer_id); > >> + } > Yeah. This accidentially slipped into the patch set. I am sorry for > including it. It was however really helpful for doing the debugging. Thinking more about it, I wonder if we can have it conditionalized on "DCO active" and "verb >=6" (D_DCO_DEBUG). This would make it useful for, well, DCO debugging, but not get in the way otherwise. Do we have enough information available at this place? gert
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 6c6385c6e..858d602ca 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -473,6 +473,10 @@ multi_instance_string(const struct multi_instance *mi, bool null, struct gc_aren buf_printf(&out, "%s/", cn); } buf_printf(&out, "%s", mroute_addr_print(&mi->real, gc)); + if (mi->context.c2.tls_multi) + { + buf_printf(&out, " peer-id=%d", mi->context.c2.tls_multi->peer_id); + } return BSTR(&out); } else if (null) @@ -3243,10 +3247,19 @@ process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi, break; case OVPN_DEL_PEER_REASON_USERSPACE: - /* This very likely ourselves but might be another process, so - * still process it */ - reason = "ovpn-dco: userspace request"; - break; + /* We assume that is ourselves. UUnfortunately, sometimes these + * events happen with enough delay that they can have an order of + * + * dco_del_peer x + * [new client connecting] + * dco_new_peer x + * event from dco_del_peer arrives. + * + * if we do not ignore this we get desynced with the kernel + * since we assume the peer-id is free again. The other way would + * be to send a dco_del_peer again + */ + return; } /* When kernel already deleted the peer, the socket is no longer
Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/multi.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)