Bug reported by: Atanu Ghosh <at...@acm.org>
I'll review + test this series soon~
On Fri, Feb 20, 2015 at 8:44 AM, Ben Pfaff <b...@nicira.com> wrote:
> The set_dscp() function, until now, tried to set the DSCP as IPv4 and as
> IPv6. This worked OK on Linux, where an ENOPROTOOPT error made it really
> clear which one was wrong, but FreeBSD uses EINVAL instead, which has
> multiple meanings and which it therefore seems somewhat risky to ignore.
> Instead, this commit just tries to set the correct address family's DSCP
> option.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> I'd like to credit whoever reported this bug, but I don't remember who it
> is. Can someone remind me?
>
> lib/socket-util.c | 44 ++++++++++++++++++++------------------------
> lib/socket-util.h | 4 ++--
> 2 files changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/lib/socket-util.c b/lib/socket-util.c
> index 8949da7..206e17b 100644
> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira,
> Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -101,11 +101,14 @@ setsockopt_tcp_nodelay(int fd)
> }
> }
>
> +/* Sets the DSCP value of socket 'fd' to 'dscp', which must be 63 or less.
> + * 'family' must indicate the socket's address family (AF_INET or
> AF_INET6, to
> + * do anything useful). */
> int
> -set_dscp(int fd, uint8_t dscp)
> +set_dscp(int fd, int family, uint8_t dscp)
> {
> + int retval;
> int val;
> - bool success;
>
> #ifdef _WIN32
> /* XXX: Consider using QoS2 APIs for Windows to set dscp. */
> @@ -115,29 +118,22 @@ set_dscp(int fd, uint8_t dscp)
> if (dscp > 63) {
> return EINVAL;
> }
> -
> - /* Note: this function is used for both of IPv4 and IPv6 sockets */
> - success = false;
> val = dscp << 2;
> - if (setsockopt(fd, IPPROTO_IP, IP_TOS, &val, sizeof val)) {
> - if (sock_errno() != ENOPROTOOPT) {
> - return sock_errno();
> - }
> - } else {
> - success = true;
> - }
> - if (setsockopt(fd, IPPROTO_IPV6, IPV6_TCLASS, &val, sizeof val)) {
> - if (sock_errno() != ENOPROTOOPT) {
> - return sock_errno();
> - }
> - } else {
> - success = true;
> - }
> - if (!success) {
> +
> + switch (family) {
> + case AF_INET:
> + retval = setsockopt(fd, IPPROTO_IP, IP_TOS, &val, sizeof val);
> + break;
> +
> + case AF_INET6:
> + retval = setsockopt(fd, IPPROTO_IPV6, IPV6_TCLASS, &val, sizeof
> val);
> + break;
> +
> + default:
> return ENOPROTOOPT;
> }
>
> - return 0;
> + return retval ? sock_errno() : 0;
> }
>
> /* Translates 'host_name', which must be a string representation of an IP
> @@ -470,7 +466,7 @@ inet_open_active(int style, const char *target,
> uint16_t default_port,
> /* The dscp bits must be configured before connect() to ensure that
> the
> * TOS field is set during the connection establishment. If set after
> * connect(), the handshake SYN frames will be sent with a TOS of 0.
> */
> - error = set_dscp(fd, dscp);
> + error = set_dscp(fd, ss.ss_family, dscp);
> if (error) {
> VLOG_ERR("%s: set_dscp: %s", target, sock_strerror(error));
> goto exit;
> @@ -611,7 +607,7 @@ inet_open_passive(int style, const char *target, int
> default_port,
> /* The dscp bits must be configured before connect() to ensure that
> the TOS
> * field is set during the connection establishment. If set after
> * connect(), the handshake SYN frames will be sent with a TOS of 0.
> */
> - error = set_dscp(fd, dscp);
> + error = set_dscp(fd, ss.ss_family, dscp);
> if (error) {
> VLOG_ERR("%s: set_dscp: %s", target, sock_strerror(error));
> goto error;
> diff --git a/lib/socket-util.h b/lib/socket-util.h
> index 5b94f20..1178fb8 100644
> --- a/lib/socket-util.h
> +++ b/lib/socket-util.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira,
> Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -30,7 +30,7 @@
> int set_nonblocking(int fd);
> void xset_nonblocking(int fd);
> void setsockopt_tcp_nodelay(int fd);
> -int set_dscp(int fd, uint8_t dscp);
> +int set_dscp(int fd, int family, uint8_t dscp);
>
> int lookup_ip(const char *host_name, struct in_addr *address);
> int lookup_ipv6(const char *host_name, struct in6_addr *address);
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
