Martin Pieuchot <[email protected]> writes: > On 14/06/16(Tue) 13:49, Jeremie Courreges-Anglas wrote: >> Vincent Gross <[email protected]> writes: >> > On Mon, 13 Jun 2016 19:57:15 +0200 >> > Jeremie Courreges-Anglas <[email protected]> wrote: >> >> Vincent Gross <[email protected]> writes: >> >> > Le Mon, 13 Jun 2016 07:35:16 +0200, >> >> > [email protected] (Jeremie Courreges-Anglas) a écrit : >> >> >> [email protected] (Jeremie Courreges-Anglas) writes: >> >> >> >> >> >> > cc'ing sthen since he also has interest in IP_SENDSRCADDR >> >> >> > >> >> >> > Jeremie Courreges-Anglas <[email protected]> writes: >> >> >> > >> >> >> >> Vincent Gross <[email protected]> writes: >> >> >> >> >> >> >> >>> This diff adds support for IP_SENDSRCADDR cmsg on UDP sockets. >> >> >> >>> As for udp6_output(), we check that the source address+port is >> >> >> >>> available only if inp_laddr != * >> >> >> >> >> >> >> >> Your last IP_SENDSRCADDR diff didn't have that check, I think >> >> >> >> it is harmful. If the socket is not bound then there is >> >> >> >> effectively no check performed by in_pcbaddrisavail(), thus I >> >> >> >> can use any random address. Other than this additional bypass >> >> >> >> check, your diff looks good to me. >> >> >> >> >> >> > [...] >> >> >> >> >> >> >> >> I haven't checked yet whether udp6_output is also affected. If >> >> >> >> you folks already know that it isn't, please let me know. >> >> >> >> >> >> The answer is "no", a few tests can't trigger the same problem. >> >> >> IIUC in6_selectsrc is responsible for rejection of non-local >> >> >> systems. Maybe we should take the same approach in netinet/, and >> >> >> extend in_selectsrc()? >> >> >> >> >> >> -- >> >> > >> >> > While validating source address inside selection functions is the >> >> > right direction, I don't think it would be a good thing to extend >> >> > further in_selectsrc() prototype. >> >> >> >> I find it nice to have all the source address selection in one place. >> >> Or do you have another refactoring in mind? >> >> >> > >> > Uh, turns out I was operating on obsolete data. I would actually be >> > easy to shrink in_selectsrc() prototype to >> > (int)(struct in_addr **, struct sockaddr_in *, struct in_pcb *). >> > But this looks like a layering violation to me ... What do you think ? >> >> I wouldn't mind adding an extra argument and staying closer to >> in6_selectsrc. > > Can we get the feature in first then see how we refactor things later?
Agreed. > I really don't think that extending in_selectsrc() is a good thing. > in6_selectsrc() is a bad example and IMHO this function should be > revisited. Having a function taking 7 arguments with 4 optional > ones makes it very difficult to understand what's happening. Your call. :) -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
