On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote: > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@xxxxxxxxxxxxx> wrote: > > > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote: > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@xxxxxxxxxx> wrote: > > > > > > > > From: Xin Long > > > > > Sent: 08 October 2019 12:25 > > > > > > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing > > > > > the Potentially Failed Path State", by which users can change > > > > > pf_expose per sock and asoc. > > > > > > > > If I read these patches correctly the default for this sockopt in 'enabled'. > > > > Doesn't this mean that old application binaries will receive notifications > > > > that they aren't expecting? > > > > > > > > I'd have thought that applications would be required to enable it. > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks. > > > > > I don't think we can safely do either of these things. Older > > applications still need to behave as they did prior to the introduction > > of this notification, and we shouldn't allow unexpected notifications to > > be sent. > > > > What if you added a check in get_peer_addr_info to only return -EACCESS > > if pf_expose is 0 and the application isn't subscribed to the PF event? > We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE > events. > > Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info > are new, we should give 'expose' a default value that would disable both. > How do think if we set 'pf_expose = -1' by default. We send the pf event > only if (asoc->pf_expose > 0) in sctp_assoc_control_transport(). > And if pf_expose = 0, we send the event, and return -EACCESS if we call the socket option and find a PF assoc? If so, yes, I think that makes sense. Neil > > > > Neil > > > > > > > > > > David > > > > > > > > - > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > > > > Registration No: 1397386 (Wales) > > > > > > > >