| Age | Commit message (Collapse) | Author |
|---|
|
This also sets the dates for 1.8.0, even though it was an internal-only
release.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
In vlan_insert_tag(), we insert a 4-byte VLAN header _after_
mac header:
memmove(skb->data, skb->data + VLAN_HLEN, 2 * ETH_ALEN);
...
veth->h_vlan_proto = htons(ETH_P_8021Q);
...
veth->h_vlan_TCI = htons(vlan_tci);
so after it, we should recompute the checksum to include these 4 bytes.
skb->data still points to the mac header, therefore VLAN header is at
(2 * ETH_ALEN = 12) bytes after it, not (ETH_HLEN = 14) bytes.
This can also be observed via tcpdump:
0x0000: ffff ffff ffff 5254 005d 6f6e 8100 000a
0x0010: 0806 0001 0800 0604 0001 5254 005d 6f6e
0x0020: c0a8 026e 0000 0000 0000 c0a8 0282
Similar for __pop_vlan_tci(), the vlan header we remove is the one
overwritten in:
memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
Therefore the VLAN_HLEN = 4 bytes after 2 * ETH_ALEN is the part
we want to sub from checksum.
Cc: David S. Miller <davem@davemloft.net>
Cc: Jesse Gross <jesse@nicira.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
Ensure that the buffer returned by ofpmp_reserve() has buf->l2 set
as this may be required by nxm_reg_load_to_nxast() when generating
the reply to an stats request
This problem was observed when dumping a large number of flows
with set_field actions using ovs-ofctl dump-flows.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Co-authored-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Simon Horman <horms@verge.net.au>
|
|
If the pointer does not represent an error then the PTR_ERR macro may still
return a nonzero value. The fix is the same as in ovs_vport_cmd_set.
Signed-off-by: Rich Lane <rlane@bigswitch.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
Suggested-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
Some tunnel code in OVS (for example, CAPWAP) uses the skb->cb to
store information while processing packets. However, if we don't
find an appropriate tunnel port on receive, then we send an ICMP
port unreachable message, which calls back into the IP stack. The
stack assumes that skb->cb will still contain valid information
about from the IP layer, including any IP options. As a result,
icmp_echo_options() can read the garbage values from OVS and
overwrite data on the stack, panicing the machine.
This simply stops sending ICMP messages when ports are not found.
Many people find them confusing and flow based tunneling will
never send them (since it always finds a port) so it solves both
problems at once.
Bug #14880
Reported-by: Deepesh Govindan <dgovindan@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
Conflicts:
datapath/vport-vxlan.c
|
|
It should be possible to feed to output of "ovs-ofctl dump-flows" to
"ovs-ofctl add-flows". However, some of the metadata needs to be
ignored. "idle_age" and "hard_age" was recently added to the output of
"ovs-ofctl dump-flows", but they were not ignored like the other
metadata. This commit ignores them.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
Found by Coverity.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Found by Coverity.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Found by Coverity.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
The 'maskp' parameter to this function can be NULL, but the function
always dereferenced it. This commit fixes the problem.
This commit also fixes the order in which the value and mask were adjusted
to correctly discard 1-bits outside of FLOW_NW_FRAG_MASK.
Found by Coverity.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Depending on how forcefully the parent process is killed, the worker
could abort when trying to read or write on their shared socket. This
changes those errors from VLOG_ABORT to VLOG_FATAL so that a core isn't
generated.
Bug #14821
Reported-by: Amey Bhide <abhide@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
In Open vSwitch, a "modify" or "modify_strict" flow_mod is supposed to
refresh the flow's last-modified time even if nothing else changes, because
this interpretation makes the "learn" action more useful. As commit
308881afb (ofproto: Reinterpret meaning of OpenFlow hard timeouts with
OFPFC_MODIFY.) notes:
I finally found a good use for hard timeouts in OpenFlow, but they
require a slight reinterpretation of the meaning of hard timeouts.
Until now, a hard timeout meant that a flow would be removed the
specified number of seconds after a flow was created. Intervening
modifications with OFPFC_MODIFY(_STRICT) had no effect on the hard
timeout; the flow would still be deleted the specified number of
seconds after its original creation.
This commit changes the effect of OFPFC_MODIFY(_STRICT). Now,
modifying a flow resets its hard timeout counter. A flow will time out
the specified number of seconds after creation or after the last time
it is modified, whichever comes later.
However, commit 080437614b (ofproto: Represent flow cookie changes as
operations too.) broke this behavior because it incorrectly optimized out
"modify" operations that didn't change the flow's actions or flow cookie.
This commit fixes the problem, and adds a test to prevent future
regression.
Thanks to Amar Padmanabhan <amar@nicira.com> for helping to track this
down.
Bug #14841.
Reported-by: Hiroshi Tanaka <htanaka@vmware.com>
CC: Amar Padmanabhan <amar@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Following patch memset ovs_key_ipv4_tunnel padding area so that
packets from a flow would be mapped to same flow in kernel datapath
flow table.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #14843
|
|
The current method of calculating the ingress policer rate
can lead to inaccuracy if ingress_policing_rate is set to
a smallish values because the rate is divided by 8 first
which causes rounding errors.
Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
The use of OpenFlow controllers may require for a subset of
the network (typically the management network intrerface) to be
brought up before the openvswitch service is started.
The newly introduced key "OVSREQUIRES" allows to specify a
list of interfaces that need the be brought up before the
openvswitch service is autostarted. It is also possible to
build a chain of bridge dependencies.
TYPE=OVSBridge
OVSREQURIES="em1"
[...]
A special UPPEDSTACK var that is passed along avoids getting
lost in dependency loops.
Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
interface up or down
This patch modifies the ifup/ifdown scripts to automatically
start the openvswitch service before ovs-vsctl is invoked thus
not making it mandatory to auto-start openvswitch on boot.
Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Messages about packets being lost are logged at level WARN, but when
they were generated at a high rate, those consolidated messages were
logged at ERR. This changes to consolidated messages to be logged at
WARN, too.
Thanks to Ben Pfaff for quickly suggesting the culprit.
Bug #14783
Reported-by: James Schmidt <jschmidt@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
ovsdb_session_destroy() was called twice but it should only be called once.
This double-free is unlikely to cause problems in practice because it only
triggers if there were ever more than two outstanding requests in the
session at a time (because the only data being freed is an hmap, which
does not allocate any heap memory unless the hmap has more than two
elements).
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
With rotates instead of shifts, the upper and lower 16 bits of the returned
hash are always the same.
I noticed this while working on replacing Jenkins hash by murmurhash,
because some of the database unit tests started failing, e.g. "row
hashing (scalars)".
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Currently, if there isn't enough space to store the actions in a
flow during a dump we return -ENOMEM. However, the standard error
in this situation is -EMSGSIZE so this changes the behavior to match.
This issue was introduced in 354d4c98a8cdaae3525848f564e58a9016bcd3af
(datapath: Fix nelink attribute size for flow.).
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
|
|
After commit 9b405f1aa8d175dc63ad3ffe5d0fe05d5ee09162 (datapath: More
flexible kernel/userspace tunneling attribute.), it was possible for a
flow dump to return a partial action list. It's better to return no
action list at all in this situation since then userspace will know
that it should request the full thing if it wants rather than have
incorrect results. Therefore, this prevents those partial lists in
situations where we have a very large number of actions.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
integrity only."
This reverts commit 00c7faf3e5b7d4020e995a1429cf94313f197171.
In general, it should not be possible have a NULL return value from
skb_gso_segment() since we're not actually trying to verify the
header integrity. No other callers with similar needs have NULL
checks. The actual cause of the problem was LRO packets, which
OVS isn't equipped to handle. The commit
33e031e99cc630baf1b0cb9256710dee7d9ab66d (datapath: Move LRO check
from transmit to receive.) solves that problem by fixing the LRO
check. In order to avoid possibly masking any other problems, this
reverts the GSO check which should no longer be needed.
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
Commit 24b019f808211a95078efd916064af0975ca5733 (datapath: Disable
LRO from userspace instead of the kernel.) accidentally moved the
check for LRO packets from the receive path to transmit. Since
this check is supposed to protect OVS (and other parts of the system)
from packets that it cannot handle it is obviously not useful on
egress. Therefore, this commit moves it back to the receive side.
The primary problem that this caused is upcalls to userspace tried
to segment the packet even though no segmentation information is
available. This would later cause NULL pointer dereferences when
skb_gso_segment() did nothing.
Bug #14772
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
|
|
skb_gso_segment() has the following comment:
* It may return NULL if the skb requires no segmentation. This is
* only possible when GSO is used for verifying header integrity.
Somehow queue_gso_packets() has never hit this case before, but some
failures have suddenly been reported. This commit should fix the problem.
Additional commentary by Jesse: We shouldn't normally be hitting this case
because we're actually trying to do GSO, not header validation. However, I
guess the guest/backend must be generating a packet with an MSS, which
tricks us into thinking that it's GSO, but no GSO is actually requested.
In the case of the bridge, header validation does take place so the
situation is handled already. It seems not ideal that the network backend
doesn't sanitize these packets but it's probably good that we handle
it in any case.
Bug #14772.
Reported-by: Deepesh Govindan <dgovindan@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
Following patch fixes flow buffer size calculation to allocate
sufficient memory for all nested attributes in new tunnel
attribute.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Bug #14767
|
|
Following patch adds null check while inserting new netlink attribute.
This was introduced by commit 9b405f1aa8d175d (datapath: More
flexible kernel/userspace tunneling attribute.)
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Bug #14767
|
|
Following patch breaks down single ipv4_tunnel netlink attribute into
individual member attributes. It will help when we extend tunneling
parameters in future.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #14611
|
|
Add Linux 3.8 kernel to the range of supported kernel versions.
Signed-off-by: James Page <james.page@ubuntu.com>
[jesse: Update NEWS and FAQ]
Signed-off-by: Jesse Gross <jesse@nicira.com>
Conflicts:
FAQ
|
|
The version.h is moved from include/linux/version.h to
include/generated/uapi/linux/version.h.
So check both pathes.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
The following call stack was possible:
poll_block()
-> vlog
-> worker_send_iovec()
-> poll_block()
which caused corruption because poll_block() is not reentrant.
Bug #14616.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
The following call stack was possible:
vlog
-> worker_request()
-> poll_block()
-> vlog
-> worker_request()
which caused problems because worker_request() is not reentrant. In a
little more detail, the second worker_request() shoves its RPC protocol
data into the middle of the first. This means that, first, you get
some binary crud in the log (the header for the second RPC). And,
second, text from the first RPC log message gets treated by the worker
as the subsequent RPC's header. That, in turn, typically causes the
worker to try to xmalloc() a huge number of bytes (0x20000000 or more,
since "space" has ASCII value 0x20), which causes the worker to die
with "virtual memory exhausted". The main process then dies because
the worker's death closes the socket it uses to communicate with it
("connection reset").
Bug #14616.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Also, use ovs_strlcpy() instead of strcpy() just to be a teensy bit safer.
Found by valgrind.
Bug #14357.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Also, add an assertion that the field is the expected size.
This bug was introduced in commit 2fdf762a006f (vswitchd: Log all tunnel
parameters of given flow.)
Found by valgrind.
Bug #14357.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Bug #14357.
Reported-by: Luca Giraudo <lgiraudo@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Signed-off-by: Jarno Rajahalme <jarno.rajahalme@nsn.com>
[jesse: correct return type of get_u32_or_zero()]
Signed-off-by: Jesse Gross <jesse@nicira.com>
Conflicts:
datapath/tunnel.c
include/openvswitch/tunnel.h
lib/netdev-vport.c
|
|
When a packet is received on a tunnel the pad member is currently
left uninitialized. This didn't previously cause problems because
userspace didn't interprete the IPV4_TUNNEL attribute and blindly
copied back the uninitialized data. However, now that userspace
knows how to serialize this attribute it was zeroing it out, which
prevented flows that had been previously installed from being
deleted. In addition to zeroing out the padding on packet reception,
it also does the same thing on flow setup since we should be ignoring
the value.
Reported-by: Anand Krishnamurthy <krishnamurt4@wisc.edu>
Reported-by: Saul St. John <sstjohn@cs.wisc.edu>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Conflicts:
datapath/flow.c
|
|
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
bug #14341
|
|
When I wrote the "trap" calls in ovs-ctl, I had the mistaken notion that
"trap $cmd $signal" would execute $cmd and then exit when $signal was
caught. This is incorrect. Instead, it executes $cmd and then resumes
executing the shell script.
On the other hand, "trap $cmd 0" does by itself what I wanted: it causes
the shell to execute $cmd and then exits due to the signal. So this commit
changes the offending traps to use this form.
Bug #14290.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
|
|
the following up of 15e473046cb6e5d18a4d0057e61d76315230382b
This patch replaces pid with portid under datapath/linux/compat
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
datapath: backport 15e473046cb6e5d18a4d0057e61d76315230382b
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
[jesse: fix kernel version in error message]
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
Fixes the following sparse warnings:
meta-flow.c:947:21: warning: incorrect type in assignment (different base types)
meta-flow.c:947:21: expected restricted __be32 [usertype] be32
meta-flow.c:947:21: got unsigned int const [unsigned] [usertype] skb_priority
meta-flow.c:951:21: warning: incorrect type in assignment (different base types)
meta-flow.c:951:21: expected restricted __be32 [usertype] be32
meta-flow.c:951:21: got unsigned int const [unsigned] [usertype] skb_mark
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
This patch adds logging support for skb_mark and skb_priority.
Acked-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Conflicts:
lib/ofp-util.c
tests/ofproto-dpif.at
|
|
This function can be implemented as a trivial wrapper around
mf_get_value(), which I hadn't noticed before, so it's better to do it
that way. Also, examining the code that is removed, it had some bugs in
it (for example, all MFF_TUN_* fields were treated as if they were
MFF_TUN_ID) which mf_get_value() does not have, so this fixes bugs too.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
When we are searching for a tunnel port to receive traffic on,
everything should be zeroed out by the time that we get to null
ports since they are wildcarded. However, if certain other ports
also exist (primarily multicast ports with keys) then this might
not be the case and the key can be set.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
|
|
If a negative number is supplied, the parsing code used to convert it
into a signed one. We ran into an incident where a third-party script
was attempting to get the OpenFlow port number for an interface, but got
-1 from the database, since the number had not yet been assigned. This
was converted to 65535, which maps to OFPP_NONE and all flows with
ingress port OFPP_NONE were modified. This commit disallows negative
port numbers to help prevent broken integration scripts from disturbing
the flow table.
Issue #14036
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
To keep control+C and other signals in the initiating session from killing
the monitor process, we need to put the monitor process into its own
session. However, until this point, we've only done that for the daemon
processes that the monitor started, which means that control+C would kill
the monitor but not the daemons that it launched.
I don't know of a benefit to putting the monitor and daemon processes in
different sessions, as opposed to one new session for both of them, so
this change does the latter.
daemonize_post_detach() is called from one additional context where we'd
want to be in a new session, the worker_start() function, but that function
is documented as to be called after daemonize_start(), in which case we
will (after this commit) already have called setsid(), so no additional
change is required there.
Bug #14280.
Reported-by: Gordon Good <ggood@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Previously, ovs-ctl would determine which bridges to run "ovs-save
save-flows" on by running "ovs-vsctl list-br". In addition to real
bridges, that command also returns fake bridges. An error is returned
when "ovs-save save-flows" is run on a fake bridge. By using the newly
added "--real" flag to "ovs-vsctl list-br", we can get rid of that
unnecessary warning.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
