On Thu, 2025-11-27 at 15:34 +0100, Sabrina Dubroca wrote:
> 2025-11-21, 01:20:44 +0100, Antonio Quartulli wrote:
> > From: Ralf Lici <[email protected]>
> > 
> > Add a selftest to verify that when a socket is bound to a local
> > address,
> > UDP traffic from ovpn is correctly routed through that address.
> > 
> > This test extends test-bind.sh by binding to the addresses on each
> > veth
> > pair and uses tcpdump to confirm that traffic flows as expected.
> 
> Same as the other bind scenario, the test works even if we don't bind
> to that address.

Right, will fix this test as well on the next version.

> A few small comments on the implementation:
> 
> > @@ -547,45 +518,83 @@ static int ovpn_socket(struct ovpn_ctx *ctx,
> > sa_family_t family, int proto)
> >             if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE, ctx-
> > >bind_dev,
> >                            strlen(ctx->bind_dev) + 1) != 0) {
> >                     perror("setsockopt for SO_BINDTODEVICE");
> > -                   return -1;
> > +                   goto close;
> 
> ret isn't reset here, ovpn_socket will return a stale value.

ACK.

> 
> >             }
> >     }
> >  
> > -   ret = bind(s, (struct sockaddr *)&local_sock, sock_len);
> > -   if (ret < 0) {
> > -           perror("cannot bind socket");
> > -           goto err_socket;
> > +   return s;
> > +close:
> > +   close(s);
> > +   return ret;
> > +}
> > +
> [...]
> > @@ -2221,6 +2228,9 @@ static int ovpn_parse_cmd_args(struct ovpn_ctx
> > *ovpn, int argc, char *argv[])
> >  
> >             ovpn->sa_family = AF_INET;
> >  
> > +           ovpn->laddr = NULL;
> > +           ovpn->lport = "1";
> 
> Why do we want lport=1 on the CONNECT side now?

That's a mistake. I should have set "0" to let the kernel pick an
ephemeral port.

> > +
> >             ret = ovpn_parse_new_peer(ovpn, argv[3], argv[4],
> > argv[5], argv[6],
> >                                       NULL);
> >             if (ret < 0) {

Thanks a lot!

-- 
Ralf Lici
Mandelbit Srl

Reply via email to