Tested on Freebsd 9.3~ works fine, with the following addition:
Acked-by: Alex Wang <al...@nicira.com>
Thx a lot for fixing~~~
diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 1af6474..416784e 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -213,14 +213,16 @@ def inet_open_active(style, target, default_port,
dscp):
is_addr_inet = is_valid_ipv4_address(address[0])
if is_addr_inet:
sock = socket.socket(socket.AF_INET, style, 0)
+ family = socket.AF_INET
else:
sock = socket.socket(socket.AF_INET6, style, 0)
+ family = socket.AF_INET6
except socket.error, e:
return get_exception_errno(e), None
try:
set_nonblocking(sock)
- set_dscp(sock, dscp)
+ set_dscp(sock, family, dscp)
try:
sock.connect(address)
except socket.error, e:
@@ -292,21 +294,20 @@ def set_nonblocking(sock):
% os.strerror(get_exception_errno(e)))
-def set_dscp(sock, dscp):
+def set_dscp(sock, family, dscp):
if dscp > 63:
raise ValueError("Invalid dscp %d" % dscp)
- # Note: this function is used for both of IPv4 and IPv6 sockets
- success = False
val = dscp << 2
- try:
- sock.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, val)
- except socket.error, e:
- if get_exception_errno(e) != errno.ENOPROTOOPT:
+ if family == socket.AF_INET:
+ try:
+ sock.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, val)
+ except socket.error, e:
raise
- success = True
- try:
- sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_TCLASS, val)
- except socket.error, e:
- if get_exception_errno(e) != errno.ENOPROTOOPT or not success:
+ elif family == socket.AF_INET6:
+ try:
+ sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_TCLASS, val)
+ except socket.error, e:
raise
+ else:
+ raise
On Fri, Feb 20, 2015 at 9:55 AM, Alex Wang <al...@nicira.com> wrote:
>
> 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
