Re: svn commit: r297193 - head/sys/netinet

2016-03-22 Thread Lawrence Stewart 
On 03/23/16 10:59, Conrad Meyer wrote:
> On Tue, Mar 22, 2016 at 8:55 AM, Jonathan T. Looney  wrote:
>> Author: jtl
>> Date: Tue Mar 22 15:55:17 2016
>> New Revision: 297193
>> URL: https://svnweb.freebsd.org/changeset/base/297193
>>
>> ...
>>
>>   MFC after:2 weeks
> 
> This change seems like it would be ineligible for a MFC due to
> changing the ABI of struct tcpopt.

Off the top of my head I can't think of why you would consider struct
tcpopt to be part of the ABI. Can you point to any concrete examples
demonstrating that it is either explicitly or in a defacto sense?

Cheers,
Lawrence
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r297193 - head/sys/netinet

2016-03-22 Thread Jonathan T. Looney 
On 3/22/16, 7:59 PM, "Conrad Meyer"  wrote:

>On Tue, Mar 22, 2016 at 8:55 AM, Jonathan T. Looney  wrote:
>> Author: jtl
>> Date: Tue Mar 22 15:55:17 2016
>> New Revision: 297193
>> URL: https://svnweb.freebsd.org/changeset/base/297193
>>
>> ...
>>
>>   MFC after:2 weeks
>
>This change seems like it would be ineligible for a MFC due to
>changing the ABI of struct tcpopt.

Conrad,

Thanks for the second set of eyes!

However, I don't think struct tcpopt is part of the ABI. As far as I know, it 
is only used for communication within the kernel. Do you have an example of 
where it is used as part of the ABI?

In the end, I don't really care. I had just planned to do the MFC to be a "good 
citizen".

Jonathan

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r297193 - head/sys/netinet

2016-03-22 Thread Conrad Meyer 
On Tue, Mar 22, 2016 at 8:55 AM, Jonathan T. Looney  wrote:
> Author: jtl
> Date: Tue Mar 22 15:55:17 2016
> New Revision: 297193
> URL: https://svnweb.freebsd.org/changeset/base/297193
>
> ...
>
>   MFC after:2 weeks

This change seems like it would be ineligible for a MFC due to
changing the ABI of struct tcpopt.

Best,
Conrad

>
> Modified:
>   head/sys/netinet/tcp_output.c
>   head/sys/netinet/tcp_var.h
>
> Modified: head/sys/netinet/tcp_output.c
> ==
> --- head/sys/netinet/tcp_output.c   Tue Mar 22 15:43:47 2016
> (r297192)
> +++ head/sys/netinet/tcp_output.c   Tue Mar 22 15:55:17 2016
> (r297193)
> @@ -1652,7 +1652,7 @@ tcp_setpersist(struct tcpcb *tp)
>  int
>  tcp_addoptions(struct tcpopt *to, u_char *optp)
>  {
> -   u_int mask, optlen = 0;
> +   u_int32_t mask, optlen = 0;
>
> for (mask = 1; mask < TOF_MAXOPT; mask <<= 1) {
> if ((to->to_flags & mask) != mask)
>
> Modified: head/sys/netinet/tcp_var.h
> ==
> --- head/sys/netinet/tcp_var.h  Tue Mar 22 15:43:47 2016(r297192)
> +++ head/sys/netinet/tcp_var.h  Tue Mar 22 15:55:17 2016(r297193)
> @@ -364,7 +364,7 @@ struct tcpcb {
>   * options in tcp_addoptions.
>   */
>  struct tcpopt {
> -   u_int64_t   to_flags;   /* which options are present */
> +   u_int32_t   to_flags;   /* which options are present */
>  #defineTOF_MSS 0x0001  /* maximum segment size */
>  #defineTOF_SCALE   0x0002  /* window scaling */
>  #defineTOF_SACKPERM0x0004  /* SACK permitted */
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

svn commit: r297193 - head/sys/netinet

2016-03-22 Thread Jonathan T. Looney 
Author: jtl
Date: Tue Mar 22 15:55:17 2016
New Revision: 297193
URL: https://svnweb.freebsd.org/changeset/base/297193

Log:
  to_flags is currently a 64-bit integer; however, we only use 7 bits.
  Furthermore, there is no reason this needs to be a 64-bit integer
  for the forseeable future.
  
  Also, there is an inconsistency between to_flags and the mask in
  tcp_addoptions(). Before r195654, to_flags was a u_long and the mask in
  tcp_addoptions() was a u_int. r195654 changed to_flags to be a u_int64_t
  but left the mask in tcp_addoptions() as a u_int, meaning that these
  variables will only be the same width on platforms with 64-bit integers.
  
  Convert both to_flags and the mask in tcp_addoptions() to be explicitly
  32-bit variables. This may save a few cycles on 32-bit platforms, and
  avoids unnecessarily mixing types.
  
  Differential Revision:https://reviews.freebsd.org/D5584
  Reviewed by:  hiren
  MFC after:2 weeks
  Sponsored by: Juniper Networks

Modified:
  head/sys/netinet/tcp_output.c
  head/sys/netinet/tcp_var.h

Modified: head/sys/netinet/tcp_output.c
==
--- head/sys/netinet/tcp_output.c   Tue Mar 22 15:43:47 2016
(r297192)
+++ head/sys/netinet/tcp_output.c   Tue Mar 22 15:55:17 2016
(r297193)
@@ -1652,7 +1652,7 @@ tcp_setpersist(struct tcpcb *tp)
 int
 tcp_addoptions(struct tcpopt *to, u_char *optp)
 {
-   u_int mask, optlen = 0;
+   u_int32_t mask, optlen = 0;
 
for (mask = 1; mask < TOF_MAXOPT; mask <<= 1) {
if ((to->to_flags & mask) != mask)

Modified: head/sys/netinet/tcp_var.h
==
--- head/sys/netinet/tcp_var.h  Tue Mar 22 15:43:47 2016(r297192)
+++ head/sys/netinet/tcp_var.h  Tue Mar 22 15:55:17 2016(r297193)
@@ -364,7 +364,7 @@ struct tcpcb {
  * options in tcp_addoptions.
  */
 struct tcpopt {
-   u_int64_t   to_flags;   /* which options are present */
+   u_int32_t   to_flags;   /* which options are present */
 #defineTOF_MSS 0x0001  /* maximum segment size */
 #defineTOF_SCALE   0x0002  /* window scaling */
 #defineTOF_SACKPERM0x0004  /* SACK permitted */
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"