From: 'Marcelo Ricardo Leitner' <marcelo.leitner@gmail.com> To: David Laight <David.Laight@aculab.com> Cc: Julien Gomes <julien@arista.com>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "davem@davemloft.net" <davem@davemloft.net>, "nhorman@tuxdriver.com" <nhorman@tuxdriver.com>, "vyasevich@gmail.com" <vyasevich@gmail.com>, "lucien.xin@gmail.com" <lucien.xin@gmail.com> Subject: Re: [PATCH net] sctp: make sctp_setsockopt_events() less strict about the option length Date: Thu, 7 Feb 2019 15:47:15 -0200 [thread overview] Message-ID: <20190207174715.GF13621@localhost.localdomain> (raw) In-Reply-To: <1257941619984a2a992af8d801855c04@AcuMS.aculab.com> On Thu, Feb 07, 2019 at 05:33:07PM +0000, David Laight wrote: > From: Marcelo Ricardo Leitner > > Sent: 06 February 2019 21:07 > > > > On Wed, Feb 06, 2019 at 12:48:38PM -0800, Julien Gomes wrote: > > > > > > > > > On 2/6/19 12:37 PM, Marcelo Ricardo Leitner wrote: > > > > On Wed, Feb 06, 2019 at 12:14:30PM -0800, Julien Gomes wrote: > > > >> Make sctp_setsockopt_events() able to accept sctp_event_subscribe > > > >> structures longer than the current definitions. > > > >> > > > >> This should prevent unjustified setsockopt() failures due to struct > > > >> sctp_event_subscribe extensions (as in 4.11 and 4.12) when using > > > >> binaries that should be compatible, but were built with later kernel > > > >> uapi headers. > > > > > > > > Not sure if we support backwards compatibility like this? > > > > > > > > My issue with this change is that by doing this, application will have > > > > no clue if the new bits were ignored or not and it may think that an > > > > event is enabled while it is not. > > > > > > > > A workaround would be to do a getsockopt and check the size that was > > > > returned. But then, it might as well use the right struct here in the > > > > first place. > > > > > > > > I'm seeing current implementation as an implicitly versioned argument: > > > > it will always accept setsockopt calls with an old struct (v4.11 or > > > > v4.12), but if the user tries to use v3 on a v1-only system, it will > > > > be rejected. Pretty much like using a newer setsockopt on an old > > > > system. > > > > > > With the current implementation, given sources that say are supposed to > > > run on a 4.9 kernel (no use of any newer field added in 4.11 or 4.12), > > > we can't rebuild the exact same sources on a 4.19 kernel and still run > > > them on 4.9 without messing with structures re-definition. > > > > Maybe what we want(ed) here then is explicit versioning, to have the 3 > > definitions available. Then the application is able to use, say struct > > sctp_event_subscribe, and be happy with it, while there is struct > > sctp_event_subscribe_v2 and struct sctp_event_subscribe_v3 there too. > > > > But it's too late for that now because that would break applications > > already using the new fields in sctp_event_subscribe. > > It is probably better to break the recompilation of the few programs > that use the new fields (and have them not work on old kernels) > than to stop recompilations of old programs stop working on old > kernels or have requested new options silently ignored. I got confused here, not sure what you mean. Seems there is one "stop" word too many. > > There are all sorts of reasons why programs get built on systems that > are newer than the ones they need to run on. > I'm currently planning to get around the glibc 'memcpy()' fubar so I > can retire some very old build systems before their disks die. You can virtualize those. That's not really a good reason for building with newer kernel and running on old systems, as virtually any old system can be virtualized. Marcelo > > Fortunately our sctp code is in the kernel - so has to be compiled > with the correct headers. > > > > I understand your point, but this still looks like a sort of uapi > > > breakage to me. > > > > Not disagreeing. I really just don't know how supported that is. > > Willing to know so I can pay more attention to this on future changes. > > Agreed, these structures should never be changed. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) >
WARNING: multiple messages have this Message-ID (diff)
From: 'Marcelo Ricardo Leitner' <marcelo.leitner@gmail.com> To: David Laight <David.Laight@aculab.com> Cc: Julien Gomes <julien@arista.com>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "davem@davemloft.net" <davem@davemloft.net>, "nhorman@tuxdriver.com" <nhorman@tuxdriver.com>, "vyasevich@gmail.com" <vyasevich@gmail.com>, "lucien.xin@gmail.com" <lucien.xin@gmail.com> Subject: Re: [PATCH net] sctp: make sctp_setsockopt_events() less strict about the option length Date: Thu, 07 Feb 2019 17:47:15 +0000 [thread overview] Message-ID: <20190207174715.GF13621@localhost.localdomain> (raw) In-Reply-To: <1257941619984a2a992af8d801855c04@AcuMS.aculab.com> On Thu, Feb 07, 2019 at 05:33:07PM +0000, David Laight wrote: > From: Marcelo Ricardo Leitner > > Sent: 06 February 2019 21:07 > > > > On Wed, Feb 06, 2019 at 12:48:38PM -0800, Julien Gomes wrote: > > > > > > > > > On 2/6/19 12:37 PM, Marcelo Ricardo Leitner wrote: > > > > On Wed, Feb 06, 2019 at 12:14:30PM -0800, Julien Gomes wrote: > > > >> Make sctp_setsockopt_events() able to accept sctp_event_subscribe > > > >> structures longer than the current definitions. > > > >> > > > >> This should prevent unjustified setsockopt() failures due to struct > > > >> sctp_event_subscribe extensions (as in 4.11 and 4.12) when using > > > >> binaries that should be compatible, but were built with later kernel > > > >> uapi headers. > > > > > > > > Not sure if we support backwards compatibility like this? > > > > > > > > My issue with this change is that by doing this, application will have > > > > no clue if the new bits were ignored or not and it may think that an > > > > event is enabled while it is not. > > > > > > > > A workaround would be to do a getsockopt and check the size that was > > > > returned. But then, it might as well use the right struct here in the > > > > first place. > > > > > > > > I'm seeing current implementation as an implicitly versioned argument: > > > > it will always accept setsockopt calls with an old struct (v4.11 or > > > > v4.12), but if the user tries to use v3 on a v1-only system, it will > > > > be rejected. Pretty much like using a newer setsockopt on an old > > > > system. > > > > > > With the current implementation, given sources that say are supposed to > > > run on a 4.9 kernel (no use of any newer field added in 4.11 or 4.12), > > > we can't rebuild the exact same sources on a 4.19 kernel and still run > > > them on 4.9 without messing with structures re-definition. > > > > Maybe what we want(ed) here then is explicit versioning, to have the 3 > > definitions available. Then the application is able to use, say struct > > sctp_event_subscribe, and be happy with it, while there is struct > > sctp_event_subscribe_v2 and struct sctp_event_subscribe_v3 there too. > > > > But it's too late for that now because that would break applications > > already using the new fields in sctp_event_subscribe. > > It is probably better to break the recompilation of the few programs > that use the new fields (and have them not work on old kernels) > than to stop recompilations of old programs stop working on old > kernels or have requested new options silently ignored. I got confused here, not sure what you mean. Seems there is one "stop" word too many. > > There are all sorts of reasons why programs get built on systems that > are newer than the ones they need to run on. > I'm currently planning to get around the glibc 'memcpy()' fubar so I > can retire some very old build systems before their disks die. You can virtualize those. That's not really a good reason for building with newer kernel and running on old systems, as virtually any old system can be virtualized. Marcelo > > Fortunately our sctp code is in the kernel - so has to be compiled > with the correct headers. > > > > I understand your point, but this still looks like a sort of uapi > > > breakage to me. > > > > Not disagreeing. I really just don't know how supported that is. > > Willing to know so I can pay more attention to this on future changes. > > Agreed, these structures should never be changed. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) >
next prev parent reply other threads:[~2019-02-07 17:47 UTC|newest] Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-06 20:14 [PATCH net] sctp: make sctp_setsockopt_events() less strict about the option length Julien Gomes 2019-02-06 20:14 ` Julien Gomes 2019-02-06 20:37 ` Marcelo Ricardo Leitner 2019-02-06 20:37 ` Marcelo Ricardo Leitner 2019-02-06 20:48 ` Julien Gomes 2019-02-06 20:48 ` Julien Gomes 2019-02-06 21:07 ` Marcelo Ricardo Leitner 2019-02-06 21:07 ` Marcelo Ricardo Leitner 2019-02-06 21:23 ` Neil Horman 2019-02-06 21:23 ` Neil Horman 2019-02-06 21:48 ` Julien Gomes 2019-02-06 21:48 ` Julien Gomes 2019-02-07 14:44 ` Neil Horman 2019-02-07 14:44 ` Neil Horman 2019-02-06 21:26 ` Julien Gomes 2019-02-06 21:26 ` Julien Gomes 2019-02-06 21:39 ` Neil Horman 2019-02-06 21:39 ` Neil Horman 2019-02-06 21:48 ` Julien Gomes 2019-02-06 21:48 ` Julien Gomes 2019-02-06 21:53 ` Julien Gomes 2019-02-06 21:53 ` Julien Gomes 2019-02-07 14:48 ` Neil Horman 2019-02-07 14:48 ` Neil Horman 2019-02-07 17:33 ` David Laight 2019-02-07 17:33 ` David Laight 2019-02-07 17:47 ` 'Marcelo Ricardo Leitner' [this message] 2019-02-07 17:47 ` 'Marcelo Ricardo Leitner' 2019-02-08 9:53 ` David Laight 2019-02-08 9:53 ` David Laight 2019-02-08 12:36 ` Neil Horman 2019-02-08 12:36 ` Neil Horman 2019-02-06 21:08 ` Neil Horman 2019-02-06 21:08 ` Neil Horman 2019-02-06 21:18 ` Marcelo Ricardo Leitner 2019-02-06 21:18 ` Marcelo Ricardo Leitner 2019-02-09 23:12 ` David Miller 2019-02-09 23:12 ` David Miller 2019-02-10 12:46 ` Marcelo Ricardo Leitner 2019-02-10 12:46 ` Marcelo Ricardo Leitner 2019-02-10 20:15 ` Marcelo Ricardo Leitner 2019-02-10 20:15 ` Marcelo Ricardo Leitner 2019-02-13 16:17 ` David Laight 2019-02-13 17:23 ` 'Marcelo Ricardo Leitner' 2019-02-13 17:23 ` 'Marcelo Ricardo Leitner' 2019-02-11 15:04 ` Neil Horman 2019-02-11 15:04 ` Neil Horman 2019-02-11 17:05 ` Marcelo Ricardo Leitner 2019-02-11 17:05 ` Marcelo Ricardo Leitner 2019-02-06 20:49 ` Neil Horman 2019-02-06 20:49 ` Neil Horman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190207174715.GF13621@localhost.localdomain \ --to=marcelo.leitner@gmail.com \ --cc=David.Laight@aculab.com \ --cc=davem@davemloft.net \ --cc=julien@arista.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sctp@vger.kernel.org \ --cc=lucien.xin@gmail.com \ --cc=netdev@vger.kernel.org \ --cc=nhorman@tuxdriver.com \ --cc=vyasevich@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.