This function is really of marginal utility. This commit drops it and
makes the existing callers instead open a new pstream with the desired
dscp.
The ulterior motive here is that the set_dscp() function that actually sets
the DSCP on a socket really wants to know the address family (AF_INET vs.
AF_INET6). We could plumb that down through the stream code, and that's
one reasonable option, but I thought that simply eliminating some calls
to set_dscp() where we don't already have the address family handy was
another reasonable way to go.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
A new function vlog_insert_module() is introduced to avoid using
list_insert() from the vlog.h header.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
On Windows platform, TCP_NODELAY can only be set when TCP is established.
(This is an observed behavior and not written in any MSDN documentation.)
The current code does not create any problems while running unit tests
(because connections get established immediately) but is reportedly
observed while connecting to a different machine.
commit 8b76839(Move setsockopt TCP_NODELAY to when TCP is connected.)
made changes to call setsockopt with TCP_NODELAY after TCP is connected
only in lib/stream-ssl.c. We need the same change for stream-tcp too and
this commit does that.
Currently, a failure of setting TCP_NODELAY results in reporting
the error and then closing the socket. This commit changes that
behavior such that an error is reported if setting TCP_NODELAY
fails, but the connection itself is not torn down.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Commit a8d819675f3 (Remove stream, vconn, and rconn functions to get
local/remote IPs/ports.) removed the code that used the local socket
address but neglected to remove the code to fetch that address. This
commit removes the latter code also.
Reported-by: Eitan Eliahu <eliahue@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Eitan Eliahu <eliahue@vmware.com>
We currently have a poll_fd_wait_event(fd, wevent, events) function that
is used at places common to Windows and Linux where we have to wait on
sockets. On Linux, 'wevent' is always set as zero. On Windows, for sockets,
when we send both 'fd' and 'wevent', we associate them with each other for
'events' and then wait on 'wevent'. Also on Windows, when we only send 'wevent'
to this function, we would simply wait for all events for that 'wevent'.
There is a disadvantage with this approach.
* Windows clients need to create a 'wevent' and then pass it along. This
means that at a lot of places where we create sockets, we also are forced
to create a 'wevent'.
With this commit, we pass the responsibility of creating a 'wevent' to
poll_fd_wait() in case of sockets. That way, a client using poll_fd_wait()
is only concerned about sockets and not about 'wevents'. There is a potential
disadvantage with this change in that we create events more often and that
may have a performance penalty. If that turns out to be the case, we will
eventually need to create a pool of wevents that can be re-used.
In Windows, there are cases where we want to wait on a event (not
associated with any sockets) and then control it using functions
like SetEvent() etc. For that purpose, introduce a new function
poll_wevent_wait(). For this function, the client needs to create a event
and then pass it along as an argument.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-By: Ben Pfaff <blp@nicira.com>
On windows platform, TCP_NODELAY can only be set when TCP
is established. If the conection is not immediately returning
success, call it when state is changed from TCP_CONNECTING
to SSL_CONNECTING.
Signed-off-by: Linda Sun <lsun@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Otherwise the indeterminate 'wevent' could frustrate poll_fd_wait_at()'s
attempt to merge "poll_node"s for the same fd.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Gurucharan Shetty <gshetty@nicira.com>
So far, we log the kernel assigned port number when the port number is
not specified. On Windows, this happens multiple times because "unix"
sockets are implemented internally via TCP ports. This means that many tests,
specially the ovs-ofctl monitor tests, need to filter out the
additional messages. Doing that is not a big deal, but I think it will
keep manifesting in future tests added by Linux developers.
With this commit, we simply don't print the kernel assigned TCP ports
on Windows when done for "unix" sockets.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
For Windows sockets, one has to call closesocket() to
close the sockets.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This commit creates events and through poll_fd_wait_event()
associates them with socket file descriptors to get woken up
from poll_block().
Some other changes:
* Windows does not have sys/fcntl.h but has a fcntl.h
On Linux, there is fctnl.h too.
* include <openssl/applink.c> to handle different C-Runtime linking
of OVS and openssl libraries as suggested at
https://www.openssl.org/support/faq.html#PROG2
The above include will not be needed if we compile Open vSwitch with
/MD compiler option.
* SHUT_RDWR is equivalent to SD_BOTH on Windows.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Does not add IPv6 support for in-band control.
Co-authored-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Nandan Nivgune <nandan.nivgune@calsoftinc.com>
Signed-off-by: Abhijit Bhopatkar <abhijit.bhopatkar@calsoftinc.com>
Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
These functions don't have any ultimate users. The in-band control code
used to use them, but not anymore, so we might as well delete them all.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
This allows other libraries to use util.h that has already
defined NOT_REACHED.
Signed-off-by: Harold Lim <haroldl@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The MSVC C library printf() implementation does not support the 'z', 't',
'j', or 'hh' format specifiers. This commit changes the Open vSwitch code
to avoid those format specifiers, switching to standard macros from
<inttypes.h> where available and inventing new macros resembling them
where necessary. It also updates CodingStyle to specify the macros' use
and adds a Makefile rule to report violations.
Signed-off-by: Alin Serdean <aserdean@cloudbasesolutions.com>
Co-authored-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The OVS code has always made a distinction between the unencrypted (TCP)
and SSL port numbers for the OpenFlow and OVSDB protocols. The default
port numbers for both protocols has changed, and there continues to be
no distinction between the unencrypted and SSL versions. This
commit removes the distinction in port numbers. A future patch will
recognize the change in default port number.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
The underlying glibc interface is deprecated because the interface itself
is not thread-safe. That means that there's no way for a layer on top of
it to be thread-safe.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Flagged with: https://github.com/lyda/misspell-check
Run with: git ls-files | misspellings -f -
Signed-off-by: Andy Hill <hillad@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The administrator can request that OVSDB bind any available TCP port, but
in that case there is no easy way to find out what port it has bound. This
commit adds that information as the "bound_port" key in the "status"
column.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This is a straight search-and-replace, except that I also removed #include
<assert.h> from each file where there were no assert calls left.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
An ovs_be32 is a more obvious way to represent an IP address than a
pointer to one. It is also more type-safe, especially since "sparse" is
able to check that the argument is in network byte order.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Introduce set_dscp method to pstream.
This will be used by dynamic dscp change of listening socket.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This situation can and will happen, and we handle it successfully, so it's
not an error.
Bug #12922.
Reported-by: Scott Hendricks <shendricks@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
If we've already reported an error at this point, then we currently report
a no-match error also, but that doesn't add any useful information; it's
just noise in the log.
Signed-off-by: Ben Pfaff <blp@nicira.com>
We occasionally see OpenSSL fail to seed its random number generator in
heavily loaded hypervisors. I suspect the following scenario:
1. OpenSSL calls read() to get 32 bytes from /dev/urandom.
2. The kernel generates 10 bytes of randomness and copies it out.
3. A signal arrives (perhaps SIGALRM).
4. The kernel interrupts the system call to service the signal.
5. Userspace gets 10 bytes of entropy.
6. OpenSSL doesn't read again to get the final 22 bytes. Therefore
OpenSSL doesn't have enough entropy to consider itself initialized.
It never tries again, so we're stuck forever.
The only part I'm not entirely sure about is #6, because the OpenSSL code
is so hard to read.
Thanks to Alex Yip for suggesting that this might be a startup problem.
Bug #10164.
Reported-by: Ram Jothikumar <ram@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Casts are sometimes necessary. One common reason that they are necessary
is for discarding a "const" qualifier. However, this can impede
maintenance: if the type of the expression being cast changes, then the
presence of the cast can hide a necessary change in the code that does the
cast. Using CONST_CAST, instead of a bare cast, makes these changes
visible.
Inspired by my own work elsewhere:
http://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/cast.h#n80
Signed-off-by: Ben Pfaff <blp@nicira.com>
Replaced all instances of Nicira Networks(, Inc) to Nicira, Inc.
Feature #10593
Signed-off-by: Raju Subramanian <rsubramanian@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
There isn't a lot of value in sending inactivity probes on unix
sockets. This patch changes the default to disable them.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
The changes allow the user to specify a separate dscp value for the
controller connection and the manager connection. The value will take
effect on resetting the connections. If no value is specified a default
value of 192 is chosen for each of the connections.
Feature #10074
Requested-by: Rajiv Ramanathan <rramanathan@nicira.com>
Signed-off-by: Mehak Mahajan <mmahajan@nicira.com>
If the CA certificate changed and OVS added the new CA certificate, the
change was ineffective. Clearing the certificate store before adding the
new CA certificate fixes the problem.
I don't know exactly why this fixes the problem, but in my testing it does.
Bug #2921.
Reported-by: Dan Wendlandt <dan@nicira.com>
Reported-by: Pierre Ettori <pettori@nicira.com>
A user report possibly implicates problems reading the mtime of the CA
cert file. Until now, nothing has logged these errors. This commit adds
such logging.
NICS-9.
A user report shows the message "reading CA cert
/etc/openvswitch/vswitchd.cacert created by another process" appearing
hundreds of times over a long period of time in the log. The only way I
can see that this would happen is if update_ssl_config() returned false,
indicating that the CA cert does not need to be re-read because it has not
changed. This commit should prevent that from happening.
We don't want to simply skip calling update_ssl_config() in this case,
because then the next call to stream_ssl_set_ca_cert_file() would usually
re-read the CA certificate, which is a waste of time.
Also, we might as well rate-limit the message.
NICS-9.
GCC and glibc conspire to allow struct sockaddr_in * to be passed in
place of struct sockaddr *, but that's non-standard and we're better
off not taking advantage of it.
Found by sparse.
This matches what xmalloc() does. It will be handled better by a monitor
process (created with --monitor), which will restart the child instead of
exiting.
Attempting to call SSL_set_session() on every trip through the SSL
connection state machine seems like it could cause the session to be
re-set to the cached one even after the server has told us which session
is actually to be used.
In testing, this change didn't make any difference, but it seems seems like
the right thing to do.
Bug #4448.
A cached SSL session may only be used for new connections after the initial
connection has shut down. As far as I can tell, nothing in the OpenSSL
documentation actually comes out and says this, but it is implied by
various examples found around the web and doing it this way makes caching
work much more reliably in my testing.
Bug #4448.
In the finest OpenSSL tradition of putting important documentation only in
code comments, ssl/ssl_sess.c in the OpenSSL tree has the following comment
inside ssl_get_prev_session():
/* We can't be sure if this session is being used out of
* context, which is especially important for SSL_VERIFY_PEER.
* The application should have used SSL[_CTX]_set_session_id_context.
*
* For this error case, we generate an error instead of treating
* the event like a cache miss (otherwise it would be easy for
* applications to effectively disable the session cache by
* accident without anyone noticing).
*/
This meant that ovs-controller couldn't effectively cache SSL server
sessions and we got a weird error whenever ovs-vswitchd tried.
Bug #4448.
CC: David Tsai <dtsai@nicira.com>
CC: Jeremy Stribling <strib@nicira.com>
Until now, the collection of coverage counters supported by a given OVS
program was not specific to that program. That means that, for example,
even though ovs-dpctl does not have anything to do with mac_learning, it
still has a coverage counter for it. This is confusing, at best.
This commit fixes the problem on some systems, in particular on ones that
use GCC and the GNU linker. It uses the feature of the GNU linker
described in its manual as:
If an orphaned section's name is representable as a C identifier then
the linker will automatically see PROVIDE two symbols: __start_SECNAME
and __end_SECNAME, where SECNAME is the name of the section. These
indicate the start address and end address of the orphaned section
respectively.
Systems that don't support these features retain the earlier behavior.
This commit also fixes the annoyance that files that include coverage
counters must be listed on COVERAGE_FILES in lib/automake.mk.
This commit also fixes the annoyance that modifying any source file that
includes a coverage counter caused all programs that link against
libopenvswitch.a to relink, even programs that the source file was not
linked into. For example, modifying ofproto/ofproto.c (which includes
coverage counters) caused tests/test-aes128 to relink, even though
test-aes128 does not link again ofproto.o.
