| Age | Commit message (Collapse) | Author |
|---|
|
Also make the table id arithmetic less confusing.
Signed-off-by: Jarno Rajahalme <jarno.rajahalme@nsn.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Commit 52a90c29 (Implement new "VLAN splinters" feature) passed in OpenFlow
port number to vsp_realdev_to_vlandev() function which asks for datapath port
number.
This patch fixes this bug by making the vsp_realdev_to_vlandev() function
take in and return OpenFlow port number.
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
When the OFPC_FRAG_DROP policy is in effect, IP fragments are supposed to
be dropped before they reach the flow table. Open vSwitch properly dropped
IP fragments in this case, but still accounted them to the packet and byte
counters for the flow that they would have hit if the OFPC_FRAG_NX_MATCh
policy had been in effect.
Reported-by: love you <thunder.love07@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Originally no rule existed for packets that did not match an
OpenFlow flow and therefore every packet with a rule could be
counted as a hit. However, newer versions of OVS have hidden
miss rules so this is no longer true. To return the correct
table stats, this subtracts packets that hit the miss rule
from the total and removes the separate counter.
Reported-by: love you <thunder.love07@gmail.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
The xlate_actions() call in handle_flow_miss_without_facet() didn't
implement fin_timeout properly because tcp_flags wasn't getting set.
I have not tested that this fixes the problem, but it seems "obviously
correct".
Bug #16506.
Reported-by: Ying Chen <yingchen@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Until now, we were presenting a separate sFlow data-source (sampler) for
each ifIndex-interface. This caused problems with samples that did not
easily map to an ifIndex being aliased together and breaking the sFlow
containment rules. This patch changes the model to present a single sFlow
data-source for each bridge. Now we can still make all reasonable effort
to map packet samples to ingress/egress ifIndex numbers, knowing that the
fallback to "unknown" does not break the sFlow model. Note that
interface-counter-polling is still handled the same way as before, with
sFlow counter-polling data only being exported for ifIndex-interfaces.
Signed-off-by: Neil Mckee <neil.mckee@inmon.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
rule_get_stats() is often called when iterating over every rule in
the flow table. To ensure up-to-date statistics, rule_get_stats()
calls push_all_stats() which can cause flow misses to be handled.
When using the learn action, this can cause rules to be added (and
potentially removed) from the OpenFlow table. This could corrupt
the caller's data structures, leading to a segmentation fault.
This patch fixes the issue by disabling flow miss handling from
within rule_get_stats().
Bug #15999.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Fixes a bug where per ofproto moving average stats did not update
when there is no active dp flows.
Reported-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
In the standard case, rate limiting facet_learn() to once ever
500ms, makes sense. The worst that can happen is a learning entry
is expired half a second to early. However, when using
fin_timeouts, we really need react quickly to delete the newly
stale flow.
Bug #15915.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
The flow-eviction-threshold presents a trade off between the
expense of maintaining large numbers of datapath flows, and the
benefit of avoid unnecessary flow misses. In some large Open
vSwitch deployments, we've seen the previous default flow eviction
threshold negatively impact performance with reasonably typical
traffic patterns. This patch increases the default to a level
which should represent a better trade off: still relatively safe,
but much more amenable to large numbers of long lived flows.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
The most natural place to push facet statistics is in
update_stats() where they're pulled from the datapath. However,
under load, update_stats() can be called as many as 10 times per
second causing us to push statistics so frequently it hurts
performance. By pushing statistics much less frequently, this
patch generates a roughly 8% improvement in TCP_CRR performance.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
ofproto-dpif is responsible for quite a few book keeping tasks in
addition to handling flow misses. Many of these tasks (flow
expiration, flow revalidation, etc) can take many hundreds of
milliseconds, during which no misses can be handled. The ideal
long term solution to this problem, is to isolate flow miss
handling into it's own thread. However, for now this patch
provides a 5% increase in TCP_CRR performance, and smooths out
results during revalidations.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Commit bf1e8ff (ofproto-dpif: Push statistics in rule_get_stats()),
started down the road towards pushing stats on demand, but it
didn't go quite far enough. First, it neglected to push stats in
port_get_stats() and mirror_get_stats(). Second, it only pushes
stats for a single ofproto, making it incomplete when patch ports
are used.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
This patch adds more flow related stats to the output of
"ovs-appctl dpif/show". Specifically, the follow information
are added per ofproto:
- Max flow table size
- Average flow table size
- Average flow table add rate
- Average flow table delete rate
- Average flow entry life in milliseconds
Feature #15366
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
This is to fix the fallout of single datapath change.
ovs-appctl dpif/show displays per bridge miss, hit
and flow counts on the screen, but the backend is
obtaining those information from the datapath.
With a single datapath, all bridges of the same
datapath would all display the same (global)
counters maintained by the datapath, obviously
not correct.
This patch fixes the bug by maintaining per ofproto_dpif
miss and hit counts, which are used for display output.
The number of flows count is obtained by counting the
number facets per ofproto.
ovs-dpctl show still displays the counters maintain by
the datapath, as before.
Bug #15369
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
In the TCP_CRR benchmark, ovs-vswitchd spends so much time in
update_stats() that it has a significant impact on flow setup
performance. Further work is needed in this area, but for now,
simply rate limiting facet_learn() has a roughly 10% improvement
with complex flow tables.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
With complex flow tables, facet_check_consistency() can be
expensive enough to show up in flow setup performance benchmarks.
In my testing this patch gives us a roughly 10% improvement in
TCP_CRR and ovs-benchmark.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
As time goes on, and flow tables become more complicated, the
tradeoff between keeping up to date statistics, and the CPU
resources needed to maintain them, will become more important.
Commit 5c0243a (ofproto-dpif: xlate actions once with subfacets.)
delayed the reporting of some statistics in an effort to achieve
higher flow setup performance. Future commits will continue in the
same direction.
This patch helps to alleviate the issue, by pushing statistics
rule_get_stats(), when users actually want them. Presumably, this
happens rarely, and thus will not have a negative impact on
ovs-vswitchd performance.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Before this patch, when ofproto-dpif decided that a particular flow
miss needed a facet, it would do action translation multiple times.
Once in subfacet_make_actions(), and once per packet in
subfacet_update_stats(). In the common case (once per miss), this
would double the amount of work required in xlate_actions().
The call to facet_push_stats() in subfacet_update_stats() is
unnecessary. If the packets are simply accounted to the facet,
they will eventually be pushed to the relevant rules in
update_stats() or when the facet is removed. Removing the
unnecessary step gives us a 20% improvement of the netperf TCP_CRR
benchmark with the complex flow tables installed by our controller.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
After tunnel packet is unencapsulated we should unset IPsec flag from
skb_mark.
Otherwise, IPsec policies would be applied one more time on internal
interfaces, if there is one. This is especially necessary after we
will introduce global, low-priority IPsec drop policy that will make
sure that we never let through marked but unencrypted packets.
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Issue: 15074
|
|
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
If there is more than one bridge, then it's easy to specify the wrong one
on an ofproto/trace command. Previously, this would produce surprising
results. With this commit, "ofproto/trace" should silently fix up the
problem.
It would be nice to not require the user to specify a bridge at all, but
it's theoretically possible to have more than one backer, in which case we
need some way to distinguish, and a bridge name is as good an identifier
as we have. We could ask the user to specify the datapath_type, I guess,
but that's a less familiar name to most users and it would be a somewhat
gratuitous change in synatx for ofproto/trace.
Bug #15419.
Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
The command "ovs-appctl dpif/dump-flows" would print slow-path actions
as "drop", which could be confusing to users. This is different from
"ovs-dpctl dump-flows", which prints a descriptive reason. This commit
replaces "drop" with the reason.
Bug #14840
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
In the kernel tunnel implementation, if a packet was marked as ECN CE on
the outer packet then we would carry this over to the inner packet on
decapsulation. With the switch to flow based tunneling, this stopped
happening. This commit reintroduces that behavior by using the set IP
header action.
Bug #15072
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
Move the check for whether tunneled packets should be dropped due to
congestion encountered (CE) when the encapsulated packet is not ECN
capable (non-ECT). This also adds some additional tests for ECN
handling on tunnel decapsulation.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
When a packet arrives on an IP tunnel, store the TOS value for later
use. This value will be used in a couple of future commits.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
For VLAN splinters, an "initial_tci" value was introduced that is passed
around during flow processing to be used later for action translation.
This commit switches to passing around a struct so that additional
values beyond TCI can be used. A future commit will use this.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
The flow_push_stats() function will need other members of the "facet"
structure in a future commit.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
|
|
Since userspace flow based tunneling code is checked in, the kernel
port based tunneling code can be removed.
Patch removes following components:
- tunnel ports hash table and moved tunnel ports list to individual
vports.
- Cleaned per tnl-port config.
- OVS_KEY_ATTR_TUN_ID action is removed.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #15078
|
|
For most of the history of Open vSwitch, one could assume that a
given datapath flow key would consistently translate into the same
userspace struct flow representation. However, with the switch to
flow based tunneling, we now have a situation where a database
configuration change can cause a datapath flow key's in_port to
correspond to a completely different OpenFlow in_port possibly on a
completely different bridge. This can cause all sorts of problems,
including traffic black holes due to confused facet revalidations.
To solve the problem, this patch verifies that each facet's
subfacets still result in the appropriate struct flow. If a facet
fails this test, it is simply removed.
Bug #15213.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
When we fail to install a subfacet, there's not much we can do
other than note that it happened. However, doing this requires us
to maintain a pointer to a subfacet which theoretically could be
destroyed by facet_revalidate() later. This patch solves the
problem by simply assuming dpif_flow_put() always succeeds. This
should have no effect on behavior.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Due to flow based tunneling, we can no longer assume that it's
possible to reconstruct a subfacet's key from its facet's flow.
The flow's in_port may be stale due to tunnel configuration
changes.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Garbage collect tnl_backers during type_run(). Add new
tnl_backers if a VXLAN port's UDP port changes.
Signed-off-by: Kyle Mestery <kmestery@cisco.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Move dpif_backer->tnl_backers from a "struct sset" to a
"struct simap". Store odp_port in the new map. This will make it easier to
access the odp_port for future patches.
Signed-off-by: Kyle Mestery <kmestery@cisco.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
A bridge's local port always has type "internal", so opening it
with type "system" can't be correct. This was causing upgrade
problems. Specifically, in certain bridge topologies, if there was
a manager set force-reload-kmod would fail. This is because the
local port netdev would open in the in-band code with type
"system", confusing the more important netdev_open() in
iface_create().
Bug #15067.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
When handling flow misses, an attempt is made to group identical packets
together. Before the single datapath, each OpenFlow port number was
unique, so the flow_equal() function was sufficient to check whether
packets are identical. With the single datapath, the OpenFlow port
numbers are shared across bridges, so packets that arrive at the same
time and are identical other than their ingress port were being serviced
by the same ofproto instance. This commit changes the duplicate flow
finding function to take the ofproto into account.
Bug #14934
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Commit 0a740f48293 (ofproto-dpif: Implement patch ports in
userspace.) allowed special packets (i.e. LACP, CFM, etc) to be
sent on patch ports, but not received. This patch implements the
logic required to receive special packets on patch ports.
Bug #15154.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Until now the flow translation code has done one get_ofp_port() call
initially to check for special processing, then one for each level of
action processing. Only one call is actually necessary, though, because
the in_port of a flow doesn't change in ordinary circumstances, and so this
commit eliminates the unnecessary calls.
The one case where the in_port can change is when a packet passes through
a patch port. The implementation here was buggy anyway: when the patch
port's peer had forwarding disabled by STP, then the code would drop all
ODP actions, even those that were executed before the packet crossed the
patch port. This commit fixes that case.
With a complicated flow table involving multiple levels of resubmit, this
increases flow setup performance by 2-3%.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
The new ovs-monitor-ipsec implementation will use skb marks in
IPsec policies. This patch will configure datapath to use these
skb marks for IPsec tunnel packets.
Issue: 14870
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
The documented behavior of ovs is that a missing key is the
same as a zero key. However, the tunneling code actually treated
them differently. This could cause problems with tunneling modes
such as vxlan which always have a key. Specifically, a tunnel with
no key configured, would send have to send traffic with a key of
zero. However, the same tunnel would drop incoming traffic with a
zero key because it was expecting there to be none at all.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
These log messages occur infrequently, and are quite useful when
debugging problems after the fact. So they should be logged at
info level which makes them more readily available.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
The caller of port_query_by_name() is responsible for freeing the
ofproto_port that it returns on success, but ofproto-dpif did not do this.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
|
|
With this patch, ovs-vswitchd uses flow based tunneling
exclusively. I.E. each kind of tunnel shares a single tunnel
backer in the datapath. Tunnel headers are set by userspace using
the ipv4_tunnel datapath action. And, the configuration of
individual tunnels is now a userspace responsibility, so
netdev-vport no longer marshals and unmarshals Netlink attributes
for tunnel configuration, instead only storing the configuration
internally. There are still some significant pieces of work to do,
but the basic building blocks are there to begin testing.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Co-authored-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
|
|
The kernel tunneling code currently needs to handle a large number
of operations when tunnel packets are encapsulated and
decapsulated. Some examples of this are: finding the correct
tunnel port on receive, TTL and ToS inheritance, ECN handling, etc.
All of these can be done on a per-flow basis in userspace now that
we have both the inner and outer header information, which allows
us to both simplify the kernel and take advantage of userspace's
information. Once tunnel packets are redirected into this code,
the redundant pieces can be removed from other places.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Before this patch, if a packet came in on a port which userspace
doesn't know about, it would be silently dropped without installing
a drop flow. Historically, this has been fine because this
situation could only occur during transient reconfiguration
periods. However, in future, this could occur when the tunneling
code decides to reject a packet due to invalid headers. In this
case, it's preferable to drop the packet in the kernel to avoid a
high bandwidth stream of invalid packets DoSing the switch.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
All datapath flows should have an in_port, so it doesn't make a lot
of sense to allow omitting it when tracing. If a user wants to
trace a flow which has no in_port, they can use the OpenFlow syntax
which doesn't go through ofproto_receive().
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
This removes a bit of duplicate code, and will be necessary to
support future patches.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Commit e503cc199 (ofproto: Optimise OpenFlow flow expiry) optimized
OpenFlow flow expiration by putting expirable flows on a list, but it
failed to remove flows from the list when they were replaced by a new
flow with an OpenFlow flow_mod "add" operation. This commit fixes the
problem.
Found by valgrind.
CC: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@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>
|
