Re: bgpd, protability and sockaddr sa_len

2019-02-15 Thread William Ahern
On Fri, Feb 15, 2019 at 03:07:15PM +0100, Claudio Jeker wrote:
> Another diff to ease portability of bgpd. The sa_len field in struct
> sockaddr does not exist on Linux so instead of using it pass a length to
> the function (e.g. like bind(2) and connect(2) and do the same when
> passing around struct sockaddr_storage in the listener case.
> The remaining sa_len, sin_len, sin6_len and ss_len usages are in very
> OpenBSD specific files.
> 
> -- 
> :wq Claudio
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.369
> diff -u -p -r1.369 bgpd.h
> --- bgpd.h15 Feb 2019 11:38:06 -  1.369
> +++ bgpd.h15 Feb 2019 13:51:28 -
> @@ -220,11 +220,12 @@ struct bgpd_addr {
>  #define  LISTENER_LISTENING  0x02
>  
>  struct listen_addr {
> - TAILQ_ENTRY(listen_addr) entry;
> - struct sockaddr_storage  sa;
> - int  fd;
> - enum reconf_action   reconf;
> - u_int8_t flags;
> + TAILQ_ENTRY(listen_addr)entry;
> + struct sockaddr_storage sa;
> + int fd;
> + enum reconf_action  reconf;
> + socklen_t   sa_len;
> + u_int8_tflags;
>  };

What's the use of maintaining and passing around sa_len if the sa member is
a fixed size? (Well, other than being a more straightforward patch.)

AFAIK the only variably sized sockaddr structure is sockaddr_un. Domain
socket paths can be longer than what sockaddr_un (or sockaddr_storage) can
nominally fit, but if the sa member is a fixed size then it's irrelevant and
you can always derive the sa object size from .sa_family or .sun_family +
.sun_path.



Re: bgpd, protability and sockaddr sa_len

2019-02-15 Thread Sebastian Benoit
ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.02.15 15:07:15 +0100:
> Another diff to ease portability of bgpd. The sa_len field in struct
> sockaddr does not exist on Linux so instead of using it pass a length to
> the function (e.g. like bind(2) and connect(2) and do the same when
> passing around struct sockaddr_storage in the listener case.
> The remaining sa_len, sin_len, sin6_len and ss_len usages are in very
> OpenBSD specific files.
> 
> -- 
> :wq Claudio
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.369
> diff -u -p -r1.369 bgpd.h
> --- bgpd.h15 Feb 2019 11:38:06 -  1.369
> +++ bgpd.h15 Feb 2019 13:51:28 -
> @@ -220,11 +220,12 @@ struct bgpd_addr {
>  #define  LISTENER_LISTENING  0x02
>  
>  struct listen_addr {
> - TAILQ_ENTRY(listen_addr) entry;
> - struct sockaddr_storage  sa;
> - int  fd;
> - enum reconf_action   reconf;
> - u_int8_t flags;
> + TAILQ_ENTRY(listen_addr)entry;
> + struct sockaddr_storage sa;
> + int fd;
> + enum reconf_action  reconf;
> + socklen_t   sa_len;
> + u_int8_tflags;
>  };
>  
>  TAILQ_HEAD(listen_addrs, listen_addr);
> @@ -1254,7 +1255,7 @@ int  set_equal(const struct 
> set_table 
>  /* util.c */
>  const char   *log_addr(const struct bgpd_addr *);
>  const char   *log_in6addr(const struct in6_addr *);
> -const char   *log_sockaddr(struct sockaddr *);
> +const char   *log_sockaddr(struct sockaddr *, socklen_t);
>  const char   *log_as(u_int32_t);
>  const char   *log_rd(u_int64_t);
>  const char   *log_ext_subtype(u_int8_t, u_int8_t);
> @@ -1288,7 +1289,7 @@ int  aid2afi(u_int8_t, u_int16_t *, u_i
>  int   afi2aid(u_int16_t, u_int8_t, u_int8_t *);
>  sa_family_t   aid2af(u_int8_t);
>  int   af2aid(sa_family_t, u_int8_t, u_int8_t *);
> -struct sockaddr  *addr2sa(struct bgpd_addr *, u_int16_t);
> +struct sockaddr  *addr2sa(struct bgpd_addr *, u_int16_t, socklen_t *);
>  void  sa2addr(struct sockaddr *, struct bgpd_addr *);
>  uint64_t  ift2ifm(uint8_t);
>  const char *  get_media_descr(uint64_t);
> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 config.c
> --- config.c  12 Feb 2019 09:00:21 -  1.81
> +++ config.c  15 Feb 2019 13:43:42 -
> @@ -397,7 +397,7 @@ prepare_listeners(struct bgpd_config *co
>   la->fd = -1;
>   la->flags = DEFAULT_LISTENER;
>   la->reconf = RECONF_REINIT;
> - la->sa.ss_len = sizeof(struct sockaddr_in);
> + la->sa_len = sizeof(struct sockaddr_in);
>   ((struct sockaddr_in *)&la->sa)->sin_family = AF_INET;
>   ((struct sockaddr_in *)&la->sa)->sin_addr.s_addr =
>   htonl(INADDR_ANY);
> @@ -409,7 +409,7 @@ prepare_listeners(struct bgpd_config *co
>   la->fd = -1;
>   la->flags = DEFAULT_LISTENER;
>   la->reconf = RECONF_REINIT;
> - la->sa.ss_len = sizeof(struct sockaddr_in6);
> + la->sa_len = sizeof(struct sockaddr_in6);
>   ((struct sockaddr_in6 *)&la->sa)->sin6_family = AF_INET6;
>   ((struct sockaddr_in6 *)&la->sa)->sin6_port = htons(BGP_PORT);
>   TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
> @@ -437,24 +437,25 @@ prepare_listeners(struct bgpd_config *co
>   &opt, sizeof(opt)) == -1)
>   fatal("setsockopt SO_REUSEADDR");
>  
> - if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa.ss_len) ==
> + if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa_len) ==
>   -1) {
>   switch (la->sa.ss_family) {
>   case AF_INET:
>   log_warn("cannot bind to %s:%u",
> - log_sockaddr((struct sockaddr *)&la->sa),
> - ntohs(((struct sockaddr_in *)
> + log_sockaddr((struct sockaddr *)&la->sa,
> + la->sa_len), ntohs(((struct sockaddr_in *)
>   &la->sa)->sin_port));
>   break;
>   case AF_INET6:
>   log_warn("cannot bind to [%s]:%u",
> - log_sockaddr((struct sockaddr *)&la->sa),
> - ntohs(((struct sockaddr_in6 *)
> + log_sockaddr((struct sockaddr *)&la->sa,
> + la->sa_len), ntohs(((struct sockaddr_in6 *

bgpd, protability and sockaddr sa_len

2019-02-15 Thread Claudio Jeker
Another diff to ease portability of bgpd. The sa_len field in struct
sockaddr does not exist on Linux so instead of using it pass a length to
the function (e.g. like bind(2) and connect(2) and do the same when
passing around struct sockaddr_storage in the listener case.
The remaining sa_len, sin_len, sin6_len and ss_len usages are in very
OpenBSD specific files.

-- 
:wq Claudio

Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.369
diff -u -p -r1.369 bgpd.h
--- bgpd.h  15 Feb 2019 11:38:06 -  1.369
+++ bgpd.h  15 Feb 2019 13:51:28 -
@@ -220,11 +220,12 @@ struct bgpd_addr {
 #defineLISTENER_LISTENING  0x02
 
 struct listen_addr {
-   TAILQ_ENTRY(listen_addr) entry;
-   struct sockaddr_storage  sa;
-   int  fd;
-   enum reconf_action   reconf;
-   u_int8_t flags;
+   TAILQ_ENTRY(listen_addr)entry;
+   struct sockaddr_storage sa;
+   int fd;
+   enum reconf_action  reconf;
+   socklen_t   sa_len;
+   u_int8_tflags;
 };
 
 TAILQ_HEAD(listen_addrs, listen_addr);
@@ -1254,7 +1255,7 @@ intset_equal(const struct 
set_table 
 /* util.c */
 const char *log_addr(const struct bgpd_addr *);
 const char *log_in6addr(const struct in6_addr *);
-const char *log_sockaddr(struct sockaddr *);
+const char *log_sockaddr(struct sockaddr *, socklen_t);
 const char *log_as(u_int32_t);
 const char *log_rd(u_int64_t);
 const char *log_ext_subtype(u_int8_t, u_int8_t);
@@ -1288,7 +1289,7 @@ intaid2afi(u_int8_t, u_int16_t *, u_i
 int afi2aid(u_int16_t, u_int8_t, u_int8_t *);
 sa_family_t aid2af(u_int8_t);
 int af2aid(sa_family_t, u_int8_t, u_int8_t *);
-struct sockaddr*addr2sa(struct bgpd_addr *, u_int16_t);
+struct sockaddr*addr2sa(struct bgpd_addr *, u_int16_t, socklen_t *);
 voidsa2addr(struct sockaddr *, struct bgpd_addr *);
 uint64_tift2ifm(uint8_t);
 const char *get_media_descr(uint64_t);
Index: config.c
===
RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
retrieving revision 1.81
diff -u -p -r1.81 config.c
--- config.c12 Feb 2019 09:00:21 -  1.81
+++ config.c15 Feb 2019 13:43:42 -
@@ -397,7 +397,7 @@ prepare_listeners(struct bgpd_config *co
la->fd = -1;
la->flags = DEFAULT_LISTENER;
la->reconf = RECONF_REINIT;
-   la->sa.ss_len = sizeof(struct sockaddr_in);
+   la->sa_len = sizeof(struct sockaddr_in);
((struct sockaddr_in *)&la->sa)->sin_family = AF_INET;
((struct sockaddr_in *)&la->sa)->sin_addr.s_addr =
htonl(INADDR_ANY);
@@ -409,7 +409,7 @@ prepare_listeners(struct bgpd_config *co
la->fd = -1;
la->flags = DEFAULT_LISTENER;
la->reconf = RECONF_REINIT;
-   la->sa.ss_len = sizeof(struct sockaddr_in6);
+   la->sa_len = sizeof(struct sockaddr_in6);
((struct sockaddr_in6 *)&la->sa)->sin6_family = AF_INET6;
((struct sockaddr_in6 *)&la->sa)->sin6_port = htons(BGP_PORT);
TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
@@ -437,24 +437,25 @@ prepare_listeners(struct bgpd_config *co
&opt, sizeof(opt)) == -1)
fatal("setsockopt SO_REUSEADDR");
 
-   if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa.ss_len) ==
+   if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa_len) ==
-1) {
switch (la->sa.ss_family) {
case AF_INET:
log_warn("cannot bind to %s:%u",
-   log_sockaddr((struct sockaddr *)&la->sa),
-   ntohs(((struct sockaddr_in *)
+   log_sockaddr((struct sockaddr *)&la->sa,
+   la->sa_len), ntohs(((struct sockaddr_in *)
&la->sa)->sin_port));
break;
case AF_INET6:
log_warn("cannot bind to [%s]:%u",
-   log_sockaddr((struct sockaddr *)&la->sa),
-   ntohs(((struct sockaddr_in6 *)
+   log_sockaddr((struct sockaddr *)&la->sa,
+   la->sa_len), ntohs(((struct sockaddr_in6 *)
&la->sa)->sin6_port));
break;
default: