Date: Mon, 14 Jan 2013 16:42:16 -0500 From: Alfred Perlstein <bright@mu.org> To: John Baldwin <jhb@freebsd.org> Cc: net@freebsd.org Subject: Re: [PATCH] Don't imply TCP and UDP socket options are bitmasks Message-ID: <50F47BB8.9000409@mu.org> In-Reply-To: <201301141550.13577.jhb@freebsd.org> References: <201301141550.13577.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Wouldn't a comment over the code suffice? Something like your email as a header would actually work very nicely! I think just using decimal would be more confusing than explicitly calling it out like: /* begin enumerated (not bitmask) socket option specifiers */ #define TCP_MAXSEG 0x02 /* set maximum segment size */ #define TCP_NOPUSH 0x04 /* don't push last block of write */ #define TCP_NOOPT 0x08 /* don't use TCP options */ #define TCP_MD5SIG 0x10 /* use MD5 digests (RFC2385) */ /* end enumerated socket option specifiers */ On 1/14/13 3:50 PM, John Baldwin wrote: > The constants used for TCP and UDP socket options (TCP_NODELAY, etc.) are > currently defined as hex values that are individual bits. However, socket > options are never masked together, they are used as a simple enumeration of > discrete values. Using a bitmask forces us to run out of bits and makes it > harder for vendors to try to use a high range of values for local custom > options (hoping that they never conflict with a new option value added in > stock FreeBSD). > > The socket options in <sys/socket.h> do use bitmasks for the low bits because > they map directly to bits so_options, but then they start a simple enumeration > at 0x1000. TCP and UDP socket options do not directly map to bits in a flags > field in the PCB (e.g. TF_NODELAY != TCP_NODELAY). I would like to change the > representation of the constants to be decimal instead of hex and encourage new > options to fill in the gaps between the existing values. This would preserve > the existing ABI but keep things more sane in the future (I believe). The > diff is this: > > Index: netinet/tcp.h > =================================================================== > --- netinet/tcp.h (revision 245225) > +++ netinet/tcp.h (working copy) > @@ -151,18 +151,18 @@ > /* > * User-settable options (used with setsockopt). > */ > -#define TCP_NODELAY 0x01 /* don't delay send to coalesce packets */ > +#define TCP_NODELAY 1 /* don't delay send to coalesce packets */ > #if __BSD_VISIBLE > -#define TCP_MAXSEG 0x02 /* set maximum segment size */ > -#define TCP_NOPUSH 0x04 /* don't push last block of write */ > -#define TCP_NOOPT 0x08 /* don't use TCP options */ > -#define TCP_MD5SIG 0x10 /* use MD5 digests (RFC2385) */ > -#define TCP_INFO 0x20 /* retrieve tcp_info structure */ > -#define TCP_CONGESTION 0x40 /* get/set congestion control algorithm */ > -#define TCP_KEEPINIT 0x80 /* N, time to establish connection */ > -#define TCP_KEEPIDLE 0x100 /* L,N,X start keeplives after this period */ > -#define TCP_KEEPINTVL 0x200 /* L,N interval between keepalives */ > -#define TCP_KEEPCNT 0x400 /* L,N number of keepalives before close */ > +#define TCP_MAXSEG 2 /* set maximum segment size */ > +#define TCP_NOPUSH 4 /* don't push last block of write */ > +#define TCP_NOOPT 8 /* don't use TCP options */ > +#define TCP_MD5SIG 16 /* use MD5 digests (RFC2385) */ > +#define TCP_INFO 32 /* retrieve tcp_info structure */ > +#define TCP_CONGESTION 64 /* get/set congestion control algorithm */ > +#define TCP_KEEPINIT 128 /* N, time to establish connection */ > +#define TCP_KEEPIDLE 256 /* L,N,X start keeplives after this period */ > +#define TCP_KEEPINTVL 512 /* L,N interval between keepalives */ > +#define TCP_KEEPCNT 1024 /* L,N number of keepalives before close */ > > #define TCP_CA_NAME_MAX 16 /* max congestion control name length */ > > Index: netinet/udp.h > =================================================================== > --- netinet/udp.h (revision 245225) > +++ netinet/udp.h (working copy) > @@ -48,7 +48,7 @@ > /* > * User-settable options (used with setsockopt). > */ > -#define UDP_ENCAP 0x01 > +#define UDP_ENCAP 1 > > > /* >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50F47BB8.9000409>