On Sat, Oct 12, 2019 at 12:25:27AM +0800, Xin Long wrote: > On Fri, Oct 11, 2019 at 11:57 PM Xin Long <lucien.xin@xxxxxxxxx> wrote: > > > > On Thu, Oct 10, 2019 at 8:40 PM Neil Horman <nhorman@xxxxxxxxxxxxx> wrote: > > > > > > 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. > > pf_expose: > > -1: compatible with old application (by default) > > 0: not expose PF to user > > 1: expose PF to user > > > > So it should be: > > if pf_expose == -1: not send event, not return -EACCESS > > if pf_expose == 0: not send event, return -EACCESS > > if pf_expose > 0: sent event, not return -EACCESS > > > > makes sense? > Oh, sorry, pf_expose is 1 bit only now in asoc/ep. > Maybe we should use 2 bits, and values could be: > 2: compatible with old application (by default) > 0: not expose PF to user > 1: expose PF to user > Yes, this version makes sense to me Best Neil > > > > > > > > Neil > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > David > > > > > > > > > > > > > > - > > > > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > > > > > > > Registration No: 1397386 (Wales) > > > > > > > > > > > > > > > > > >
