Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
Hi,
2018-06-28 15:40 GMT+09:00 Xin Long :
> On Tue, Jun 26, 2018 at 8:02 PM, 吉藤英明
> wrote:
>> 2018-06-26 13:33 GMT+09:00 Xin Long :
>>> On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
>>> wrote:
Hi,
On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
> Hi,
>
> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner
> :
> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
> >> > From: Xin Long
> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
> >> >
> >> > > struct sctp_paddrparams {
> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> >> > > __u32 spp_pathmtu;
> >> > > __u32 spp_sackdelay;
> >> > > __u32 spp_flags;
> >> > > + __u32 spp_ipv6_flowlabel;
> >> > > + __u8spp_dscp;
> >> > > } __attribute__((packed, aligned(4)));
> >> >
> >> > I don't think you can change the size of this structure like this.
> >> >
> >> > This check in sctp_setsockopt_peer_addr_params():
> >> >
> >> > if (optlen != sizeof(struct sctp_paddrparams))
> >> > return -EINVAL;
> >> >
> >> > is going to trigger in old kernels when executing programs
> >> > built against the new struct definition.
> >
> > That will happen, yes, but do we really care about being future-proof
> > here? I mean: if we also update such check(s) to support dealing with
> > smaller-than-supported structs, newer kernels will be able to run
> > programs built against the old struct, and the new one; while building
> > using newer headers and running on older kernel may fool the
> > application in other ways too (like enabling support for something
> > that is available on newer kernel and that is not present in the older
> > one).
>
> We should not break existing apps.
> We still accept apps of pre-2.4 era without sin6_scope_id
> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
Yes. That's what I tried to say. That is supporting an old app built
with old kernel headers and running on a newer kernel, and not the
other way around (an app built with fresh headers and running on an
old kernel).
>>> To make it, I will update the check like:
>>>
>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>> index 1df5d07..c949d8c 100644
>>> --- a/net/sctp/socket.c
>>> +++ b/net/sctp/socket.c
>>> @@ -2715,13 +2715,18 @@ static int
>>> sctp_setsockopt_peer_addr_params(struct sock *sk,
>>> struct sctp_sock*sp = sctp_sk(sk);
>>> int error;
>>> int hb_change, pmtud_change, sackdelay_change;
>>> + int plen = sizeof(params);
>>> + int old_plen = plen - sizeof(u32) * 2;
>>
>> if (optlen < offsetof(struct sctp_paddrparams, spp_ipv6_flowlabel))
>> maybe?
> Hi, yoshfuji,
> offsetof() is better. thank you.
>
>>
>>>
>>> - if (optlen != sizeof(struct sctp_paddrparams))
>>> + if (optlen != plen && optlen != old_plen)
>>> return -EINVAL;
>>>
>>> if (copy_from_user(, optval, optlen))
>>> return -EFAULT;
>>>
>>> + if (optlen == old_plen)
>>> + params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);
>>
>> I think we should return -EINVAL if size is not new one.
> Sorry, if we returned -EINVAL when size is the old one,
> how can we guarantee an old app built with old kernel
> headers and running on a newer kernel works well?
> or you meant?
> if ((params.spp_flags & (SPP_DSCP | SPP_IPV6_FLOWLABEL)) &&
> optlen != plen)
> return EINVAL;
Yes, I meant this (it should be -EINVAL though).
>
>>
>> --yoshfuji
>>
>>> +
>>> /* Validate flags and value parameters. */
>>> hb_change= params.spp_flags & SPP_HB;
>>> pmtud_change = params.spp_flags & SPP_PMTUD;
>>> @@ -5591,10 +5596,13 @@ static int
>>> sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
>>> struct sctp_transport *trans = NULL;
>>> struct sctp_association *asoc = NULL;
>>> struct sctp_sock*sp = sctp_sk(sk);
>>> + int plen = sizeof(params);
>>> + int old_plen = plen - sizeof(u32) * 2;
>>>
>>> - if (len < sizeof(struct sctp_paddrparams))
>>> + if (len < old_plen)
>>> return -EINVAL;
>>> - len = sizeof(struct sctp_paddrparams);
>>> +
>>> + len = len >= plen ? plen : old_plen;
>>> if (copy_from_user(, optval, len))
>>> return -EFAULT;
>>>
>>> does it look ok to you?
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
On Tue, Jun 26, 2018 at 8:02 PM, 吉藤英明
wrote:
> 2018-06-26 13:33 GMT+09:00 Xin Long :
>> On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
>> wrote:
>>> Hi,
>>>
>>> On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
Hi,
2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner
:
> On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>> > From: Xin Long
>> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>> >
>> > > struct sctp_paddrparams {
>> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>> > > __u32 spp_pathmtu;
>> > > __u32 spp_sackdelay;
>> > > __u32 spp_flags;
>> > > + __u32 spp_ipv6_flowlabel;
>> > > + __u8spp_dscp;
>> > > } __attribute__((packed, aligned(4)));
>> >
>> > I don't think you can change the size of this structure like this.
>> >
>> > This check in sctp_setsockopt_peer_addr_params():
>> >
>> > if (optlen != sizeof(struct sctp_paddrparams))
>> > return -EINVAL;
>> >
>> > is going to trigger in old kernels when executing programs
>> > built against the new struct definition.
>
> That will happen, yes, but do we really care about being future-proof
> here? I mean: if we also update such check(s) to support dealing with
> smaller-than-supported structs, newer kernels will be able to run
> programs built against the old struct, and the new one; while building
> using newer headers and running on older kernel may fool the
> application in other ways too (like enabling support for something
> that is available on newer kernel and that is not present in the older
> one).
We should not break existing apps.
We still accept apps of pre-2.4 era without sin6_scope_id
(e.g., net/ipv6/af_inet6.c:inet6_bind()).
>>>
>>> Yes. That's what I tried to say. That is supporting an old app built
>>> with old kernel headers and running on a newer kernel, and not the
>>> other way around (an app built with fresh headers and running on an
>>> old kernel).
>> To make it, I will update the check like:
>>
>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>> index 1df5d07..c949d8c 100644
>> --- a/net/sctp/socket.c
>> +++ b/net/sctp/socket.c
>> @@ -2715,13 +2715,18 @@ static int
>> sctp_setsockopt_peer_addr_params(struct sock *sk,
>> struct sctp_sock*sp = sctp_sk(sk);
>> int error;
>> int hb_change, pmtud_change, sackdelay_change;
>> + int plen = sizeof(params);
>> + int old_plen = plen - sizeof(u32) * 2;
>
> if (optlen < offsetof(struct sctp_paddrparams, spp_ipv6_flowlabel))
> maybe?
Hi, yoshfuji,
offsetof() is better. thank you.
>
>>
>> - if (optlen != sizeof(struct sctp_paddrparams))
>> + if (optlen != plen && optlen != old_plen)
>> return -EINVAL;
>>
>> if (copy_from_user(, optval, optlen))
>> return -EFAULT;
>>
>> + if (optlen == old_plen)
>> + params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);
>
> I think we should return -EINVAL if size is not new one.
Sorry, if we returned -EINVAL when size is the old one,
how can we guarantee an old app built with old kernel
headers and running on a newer kernel works well?
or you meant?
if ((params.spp_flags & (SPP_DSCP | SPP_IPV6_FLOWLABEL)) &&
optlen != plen)
return EINVAL;
>
> --yoshfuji
>
>> +
>> /* Validate flags and value parameters. */
>> hb_change= params.spp_flags & SPP_HB;
>> pmtud_change = params.spp_flags & SPP_PMTUD;
>> @@ -5591,10 +5596,13 @@ static int
>> sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
>> struct sctp_transport *trans = NULL;
>> struct sctp_association *asoc = NULL;
>> struct sctp_sock*sp = sctp_sk(sk);
>> + int plen = sizeof(params);
>> + int old_plen = plen - sizeof(u32) * 2;
>>
>> - if (len < sizeof(struct sctp_paddrparams))
>> + if (len < old_plen)
>> return -EINVAL;
>> - len = sizeof(struct sctp_paddrparams);
>> +
>> + len = len >= plen ? plen : old_plen;
>> if (copy_from_user(, optval, len))
>> return -EFAULT;
>>
>> does it look ok to you?
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
2018-06-26 13:33 GMT+09:00 Xin Long :
> On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
> wrote:
>> Hi,
>>
>> On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
>>> Hi,
>>>
>>> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner
>>> :
>>> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>>> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>>> >> > From: Xin Long
>>> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>>> >> >
>>> >> > > struct sctp_paddrparams {
>>> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>>> >> > > __u32 spp_pathmtu;
>>> >> > > __u32 spp_sackdelay;
>>> >> > > __u32 spp_flags;
>>> >> > > + __u32 spp_ipv6_flowlabel;
>>> >> > > + __u8spp_dscp;
>>> >> > > } __attribute__((packed, aligned(4)));
>>> >> >
>>> >> > I don't think you can change the size of this structure like this.
>>> >> >
>>> >> > This check in sctp_setsockopt_peer_addr_params():
>>> >> >
>>> >> > if (optlen != sizeof(struct sctp_paddrparams))
>>> >> > return -EINVAL;
>>> >> >
>>> >> > is going to trigger in old kernels when executing programs
>>> >> > built against the new struct definition.
>>> >
>>> > That will happen, yes, but do we really care about being future-proof
>>> > here? I mean: if we also update such check(s) to support dealing with
>>> > smaller-than-supported structs, newer kernels will be able to run
>>> > programs built against the old struct, and the new one; while building
>>> > using newer headers and running on older kernel may fool the
>>> > application in other ways too (like enabling support for something
>>> > that is available on newer kernel and that is not present in the older
>>> > one).
>>>
>>> We should not break existing apps.
>>> We still accept apps of pre-2.4 era without sin6_scope_id
>>> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
>>
>> Yes. That's what I tried to say. That is supporting an old app built
>> with old kernel headers and running on a newer kernel, and not the
>> other way around (an app built with fresh headers and running on an
>> old kernel).
> To make it, I will update the check like:
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 1df5d07..c949d8c 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2715,13 +2715,18 @@ static int
> sctp_setsockopt_peer_addr_params(struct sock *sk,
> struct sctp_sock*sp = sctp_sk(sk);
> int error;
> int hb_change, pmtud_change, sackdelay_change;
> + int plen = sizeof(params);
> + int old_plen = plen - sizeof(u32) * 2;
if (optlen < offsetof(struct sctp_paddrparams, spp_ipv6_flowlabel))
maybe?
>
> - if (optlen != sizeof(struct sctp_paddrparams))
> + if (optlen != plen && optlen != old_plen)
> return -EINVAL;
>
> if (copy_from_user(, optval, optlen))
> return -EFAULT;
>
> + if (optlen == old_plen)
> + params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);
I think we should return -EINVAL if size is not new one.
--yoshfuji
> +
> /* Validate flags and value parameters. */
> hb_change= params.spp_flags & SPP_HB;
> pmtud_change = params.spp_flags & SPP_PMTUD;
> @@ -5591,10 +5596,13 @@ static int
> sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
> struct sctp_transport *trans = NULL;
> struct sctp_association *asoc = NULL;
> struct sctp_sock*sp = sctp_sk(sk);
> + int plen = sizeof(params);
> + int old_plen = plen - sizeof(u32) * 2;
>
> - if (len < sizeof(struct sctp_paddrparams))
> + if (len < old_plen)
> return -EINVAL;
> - len = sizeof(struct sctp_paddrparams);
> +
> + len = len >= plen ? plen : old_plen;
> if (copy_from_user(, optval, len))
> return -EFAULT;
>
> does it look ok to you?
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
On Tue, Jun 26, 2018 at 12:31 AM, Marcelo Ricardo Leitner
wrote:
> Hi,
>
> On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
>> Hi,
>>
>> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner
>> :
>> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>> >> > From: Xin Long
>> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>> >> >
>> >> > > struct sctp_paddrparams {
>> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>> >> > > __u32 spp_pathmtu;
>> >> > > __u32 spp_sackdelay;
>> >> > > __u32 spp_flags;
>> >> > > + __u32 spp_ipv6_flowlabel;
>> >> > > + __u8spp_dscp;
>> >> > > } __attribute__((packed, aligned(4)));
>> >> >
>> >> > I don't think you can change the size of this structure like this.
>> >> >
>> >> > This check in sctp_setsockopt_peer_addr_params():
>> >> >
>> >> > if (optlen != sizeof(struct sctp_paddrparams))
>> >> > return -EINVAL;
>> >> >
>> >> > is going to trigger in old kernels when executing programs
>> >> > built against the new struct definition.
>> >
>> > That will happen, yes, but do we really care about being future-proof
>> > here? I mean: if we also update such check(s) to support dealing with
>> > smaller-than-supported structs, newer kernels will be able to run
>> > programs built against the old struct, and the new one; while building
>> > using newer headers and running on older kernel may fool the
>> > application in other ways too (like enabling support for something
>> > that is available on newer kernel and that is not present in the older
>> > one).
>>
>> We should not break existing apps.
>> We still accept apps of pre-2.4 era without sin6_scope_id
>> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
>
> Yes. That's what I tried to say. That is supporting an old app built
> with old kernel headers and running on a newer kernel, and not the
> other way around (an app built with fresh headers and running on an
> old kernel).
To make it, I will update the check like:
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 1df5d07..c949d8c 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2715,13 +2715,18 @@ static int
sctp_setsockopt_peer_addr_params(struct sock *sk,
struct sctp_sock*sp = sctp_sk(sk);
int error;
int hb_change, pmtud_change, sackdelay_change;
+ int plen = sizeof(params);
+ int old_plen = plen - sizeof(u32) * 2;
- if (optlen != sizeof(struct sctp_paddrparams))
+ if (optlen != plen && optlen != old_plen)
return -EINVAL;
if (copy_from_user(, optval, optlen))
return -EFAULT;
+ if (optlen == old_plen)
+ params.spp_flags &= ~(SPP_DSCP | SPP_IPV6_FLOWLABEL);
+
/* Validate flags and value parameters. */
hb_change= params.spp_flags & SPP_HB;
pmtud_change = params.spp_flags & SPP_PMTUD;
@@ -5591,10 +5596,13 @@ static int
sctp_getsockopt_peer_addr_params(struct sock *sk, int len,
struct sctp_transport *trans = NULL;
struct sctp_association *asoc = NULL;
struct sctp_sock*sp = sctp_sk(sk);
+ int plen = sizeof(params);
+ int old_plen = plen - sizeof(u32) * 2;
- if (len < sizeof(struct sctp_paddrparams))
+ if (len < old_plen)
return -EINVAL;
- len = sizeof(struct sctp_paddrparams);
+
+ len = len >= plen ? plen : old_plen;
if (copy_from_user(, optval, len))
return -EFAULT;
does it look ok to you?
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
Hi,
On Tue, Jun 26, 2018 at 01:12:00AM +0900, 吉藤英明 wrote:
> Hi,
>
> 2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner
> :
> > On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
> >> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
> >> > From: Xin Long
> >> > Date: Mon, 25 Jun 2018 10:14:35 +0800
> >> >
> >> > > struct sctp_paddrparams {
> >> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> >> > > __u32 spp_pathmtu;
> >> > > __u32 spp_sackdelay;
> >> > > __u32 spp_flags;
> >> > > + __u32 spp_ipv6_flowlabel;
> >> > > + __u8spp_dscp;
> >> > > } __attribute__((packed, aligned(4)));
> >> >
> >> > I don't think you can change the size of this structure like this.
> >> >
> >> > This check in sctp_setsockopt_peer_addr_params():
> >> >
> >> > if (optlen != sizeof(struct sctp_paddrparams))
> >> > return -EINVAL;
> >> >
> >> > is going to trigger in old kernels when executing programs
> >> > built against the new struct definition.
> >
> > That will happen, yes, but do we really care about being future-proof
> > here? I mean: if we also update such check(s) to support dealing with
> > smaller-than-supported structs, newer kernels will be able to run
> > programs built against the old struct, and the new one; while building
> > using newer headers and running on older kernel may fool the
> > application in other ways too (like enabling support for something
> > that is available on newer kernel and that is not present in the older
> > one).
>
> We should not break existing apps.
> We still accept apps of pre-2.4 era without sin6_scope_id
> (e.g., net/ipv6/af_inet6.c:inet6_bind()).
Yes. That's what I tried to say. That is supporting an old app built
with old kernel headers and running on a newer kernel, and not the
other way around (an app built with fresh headers and running on an
old kernel).
>
> >
> >> >
> >> I think thats also the reason its a packed aligned attribute, it can't be
> >> changed, or older kernels won't be able to fill it out properly.
> >> Neil
> >
> > It's more for supporting running 32-bits apps on 64-bit kernels
> > (according to 20c9c825b12fc).
> >
> > Marcelo
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
Hi,
2018-06-25 22:03 GMT+09:00 Marcelo Ricardo Leitner :
> On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
>> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
>> > From: Xin Long
>> > Date: Mon, 25 Jun 2018 10:14:35 +0800
>> >
>> > > struct sctp_paddrparams {
>> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
>> > > __u32 spp_pathmtu;
>> > > __u32 spp_sackdelay;
>> > > __u32 spp_flags;
>> > > + __u32 spp_ipv6_flowlabel;
>> > > + __u8spp_dscp;
>> > > } __attribute__((packed, aligned(4)));
>> >
>> > I don't think you can change the size of this structure like this.
>> >
>> > This check in sctp_setsockopt_peer_addr_params():
>> >
>> > if (optlen != sizeof(struct sctp_paddrparams))
>> > return -EINVAL;
>> >
>> > is going to trigger in old kernels when executing programs
>> > built against the new struct definition.
>
> That will happen, yes, but do we really care about being future-proof
> here? I mean: if we also update such check(s) to support dealing with
> smaller-than-supported structs, newer kernels will be able to run
> programs built against the old struct, and the new one; while building
> using newer headers and running on older kernel may fool the
> application in other ways too (like enabling support for something
> that is available on newer kernel and that is not present in the older
> one).
We should not break existing apps.
We still accept apps of pre-2.4 era without sin6_scope_id
(e.g., net/ipv6/af_inet6.c:inet6_bind()).
>
>> >
>> I think thats also the reason its a packed aligned attribute, it can't be
>> changed, or older kernels won't be able to fill it out properly.
>> Neil
>
> It's more for supporting running 32-bits apps on 64-bit kernels
> (according to 20c9c825b12fc).
>
> Marcelo
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
On Mon, Jun 25, 2018 at 07:28:47AM -0400, Neil Horman wrote:
> On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
> > From: Xin Long
> > Date: Mon, 25 Jun 2018 10:14:35 +0800
> >
> > > struct sctp_paddrparams {
> > > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> > > __u32 spp_pathmtu;
> > > __u32 spp_sackdelay;
> > > __u32 spp_flags;
> > > + __u32 spp_ipv6_flowlabel;
> > > + __u8spp_dscp;
> > > } __attribute__((packed, aligned(4)));
> >
> > I don't think you can change the size of this structure like this.
> >
> > This check in sctp_setsockopt_peer_addr_params():
> >
> > if (optlen != sizeof(struct sctp_paddrparams))
> > return -EINVAL;
> >
> > is going to trigger in old kernels when executing programs
> > built against the new struct definition.
That will happen, yes, but do we really care about being future-proof
here? I mean: if we also update such check(s) to support dealing with
smaller-than-supported structs, newer kernels will be able to run
programs built against the old struct, and the new one; while building
using newer headers and running on older kernel may fool the
application in other ways too (like enabling support for something
that is available on newer kernel and that is not present in the older
one).
> >
> I think thats also the reason its a packed aligned attribute, it can't be
> changed, or older kernels won't be able to fill it out properly.
> Neil
It's more for supporting running 32-bits apps on 64-bit kernels
(according to 20c9c825b12fc).
Marcelo
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
On Mon, Jun 25, 2018 at 04:31:26PM +0900, David Miller wrote:
> From: Xin Long
> Date: Mon, 25 Jun 2018 10:14:35 +0800
>
> > struct sctp_paddrparams {
> > @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> > __u32 spp_pathmtu;
> > __u32 spp_sackdelay;
> > __u32 spp_flags;
> > + __u32 spp_ipv6_flowlabel;
> > + __u8spp_dscp;
> > } __attribute__((packed, aligned(4)));
>
> I don't think you can change the size of this structure like this.
>
> This check in sctp_setsockopt_peer_addr_params():
>
> if (optlen != sizeof(struct sctp_paddrparams))
> return -EINVAL;
>
> is going to trigger in old kernels when executing programs
> built against the new struct definition.
>
I think thats also the reason its a packed aligned attribute, it can't be
changed, or older kernels won't be able to fill it out properly.
Neil
Re: [PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
From: Xin Long
Date: Mon, 25 Jun 2018 10:14:35 +0800
> struct sctp_paddrparams {
> @@ -773,6 +775,8 @@ struct sctp_paddrparams {
> __u32 spp_pathmtu;
> __u32 spp_sackdelay;
> __u32 spp_flags;
> + __u32 spp_ipv6_flowlabel;
> + __u8spp_dscp;
> } __attribute__((packed, aligned(4)));
I don't think you can change the size of this structure like this.
This check in sctp_setsockopt_peer_addr_params():
if (optlen != sizeof(struct sctp_paddrparams))
return -EINVAL;
is going to trigger in old kernels when executing programs
built against the new struct definition.
[PATCH net-next 3/5] sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
spp_ipv6_flowlabel and spp_dscp are added in sctp_paddrparams in
this patch so that users could set sctp_sock/asoc/transport dscp
and flowlabel with spp_flags SPP_IPV6_FLOWLABEL or SPP_DSCP by
SCTP_PEER_ADDR_PARAMS , as described section 8.1.12 in RFC6458.
As said in last patch, it uses '| 0x10' or '|0x1' to mark
flowlabel or dscp is set, so that their values could be set
to 0.
Signed-off-by: Xin Long
---
include/uapi/linux/sctp.h | 4 ++
net/sctp/socket.c | 152 ++
2 files changed, 156 insertions(+)
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index c02986a..b479db5 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -763,6 +763,8 @@ enum sctp_spp_flags {
SPP_SACKDELAY_DISABLE = 1<<6, /*Disable SACK*/
SPP_SACKDELAY = SPP_SACKDELAY_ENABLE | SPP_SACKDELAY_DISABLE,
SPP_HB_TIME_IS_ZERO = 1<<7, /* Set HB delay to 0 */
+ SPP_IPV6_FLOWLABEL = 1<<8,
+ SPP_DSCP = 1<<9,
};
struct sctp_paddrparams {
@@ -773,6 +775,8 @@ struct sctp_paddrparams {
__u32 spp_pathmtu;
__u32 spp_sackdelay;
__u32 spp_flags;
+ __u32 spp_ipv6_flowlabel;
+ __u8spp_dscp;
} __attribute__((packed, aligned(4)));
/*
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index bf11f9c..857de62 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2393,6 +2393,8 @@ static int sctp_setsockopt_autoclose(struct sock *sk,
char __user *optval,
* uint32_tspp_pathmtu;
* uint32_tspp_sackdelay;
* uint32_tspp_flags;
+ * uint32_tspp_ipv6_flowlabel;
+ * uint8_t spp_dscp;
* };
*
* spp_assoc_id- (one-to-many style socket) This is filled in the
@@ -2472,6 +2474,45 @@ static int sctp_setsockopt_autoclose(struct sock *sk,
char __user *optval,
* also that this field is mutually exclusive to
* SPP_SACKDELAY_ENABLE, setting both will have undefined
* results.
+ *
+ * SPP_IPV6_FLOWLABEL: Setting this flag enables the
+ * setting of the IPV6 flow label value. The value is
+ * contained in the spp_ipv6_flowlabel field.
+ * Upon retrieval, this flag will be set to indicate that
+ * the spp_ipv6_flowlabel field has a valid value returned.
+ * If a specific destination address is set (in the
+ * spp_address field), then the value returned is that of
+ * the address. If just an association is specified (and
+ * no address), then the association's default flow label
+ * is returned. If neither an association nor a
destination
+ * is specified, then the socket's default flow label is
+ * returned. For non-IPv6 sockets, this flag will be left
+ * cleared.
+ *
+ * SPP_DSCP: Setting this flag enables the setting of the
+ * Differentiated Services Code Point (DSCP) value
+ * associated with either the association or a specific
+ * address. The value is obtained in the spp_dscp field.
+ * Upon retrieval, this flag will be set to indicate that
+ * the spp_dscp field has a valid value returned. If a
+ * specific destination address is set when called (in the
+ * spp_address field), then that specific destination
+ * address's DSCP value is returned. If just an
association
+ * is specified, then the association's default DSCP is
+ * returned. If neither an association nor a destination
is
+ * specified, then the socket's default DSCP is returned.
+ *
+ * spp_ipv6_flowlabel
+ * - This field is used in conjunction with the
+ * SPP_IPV6_FLOWLABEL flag and contains the IPv6 flow
label.
+ * The 20 least significant bits are used for the flow
+ * label. This setting has precedence over any IPv6-layer
+ * setting.
+ *
+ * spp_dscp- This field is used in conjunction with the SPP_DSCP flag
+ * and contains the DSCP. The 6 most significant bits are
+ * used for the DSCP. This setting has precedence over any
+ * IPv4- or IPv6- layer setting.
*/
static int sctp_apply_peer_addr_params(struct sctp_paddrparams *params,
struct sctp_transport *trans,
@@ -2611,6 +2652,51 @@ static int sctp_apply_peer_addr_params(struct
