| Age | Commit message (Collapse) | Author |
|---|
|
Before we submitted the kernel module upstream, we updated the flow format
by adding two fields to the description of packets with VLAN headers, but
we forgot to update ODPUTIL_FLOW_KEY_BYTES to reflect these changes. The
result was that a maximum-length flow did not fit in the given space.
This fixes a crash processing IPv6 neighbor discovery packets with VLAN
headers received in a tunnel configured with key=flow or in_key=flow.
This updates some comments to better describe the implications of
ODPUTIL_FLOW_KEY_BYTES (suggested by Justin).
This also updates test-odp.c so that it would have caught this problem, and
updates odp.at to demonstrate that a full 156 bytes are necessary. (To see
that, revert the change to ODPUTIL_FLOW_KEY_BYTES and run the test.)
Reported-by: Dan Wendlandt <dan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Some OpenFlow 1.0 controllers incorrectly use OPFP_CONTROLLER as the
in_port in packet-out messages, when OFPP_NONE is their intent. Until now,
Open vSwitch has rejected such requests with an error message. This commit
makes Open vSwitch instead treat OFPP_CONTROLLER the same as OFPP_NONE for
compatibility with those controllers.
(Also, as of this writing, OpenFlow 1.0.1 appears to be changing the port
to use from OFPP_NONE to OFPP_CONTROLLER.)
Suggested-by: Rob Sherwood <rob.sherwood@bigswitch.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
I have an outstanding bug report that has tons of "***56 leftover bytes***"
messages with no way to guess what the problem is. I hope this makes
debugging possible.
Bug #9346.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
One of the major tasks of ofproto-dpif is to translate OpenFlow
actions into "ODP" datapath actions. These translations are essentially
a cache that requires revalidation when certain state changes occur. For
best performance it's important to revalidate flows only when necessary,
so from time to time Open vSwitch has gotten this wrong, which meant that
stale flows could persist in the kernel and cause surprising behavior.
This commit implements a simple "self check": every trip through the
Open vSwitch main loop randomly chooses one flow entry and checks that
its actions have been correctly translated. If not, Open vSwitch logs
the details of the problem. This should help find problems more
quickly in the future.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Rather than silently skipping ipv6 action generation, following patch
generates OVS_ACTION_ATTR_SET action for ipv6. Datapath which do not
support ipv6 action can reject this action.
Bug #8758
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
|
|
In future patches, PACKET_IN messages will include meta-data which
is only available in userspace during action translation. Either,
this data needs to be stored until it's required by a userspace
datapath action, or the PACKET_IN messages must be sent at the time
the data is available. This patch implements the latter.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
This priority's mean is completely different from the priority of an
OpenFlow rule, so it is confusing for it to have the same name.
We should be on the lookout for a less Linux-specific name, but this one
seems fine for now.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
In an effort to simplify ofproto-dpif, this commit moves the
definition of commit_odp_actions() to odp-util.
|
|
Feature #4886.
|
|
The compare-odp-actions.pl utility isn't fully general, even for its
intended purpose of allowing sets of ODP actions to be compared
ignoring unimportant differences in ordering of output actions and
VLAN set actions. I decided that the proper way to do it was to have
a utility that can actually parse the actions, instead of just
doing textual transformations on them. So, this commit replaces
compare-odp-actions.pl by "ovs-dpctl normalize-actions", which is
sufficiently general for the intended purpose.
The new ovs-dpctl functionality can be easily extended to handle
differences in fields other than VLAN, but only VLAN is needed so
far.
This will be needed in an upcoming commit that in some cases
introduces redundant "set vlan" actions into the ODP actions, which
compare-odp-actions.pl doesn't tolerate.
|
|
An upcoming commit will add a user. The only planned users for now are
part of unit tests themselves, so it doesn't seem important to unit test
it.
|
|
Really the "trace" command should support this but in fact I need it for
an upcoming update to a test.
|
|
An upcoming patch to odp-util will add a new user, but this seems like a
reasonable change in any case.
|
|
|
|
This matches the order of their definition in openvswitch.h.
Reported-by: Justin Pettit <jpettit@nicira.com>
|
|
In the future it is likely that our vlan support will expand to
include multiply tagged packets. When this happens, we would
ideally like for it to be consistent with our current tagging.
Currently, if we receive a packet with a partial VLAN tag we will
automatically drop it in the kernel, which is unique among the
protocols we support. The only other reason to drop a packet is
a memory allocation error. For a doubly tagged packet, we will
parse the first tag and indicate that another tag was present but
do not drop if the second tag is incorrect as we do not parse it.
This changes the behavior of the vlan parser to match other protocols
and also deeper tags by indicating the presence of a broken tag with
the 802.1Q EtherType but no vlan information. This shifts the policy
decision to userspace on whether to drop broken tags and allows us to
uniformly add new levels of tag parsing.
Although additional levels of control are provided to userspace, this
maintains the current behavior of dropping packets with a broken
tag when using the NORMAL action because that is the correct behavior
for an 802.1Q-aware switch. The userspace flow parser actually
already had the new behavior so this corrects an inconsistency.
Reported-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
When the datapath was converted to use Netlink attributes for describing
flow keys, I had a vague idea of how it could be smoothly extensible, but
I didn't actually implement extensibility or carefully think it through.
This commit adds a document that describes how flow keys can be extended
in a compatible fashion and adapts the existing interface to match what
it says.
This commit doesn't actually implement extensibility. I already have a
separate patch series out for that. This patch series borrows from that
one heavily, but the extensibility series will need to be reworked
somewhat once this one is in.
This commit is only lightly tested because I don't have a good test setup
for VLANs.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
This is more conventional use of Netlink.
For upstreaming, 'u64 attrs' can be changed to u32 and the uses of 1ULL
can be changed to 1.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
|
|
Usually we don't put a new-line in prototypes unless we have to. I also
think that this comment deserves to be higher up in the file, so I moved
it.
|
|
I usually do things this way but must have overlooked this one a while ago.
|
|
This seems like a worthwhile improvement in itself, but it will also see
additional users in upcoming commits.
|
|
Most of the members in structures referring to network elements indicate
the layer (e.g., "tl_", "nw_", "tp_"). The "frag" and "tos" members
didn't, so this commit add them.
|
|
IPv6 uses the term "traffic class" for what IPv4 calls
"type-of-service". This commit renames the the "ipv6_tos" field to
"ipv6_tclass" in the "ovs-key_ipv6" struct to be more consistent with
the IPv6 terminology.
Suggested-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
Suggested-by: Ben Pfaff <blp@nicira.com>
|
|
Add support matching the IPv4 TTL and IPv6 hop limit fields. This
commit also adds support for modifying the IPv4 TTL. Modifying the IPv6
hop limit isn't currently supported, since we don't support modifying
IPv6 headers.
We will likely want to change the user-space interface, since basic
matching and setting the TTL are not generally useful. We will probably
want the ability to match on extraordinary events (such as TTL of 0 or 1)
and a decrement action.
Feature #8024
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
This will be useful later when we add support for matching the ECN bits
within the TOS field.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
Following patch adds skb-priority to flow key. So userspace will know
what was priority when packet arrived and we can remove the pop/reset
priority action. It's no longer necessary to have a special action for
pop that is based on the kernel remembering original skb->priority.
Userspace can just emit a set priority action with the original value.
Since the priority field is a match field with just a normal set action,
we can convert it into the new model for actions that are based on
matches.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #7715
|
|
Patch below fixes build on FreeBSD; tested on 10.0-CURRENT.
Signed-off-by: Edward Tomasz Napierala <trasz@FreeBSD.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Until now, OVS has handled IP fragments more awkwardly than necessary. It
has not been possible to match on L4 headers, even in fragments with offset
0 where they are actually present. This means that there was no way to
implement ACLs that treat, say, different TCP ports differently, on
fragmented traffic; instead, all decisions for fragment forwarding had to
be made on the basis of L2 and L3 headers alone.
This commit improves the situation significantly. It is still not possible
to match on L4 headers in fragments with nonzero offset, because that
information is simply not present in such fragments, but this commit adds
the ability to match on L4 headers for fragments with zero offset. This
means that it becomes possible to implement ACLs that drop such "first
fragments" on the basis of L4 headers. In practice, that effectively
blocks even fragmented traffic on an L4 basis, because the receiving IP
stack cannot reassemble a full packet when the first fragment is missing.
This commit works by adding a new "fragment type" to the kernel flow match
and making it available through OpenFlow as a new NXM field named
NXM_NX_IP_FRAG. Because OpenFlow 1.0 explicitly says that the L4 fields
are always 0 for IP fragments, it adds a new OpenFlow fragment handling
mode that fills in the L4 fields for "first fragments". It also enhances
ovs-ofctl to allow users to configure this new fragment handling mode and
to parse the new field.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Bug #7557.
|
|
Almost all current actions can be expressed in the form of
push/pop/set <field>, where field is one of the match fields. We can
create three base actions and take a field. This has both a nice
symmetry and avoids inconsistencies where we can match on the vlan
TPID but not set it.
Following patch converts all actions to this new format.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #7115
|
|
Most of the enum tags in this file are lowercased versions of the uppercase
enum prefixes (or slightly less abbreviated versions, e.g. "dp" becomes
"datapath"). This commit fixes up the others for consistency.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
Commit b063d9f06 "datapath: Use unicast Netlink sockets for upcalls" that
switched from multicast to unicast Netlink for sending upcalls added a
Netlink PID to each kernel flow, used by OVS_ACTION_ATTR_USERSPACE actions
within the flow as target.
This commit drops this per-flow PID in favor of a per-action PID, because
that is more flexible. It does not yet make use of this additional
flexibility, so behavior should not change.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #7559.
|
|
These macros caused trouble if datapath-protocol.h was included before
openflow.h. Later references to the icmp_type and icmp_code members of
struct ovs_key_icmp caused compiler errors, because the macros caused them
to try to refer to nonexistent tp_src and tp_dst members in those
structures.
|
|
|
|
Following patch adds sampling action which takes probability and set
of actions as arguments. When probability is hit, actions are executed for
given packet.
USERSPACE action's userdata (u64) is used to store struct
user_action_cookie as cookie. CONTROLLER action is fixed accordingly.
Now we can remove sFlow code from kernel and implement sFlow generically
as SAMPLE action. sFlow is defined as SAMPLE Action with probability (sFlow
sampling rate) and USERSPACE action as argument. USERSPACE action's data
is used as cookie. sFlow uses this cookie to store output-port, number of
output ports and vlan-id. sample-pool is calculated by using vport
stats.
Signed-off-by: Pravin Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
|
|
|
|
Currently the kernel vlan actions mirror those used by OpenFlow 1.0.
i.e. MODIFY and STRIP. More flexible approach is to have an action to
push a tag and pop a tag off, so that it can handle multiple levels of vlan
tags. Plus it aligns with newer version of OpenFlow.
As this patch replaces MODIFY with PUSH semantic, action
mapping done in userpace is fixed accordingly.
GSO handling for multiple levels of vlan tags is also added as
Jesse suggested before.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
When ovs-vswitchd executes actions on a synthesized packet, that is, on a
packet that is not being forwarded from any particular port but is being
generated by ovs-vswitchd itself or by an OpenFlow controller (using a
OFPT_PACKET_OUT message with an in_port of OFPP_NONE), there is no good
choice for the in_port to pass to the kernel in the flow in the
OVS_PACKET_CMD_EXECUTE message. This commit allows ovs-vswitchd to omit
the in_port entirely in this case.
This fixes a bug in OFPT_PACKET_OUT: using an in_port of OFPP_NONE would
cause the packet to be dropped by the kernel, since that's an invalid
input port.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Reported-by: Aaron Rosen <arosen@clemson.edu>
|
|
The prefix "ODP_*" is not overly descriptive in the context of the
larger Linux tree. This commit changes the prefix to "OVS_*" for the
userpace to kernel interactions. The userspace libraries still use
"ODP_" in many of their interfaces since it is more descriptive in the
OVS oeuvre.
Feature #6904
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
An existing comment in the function being updated explains the problem:
* Many of the sscanf calls in this function use oversized destination
* fields because some sscanf() implementations truncate the range of %i
* directives, so that e.g. "%"SCNi16 interprets input of "0xfedc" as a
* value of 0x7fff. The other alternatives are to allow only a single
* radix (e.g. decimal or hexadecimal) or to write more sophisticated
* parsers.
|
|
|
|
This will be used in upcoming commits.
|
|
The rest of the headers all follow the form "header(value)" or
"header(key1=value1,key2=value2,...)" but VLAN headers left out the "="
characters. This adds them in for consistency.
|
|
ODP_ACTION_ATTR_CONTROLLER in the kernel actually sends packets to
userspace, not the controller. To make it generic rename this action
to ODP_ACTION_ATTR_USERSPACE.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
|
|
The NXAST_DROP_SPOOFED_ARP action has been deprecated in favor of
defining flows using the NXM_NX_ARP_SHA flow match for a while. This
commit removes it.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
|
|
In addition to the changes to ofproto, this commit changes all of the
instances of "struct flow" in the tree so that the "in_port" member is an
OpenFlow port number. Previously, this member was an OpenFlow port number
in some cases and an ODP port number in other cases.
|
