| Message ID | 20220516185621.6182-5-kprovost@netgate.com |
|---|---|
| State | Superseded |
| Delegated to: | Antonio Quartulli |
| Headers | show |
| Series | [Openvpn-devel,1/4] mtcp: Handle multi_create_instance() returning NULL | expand |
Hi, On 16/05/2022 20:56, Kristof Provost via Openvpn-devel wrote: > From: Kristof Provost <kp@FreeBSD.org> > > Signed-off-by: Kristof Provost <kprovost@netgate.com> > --- > src/openvpn/init.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/src/openvpn/init.c b/src/openvpn/init.c > index 0d991ba4..701749cd 100644 > --- a/src/openvpn/init.c > +++ b/src/openvpn/init.c > @@ -2232,6 +2232,21 @@ do_deferred_p2p_ncp(struct context *c) > msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher"); > return false; > } > + > + if (dco_enabled(&c->options) && (c->options.ping_send_timeout || c->c2.frame.mss_fix)) > + { > + int ret = dco_set_peer(&c->c1.tuntap->dco, > + c->c2.tls_multi->peer_id, > + c->options.ping_send_timeout, > + c->options.ping_rec_timeout, > + c->c2.frame.mss_fix); > + if (ret < 0) > + { > + msg(D_DCO, "Cannot set DCO peer: %s", strerror(-ret)); > + return false; > + } > + } > + unless I am missing something, I don't think we need another dco_set_peer call. It should be enough to move the invocation of dco_set_peer in finish_options at the very top, before any attempt of bailing out. Regards,
On 17 May 2022, at 15:25, Antonio Quartulli wrote: > Hi, > > On 16/05/2022 20:56, Kristof Provost via Openvpn-devel wrote: >> From: Kristof Provost <kp@FreeBSD.org> >> >> Signed-off-by: Kristof Provost <kprovost@netgate.com> >> --- >> src/openvpn/init.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/src/openvpn/init.c b/src/openvpn/init.c >> index 0d991ba4..701749cd 100644 >> --- a/src/openvpn/init.c >> +++ b/src/openvpn/init.c >> @@ -2232,6 +2232,21 @@ do_deferred_p2p_ncp(struct context *c) >> msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher"); >> return false; >> } >> + >> + if (dco_enabled(&c->options) && (c->options.ping_send_timeout || c->c2.frame.mss_fix)) >> + { >> + int ret = dco_set_peer(&c->c1.tuntap->dco, >> + c->c2.tls_multi->peer_id, >> + c->options.ping_send_timeout, >> + c->options.ping_rec_timeout, >> + c->c2.frame.mss_fix); >> + if (ret < 0) >> + { >> + msg(D_DCO, "Cannot set DCO peer: %s", strerror(-ret)); >> + return false; >> + } >> + } >> + > > unless I am missing something, I don't think we need another dco_set_peer call. > > It should be enough to move the invocation of dco_set_peer in finish_options at the very top, before any attempt of bailing out. > Good point. I’ll update my patchset and include this in the next update. Br, Kristof
diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 0d991ba4..701749cd 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2232,6 +2232,21 @@ do_deferred_p2p_ncp(struct context *c) msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher"); return false; } + + if (dco_enabled(&c->options) && (c->options.ping_send_timeout || c->c2.frame.mss_fix)) + { + int ret = dco_set_peer(&c->c1.tuntap->dco, + c->c2.tls_multi->peer_id, + c->options.ping_send_timeout, + c->options.ping_rec_timeout, + c->c2.frame.mss_fix); + if (ret < 0) + { + msg(D_DCO, "Cannot set DCO peer: %s", strerror(-ret)); + return false; + } + } + return true; }