[net-next,0/5] SCTP updates

On 07/10/2014 11:02 AM, David Laight wrote:
> From: Neil Horman
> ...
>>> No there is not direct overlap between the two.  However, as Michael pointed out,
>>> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
>>> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
>>> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
>>>
>> Ok, so we should still consider deprecation warnings then.  Daniel, what about
>> ratelimited warnings with pids included then?
>
> Can you defer any deprecation warnings for a few kernel versions?
> This gives time for applications to be recoded.

I am fine either way, adding the warning a bit later seems fine, too.

Right now, even in-kernel users like dlm would hit it if we include
it immediately.

Otherwise, I'll just add something like the below ...


> Including argv[0] (even just the exec-time value) is much more use than the pid.
>
> Actually this is 'right PITA' for an application.
> A program binary that needs to work with old and new kernels will have to
> try the new option, and if it fails fall back to the old one, and then
> conditionally create/inspect the cmsg data.
> I can't actually imagine anyone bothering!
>
> Our sctp code is actually in a kernel module, so we can look at the kernel
> version when (part of) the driver is compiled on the target system.
>
> 	David
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Jul 10, 2014 at 11:37:19AM +0200, Daniel Borkmann wrote:
> On 07/10/2014 11:02 AM, David Laight wrote:
> >From: Neil Horman
> >...
> >>>No there is not direct overlap between the two.  However, as Michael pointed out,
> >>>there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> >>>warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> >>>Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> >>>
> >>Ok, so we should still consider deprecation warnings then.  Daniel, what about
> >>ratelimited warnings with pids included then?
> >
> >Can you defer any deprecation warnings for a few kernel versions?
> >This gives time for applications to be recoded.
> 
> I am fine either way, adding the warning a bit later seems fine, too.
> 
> Right now, even in-kernel users like dlm would hit it if we include
> it immediately.
> 
> Otherwise, I'll just add something like the below ...
> 
This works for me, I don't see any reason to wait, and its a pretty easy
changeover for most applications to make.

Thanks David.
Neil

> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d95a50c..6a0e5a4 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user *optval,
>  	if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
>  		return -EFAULT;
> 
> -	/*
> -	 * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
> +	if (sctp_sk(sk)->subscribe.sctp_data_io_event)
> +		pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
> +				    "Requested SCTP_SNDRCVINFO event.\n"
> +				    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
> +				    current->comm, task_pid_nr(current));
> +
> +	/* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
>  	 * if there is no data to be sent or retransmit, the stack will
>  	 * immediately send up this notification.
>  	 */
> 
> >Including argv[0] (even just the exec-time value) is much more use than the pid.
> >
> >Actually this is 'right PITA' for an application.
> >A program binary that needs to work with old and new kernels will have to
> >try the new option, and if it fails fall back to the old one, and then
> >conditionally create/inspect the cmsg data.
> >I can't actually imagine anyone bothering!
> >
> >Our sctp code is actually in a kernel module, so we can look at the kernel
> >version when (part of) the driver is compiled on the target system.
> >
> >	David
> >
> >
> >
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 07/10/2014 05:37 AM, Daniel Borkmann wrote:
> On 07/10/2014 11:02 AM, David Laight wrote:
>> From: Neil Horman
>> ...
>>>> No there is not direct overlap between the two.  However, as Michael pointed out,
>>>> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
>>>> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
>>>> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
>>>>
>>> Ok, so we should still consider deprecation warnings then.  Daniel, what about
>>> ratelimited warnings with pids included then?
>>
>> Can you defer any deprecation warnings for a few kernel versions?
>> This gives time for applications to be recoded.
> 
> I am fine either way, adding the warning a bit later seems fine, too.
> 
> Right now, even in-kernel users like dlm would hit it if we include
> it immediately.
> 
> Otherwise, I'll just add something like the below ...
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d95a50c..6a0e5a4 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user
> *optval,
>      if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
>          return -EFAULT;
> 
> -    /*
> -     * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
> +    if (sctp_sk(sk)->subscribe.sctp_data_io_event)
> +        pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
> +                    "Requested SCTP_SNDRCVINFO event.\n"
> +                    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
> +                    current->comm, task_pid_nr(current));
> +
> +    /* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
>       * if there is no data to be sent or retransmit, the stack will
>       * immediately send up this notification.
>       */
> 

This is OK for step 1.  Once we implement SCTP_EVENT, we can cover the rest of the events.

-vlad

>> Including argv[0] (even just the exec-time value) is much more use than the pid.
>>
>> Actually this is 'right PITA' for an application.
>> A program binary that needs to work with old and new kernels will have to
>> try the new option, and if it fails fall back to the old one, and then
>> conditionally create/inspect the cmsg data.
>> I can't actually imagine anyone bothering!
>>
>> Our sctp code is actually in a kernel module, so we can look at the kernel
>> version when (part of) the driver is compiled on the target system.
>>
>>     David
>>
>>
>>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html