[Drbd-dev] [PATCH 32/33] net: add a new bind_add method
Marcelo Ricardo Leitner
marcelo.leitner at gmail.com
Sat Jun 13 12:18:17 CEST 2020
On Thu, May 21, 2020 at 10:42:24AM +0200, Christoph Hellwig wrote:
> On Wed, May 20, 2020 at 08:00:25PM -0300, Marcelo Ricardo Leitner wrote:
> > > + if (err)
> > > + return err;
> > > +
> > > + lock_sock(sk);
> > > + err = sctp_do_bind(sk, (union sctp_addr *)addr, af->sockaddr_len);
> > > + if (!err)
> > > + err = sctp_send_asconf_add_ip(sk, addr, 1);
> > Some problems here.
> > - addr may contain a list of addresses
> > - the addresses, then, are not being validated
> > - sctp_do_bind may fail, on which it requires some undoing
> > (like sctp_bindx_add does)
> > - code duplication with sctp_setsockopt_bindx.
> sctp_do_bind and thus this function only support a single address, as
> that is the only thing that the DLM code requires. I could move the
> user copy out of sctp_setsockopt_bindx and reuse that, but it is a
> rather rcane API.
Yes. With David's patch, which is doing that, it can be as simple as:
static int sctp_bind_add(struct sock *sk, struct sockaddr *addr,
ret = sctp_setsockopt_bindx(sk, addr, addrlen, SCTP_BINDX_ADD_ADDR);
and then dlm would be using code that we can test through sctp-only tests as
> > This patch will conflict with David's one,
> > [PATCH net-next] sctp: Pull the user copies out of the individual sockopt functions.
> Do you have a link? A quick google search just finds your mail that
> I'm replying to.
> > (I'll finish reviewing it in the sequence)
> > AFAICT, this patch could reuse/build on his work in there. The goal is
> > pretty much the same and would avoid the issues above.
> > This patch could, then, point the new bind_add proto op to the updated
> > sctp_setsockopt_bindx almost directly.
> > Question then is: dlm never removes an addr from the bind list. Do we
> > want to add ops for both? Or one that handles both operations?
> > Anyhow, having the add operation but not the del seems very weird to
> > me.
> We generally only add operations for things that we actually use.
> bind_del is another logical op, but we can trivially add that when we
> need it.
More information about the drbd-dev