Re: [Lksctp-developers] [RFC PATCH] [SCTP]: Support the new specification

Re: [Lksctp-developers] [RFC PATCH] [SCTP]: Support the new specification

[Vlad Yasevich]

Ivan Skytte Jørgensen wrote:
> On Friday 28 March 2008 20:54:38 Vlad Yasevich wrote:
>> The specification of sctp_connectx() has been changed to return
>> an association id.  Sence it is guaranteed the association id will
>> be a positive 32 bit integer, we can simply return that from our
>> setsockopt() in the case of success.  The library that implements
>> sctp_connectx() will re-interpret the call.
> 
> Partial NAK.
> 
> It breaks old versions of lksctp. I am not aware that we previously have 
> required lksctp to match the kernel exactly unless there was no other way.
> 
> A new sockopt is safer.
> 
> /isj


In what way?  The only possible problem is that sctp_connectx()
can return a positive value on success in addition to 0, depending
on which kernel is used.

How much of a problem is this really?

A new socket option is actually problematic as well, because the 
behavior will depend on the system that lksctp libraries are built
on.

So, I am not sure which tradeoff is better...

-vlad

Re: [Lksctp-developers] [RFC PATCH] [SCTP]: Support the new specification of sctp

[Ivan Skytte Jørgensen]

On Tuesday 01 April 2008 23:46:54 Vlad Yasevich wrote:
> Ivan Skytte Jørgensen wrote:
> > On Friday 28 March 2008 20:54:38 Vlad Yasevich wrote:
> >> The specification of sctp_connectx() has been changed to return
> >> an association id.  Sence it is guaranteed the association id will
> >> be a positive 32 bit integer, we can simply return that from our
> >> setsockopt() in the case of success.  The library that implements
> >> sctp_connectx() will re-interpret the call.
> >
> > Partial NAK.
> >
> > It breaks old versions of lksctp. I am not aware that we previously have
> > required lksctp to match the kernel exactly unless there was no other
> > way.
> >
> > A new sockopt is safer.
> >
> > /isj
>
> In what way?  The only possible problem is that sctp_connectx()
> can return a positive value on success in addition to 0, depending
> on which kernel is used.


Your kernel patch will return positive values on success (the association id).

lksctp-tools-1.0.8 has this in sctp_connectx():
	return setsockopt(fd, SOL_SCTP, SCTP_SOCKOPT_CONNECTX, addrs, addrs_size);

and according to draft-ietf-tsvwg-sctpsocket-15.txt
"
   The call returns 0 on success or -1 if an error occured.  The
   variable errno is then set appropriately."

So new kernel + lksctp108 will not work, because correctly written programs 
that tests for zero/non-zero returncodes for success/failure will suddenly 
start interpreting the return value from the new kernel as an error.

> How much of a problem is this really?

That is more interesting. I will only affect applications that has "=0" 
instead of ">=0" test in calls to sctp_connectx(). I don't know how often 
sctp_connectx() is actually used - I have never used it.

But at least it needs a notice somewhere that there is potential breakage, and 
that there is a tighter-than-usual dependency between kernel and sctplib.

> A new socket option is actually problematic as well, because the
> behavior will depend on the system that lksctp libraries are built
> on.

Uhmm no, because the socket option names are in lksctp's local 
include/netinet/sctp.h. lksctp does not depend on kernel headers at all.

the sctp_connectx() in lksctp will of course have to use the old socket name 
as fallback:
  call brand new sockopt
  ok: good
  failure:
    if application don't care about assoc-id ('id' parameter is NULL):
      call old sockopt
    else
      fail (kernel too old)


And while we are at it: please reread my message from 2006-12-27 about some 
thoughtsI had about sctp_connectx(). I have still not had any time to do 
anything about it :-/

/isj

Re: [Lksctp-developers] [RFC PATCH] [SCTP]: Support the new specification

[Vlad Yasevich]

Hi Ivan

Ivan Skytte Jørgensen wrote:
> 
> 
> And while we are at it: please reread my message from 2006-12-27 about some 
> thoughtsI had about sctp_connectx(). I have still not had any time to do 
> anything about it :-/
> 
> /isj
> 

I've been pondering the library change as well and the conclusion I've 
come to is that we either introduce some ugliness into the 
include/net/sctp.h or we break the API and force people to recompile.

This is really a result of breaking the API in the specification.  That
should not have been allowed, but we now have to live with it.

The good thing is the lksctp library has been unofficially tied to the 
kernel for some time; however, we haven't had to deal with such blatant
API breakage yet.

-vlad

Re: [Lksctp-developers] [RFC PATCH] [SCTP]: Support the new specification

[Vlad Yasevich]

Ivan Skytte Jørgensen wrote:
> On Friday 11 April 2008 15:20:44 Vlad Yasevich wrote:
>> Here is an update to the connectx() implementation the way
>> Ivan proposed.
> 
> The patch looks fine.
> 
>> The hard part to figure out is how to not break the library
>> ABI.  Right now there is really no way to do that without making
>> it a compile time feature for the library.  That is not really
>> an acceptable solution though.  So any ifdefs tricks will not
>> really solve the issue.
> 
> 
> For the user-level library I assume something like this will work:
> 
> ---connect.c---
> int connectx(void) {
> 	return 13; /*old implementation*/
> }
> 
> int connectx_draft14(void) {
> 	return 14; /*new implementation*/
> }
> ---sctp.h---
> #ifndef SCTP_DRAFT13_CONNECTX
> extern int connectx(void) asm ("connectx_draft14");
> #else
> extern int connectx(void);
> #endif

The problem is that the number of parameters to connectx() differs.

This also means that any application attempting to rebuild with with
the new library, but using an old API, will have to change.
I don't know if we can get away this.

Also, who defines SCTP_DRAFT13_CONNECTX or SCTP_DRAFT14_CONNECTX, depending
on how ifdef is written?

It's a non-starter to do that for applications.

If we put that in netinet/sctp.h, then simply by recompiling against the newer
header, you have to change your program to comply to the new symbol without ability
to use the older one.

> ---example program---
> #include <stdio.h>
> int main(void) {
> 	printf("%d\n",connectx());
> 	return 0;
> }
> ---end
> 
> I think the solution above fulfills:
> - old programs continue to use the old function, even if a new library is 
> present.
> 	-	new programs will use the new API.
> 	-	new programs can use the old API if they are in a hurry to recompile and 
> don't have the time to change their source files.
> 	-	new programs will fail to run (hard) if only the old API is available
> 	-	a program can use both APIs (eg. though shared libraries etc)
> 	-	and last: new programs don't have to do anything extra to to use the new 
> API.
> 
> There are other ways to do it, eg. with symbol versioning, with macro 
> definitions, inline functions, etc. I can see that glibc uses internal symbol 
> versioning. I don't know if there is any downside to use the asm("...."). 
> Both gcc and icc supports it.
> 

I guess I have to figure out symbol versioning since I think anything else
falls short.

Of course we can just swallow the whole incompatibility thing and break the API/ABI
stating that you must change your apps.

-vlad

Re: [Lksctp-developers] [RFC PATCH] [SCTP]: Support the new specification of sctp_connectx()

[Ivan Skytte Jørgensen]

On Friday 11 April 2008 21:56:11 Vlad Yasevich wrote:
> Ivan Skytte Jørgensen wrote:
> > ---connect.c---
> > int connectx(void) {
> > 	return 13; /*old implementation*/
> > }
> >
> > int connectx_draft14(void) {
> > 	return 14; /*new implementation*/
> > }
> > ---sctp.h---
> > #ifndef SCTP_DRAFT13_CONNECTX
> > extern int connectx(void) asm ("connectx_draft14");
> > #else
> > extern int connectx(void);
> > #endif
>
> This also means that any application attempting to rebuild with with
> the new library, but using an old API, will have to change.
> I don't know if we can get away this.

We have to. the API is a live draft. The burden should not be one programs 
that follow the latest draft.

> Also, who defines SCTP_DRAFT13_CONNECTX or SCTP_DRAFT14_CONNECTX, depending
> on how ifdef is written?

The application programmer define that.

If he wants to use the new API then he does nothing. This means that the line
	extern int connectx(void) asm ("connectx_draft14");
will be used. The compiler will generate references to connectx_draft14. on 
runtime connectx_draft14 will be called.

If a newly compiledprogram tries to run with an old lksctp then the program 
cannot start at all due to missing symbol.

If the application programmer thinks it is too much of a burden to fix the 
calls to sctp_connectx() then he can #define SCTP_DRAFT13_CONNECTX and the 
line:
	extern int connectx(void);
will be used. The compiler will generate references to connectx. on runtime 
connectx will be called. If run with an old lksctp the program will function 
fine.

If a non-recompiled program uses the old API it will have references to 
connectx, which is the old API.

> If we put that in netinet/sctp.h, then simply by recompiling against the
> newer header, you have to change your program to comply to the new symbol
> without ability to use the older one.

No. The programmer can define SCTP_DRAFT13_CONNECTX  if he wants to use the 
old API.


[snip]
> I guess I have to figure out symbol versioning since I think anything
> else falls short.

Symbol versioning does not solve the compile-time problem of old/new API.

Re: [Lksctp-developers] [RFC PATCH] [SCTP]: Support the new specification

[Vlad Yasevich]

Ivan Skytte Jørgensen wrote:
> On Friday 11 April 2008 21:56:11 Vlad Yasevich wrote:
>> Ivan Skytte Jørgensen wrote:
>>> ---connect.c---
>>> int connectx(void) {
>>> 	return 13; /*old implementation*/
>>> }
>>>
>>> int connectx_draft14(void) {
>>> 	return 14; /*new implementation*/
>>> }
>>> ---sctp.h---
>>> #ifndef SCTP_DRAFT13_CONNECTX
>>> extern int connectx(void) asm ("connectx_draft14");
>>> #else
>>> extern int connectx(void);
>>> #endif
>> This also means that any application attempting to rebuild with with
>> the new library, but using an old API, will have to change.
>> I don't know if we can get away this.
> 
> We have to. the API is a live draft. The burden should not be one programs 
> that follow the latest draft.
> 
>> Also, who defines SCTP_DRAFT13_CONNECTX or SCTP_DRAFT14_CONNECTX, depending
>> on how ifdef is written?
> 
> The application programmer define that.

Sorry, but once we do that, we essentially introduce dead code since this requires
modifications by the programmer and they should just fix the code and pass in NULL.

> 
> If he wants to use the new API then he does nothing. This means that the line
> 	extern int connectx(void) asm ("connectx_draft14");
> will be used. The compiler will generate references to connectx_draft14. on 
> runtime connectx_draft14 will be called.
> 
> If a newly compiledprogram tries to run with an old lksctp then the program 
> cannot start at all due to missing symbol.
> 
> If the application programmer thinks it is too much of a burden to fix the 
> calls to sctp_connectx() then he can #define SCTP_DRAFT13_CONNECTX and the 
> line:
> 	extern int connectx(void);
> will be used. The compiler will generate references to connectx. on runtime 
> connectx will be called. If run with an old lksctp the program will function 
> fine.

Sorry, but I don't buy that.  If the programmer doesn't want to full power of
the new connectx(), he just pass in NULL as additional parameter and he done.
That's easier, IMHO, then to add a #define (less characters to type :>).

> 
> If a non-recompiled program uses the old API it will have references to 
> connectx, which is the old API.
>

That's the only part I am concerned about, but then your argument about the
api being a live draft also applies and once could put on onus on programmer again.

I think that we've accepted that we can not get away from breaking the API;
however, preserving the ABI is a worth goal and I look into that a bit more.
 
>> If we put that in netinet/sctp.h, then simply by recompiling against the
>> newer header, you have to change your program to comply to the new symbol
>> without ability to use the older one.
> 
> No. The programmer can define SCTP_DRAFT13_CONNECTX  if he wants to use the 
> old API.
> 
> 
> [snip]
>> I guess I have to figure out symbol versioning since I think anything
>> else falls short.
> 
> Symbol versioning does not solve the compile-time problem of old/new API.
> 

Neither does a #define.  It just works around it at the cost to the library providers.
I haven't been convinced that that cost is worth it.

-vlad