ofpbuf was complicated due to its wide usage across all
layers of OVS, Now we have introduced independent dp_packet
which can be used for datapath packet, we can simplify ofpbuf.
Following patch removes DPDK mbuf and access API of ofpbuf
members.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
A "conjunctive match" allows higher-level matches in the flow table, such
as set membership matches, without causing a cross-product explosion for
multidimensional matches. Please refer to the documentation that this
commit adds to ovs-ofctl(8) for a better explanation, including an example.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@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>
The following macros are renamed to avoid conflicts with other headers:
* WARN_UNUSED_RESULT to OVS_WARN_UNUSED_RESULT
* PRINTF_FORMAT to OVS_PRINTF_FORMAT
* NO_RETURN to OVS_NO_RETURN
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Preparation for supporting ONFACT_ET_COPY_FIELD.
ONF-JIRA: EXT-320
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Thomas Graf <tgraf@noironetworks.com>
Since my original prototype, the oxm_id_len field was removed and
replaced by 2 bytes of padding.
ONF-JIRA: EXT-320
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Thomas Graf <tgraf@noironetworks.com>
Commit 7eb4b1f1d70345f ("ofp-actions: Support OF1.5 (draft) masked
Set-Field, merge with reg_load.") introduced a bug in that a set_field
action that set an entire field would be translated incorrectly to
reg_load, if the field being set only occupied a portion of the bytes that
it contains. For example, an MPLS label is 20 bits but has a 4-byte field,
which meant that a set_field would get translated into a reg_load that
wrote all 32 bits; in turn, the receiver of that reg_load would reject it
because it was attempting to set invalid bits (the top 12 bits).
This commit fixes the problem by omitting invalid bits when encoding a
reg_load action.
Reported-by: Pravin Shelar <pshelar@nicira.com>
Tested-by: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Packets with 'LATER' fragment do not have a transport header, so it is
not possible to either match on or set transport ports on such
packets. Matching is prevented by augmenting mf_are_prereqs_ok() with
a nw_frag 'LATER' bit check. Setting the transport headers on such
packets is prevented in three ways:
1. Flows with an explicit match on nw_frag, where the LATER bit is 1:
existing calls to the modified mf_are_prereqs_ok() prohibit using
transport header fields (port numbers) in OXM/NXM actions
(set_field, move). SET_TP_* actions need a new check on the LATER
bit.
2. Flows that wildcard the nw_frag LATER bit: At flow translation
time, add calls to mf_are_prereqs_ok() to make sure that we do not
use transport ports in flows that do not have them.
3. At action execution time, do not set transport ports, if the packet
does not have a full transport header. This ensures that we never
call the packet_set functions, that require a valid transport
header, with packets that do not have them. For example, if the
flow was created with a IPv6 first fragment that had the full TCP
header, but the next packet's first fragment is missing them.
3 alone would suffice for correct behavior, but 1 and 2 seem like a
right thing to do, anyway.
Currently, if we are setting port numbers, we will also match them,
due to us tracking the set fields with the same flow_wildcards as the
matched fields. Hence, if the incoming port number was not zero, the
flow would not match any packets with missing or truncated transport
headers. However, relying on no packets having zero port numbers
would not be very robust. Also, we may separate the tracking of set
and matched fields in the future, which would allow some flows that
blindly set port numbers to not match on them at all.
For TCP in case 3 we use ofpbuf_get_tcp_payload() that requires the
whole (potentially variable size) TCP header to be present. However,
when parsing a flow, we only require the fixed size portion of the TCP
header to be present, which would be enough to set the port numbers
and fix the TCP checksum.
Finally, we add tests testing the new behavior.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Commit c2d936a44fa (ofp-actions: Centralize all OpenFlow action code for
maintainability.) rewrote OpenFlow action parsing but failed to check that
actions don't overflow their buffers. This commit fixes the problem and
adds negative tests so that this bug doesn't recur.
Reported-by: Tomer Pearl <Tomer.Pearl@Contextream.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>
Some of the Nicira extension actions include fixed-size 32-bit members that
designate NXM fields. These actions can't accommodate 64-bit experimenter
OXMs, so we need to figure out some kind of solution. This commit does
that, in different ways for different actions.
For some actions, I did not think it was worthwhile to worry about
experimenter OXM, so I just disabled use of them. This is what I did for
bundle, learn, and multipath actions.
Other actions could be gracefully reinterpreted to support experimenter
OXM. This is true of reg_move, which use NXM headers only at the end of
the action and such that using an experimenter OXM would make the action
longer (which unambigously signals to older OVS that the action is an
error, which is desired behavior since older OVS cannot interpret this
action). The stack push and pop actions are also in this category.
reg_load was the most frustrating case. In OpenFlow 1.5 we had already
eliminated this action in favor of OF1.5+ set_field. In other OpenFlow
versions, though, reg_load is more powerful than set_field because it
can modify partial fields. This commit therefore adds a new variant of
reg_load, called reg_load2, which is simply OF1.5+ set_field with a Nicira
extension header on it.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
OpenFlow 1.2+ defines a means for vendors to define vendor-specific OXM
fields, called "experimenter OXM". These OXM fields are expressed with a
64-bit OXM header instead of the 32-bit header used for standard OXM (and
NXM). Until now, OVS has not implemented experimenter OXM, and indeed we
have had little need to do so because of a pair of special 32-bit OXM classes
grandfathered to OVS as part of the OpenFlow 1.2 standardization process.
However, I want to prototype a feature for OpenFlow 1.5 that uses an
experimenter OXM as part of the prototype, so to do this OVS needs to
support experimenter OXM. This commit adds that support.
Most of this commit is a fairly straightforward change: it extends the type
used for OXM/NXM from 32 to 64 bits and adds code to encode and decode the
longer headers when necessary. Some other changes are necessary because
experimenter OXMs have a funny idea of the division between "header" and
"body": the extra 32 bits for experimenter OXMs are counted as part of the body
rather than the header according to the OpenFlow standard (even though this
does not entirely make sense), so arithmetic in various places has to be
adjusted, which is the reason for the new functions nxm_experimenter_len(),
nxm_payload_len(), and nxm_header_len().
Another change that calls for explanation is the new function mf_nxm_header()
that has been split from mf_oxm_header(). This function is used in actions
where the space for an NXM or OXM header is fixed so that there is no room
for a 64-bit experimenter type. An upcoming commit will add new variations
of these actions that can support experimenter OXM.
Testing experimenter OXM is tricky because I do not know of any in
widespread use. Two ONF proposals use experimenter OXMs: EXT-256 and
EXT-233. EXT-256 is not suitable to implement for testing because its use
of experimenter OXM is wrong and will be changed. EXT-233 is not suitable
to implement for testing because it requires adding a new field to struct
flow and I am not yet convinced that that field and the feature that it
supports is worth having in Open vSwitch. Thus, this commit assigns an
experimenter OXM code point to an existing OVS field that is currently
restricted from use by controllers, "dp_hash", and uses that for testing.
Because controllers cannot use it, this leaves future versions of OVS free
to drop the support for the experimenter OXM for this field without causing
backward compatibility problems.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
OpenFlow 1.5 (draft) extends the OFPAT_SET_FIELD action originally
introduced in OpenFlow 1.2 so that it can set not just entire fields but
any subset of bits within a field as well. This commit adds support for
that feature when OpenFlow 1.5 is used.
With this feature, OFPAT_SET_FIELD becomes a superset of NXAST_REG_LOAD.
Thus, this commit merges the implementations of the two actions into a
single ofpact_set_field.
ONF-JIRA: EXT-314
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
The OpenFlow 1.5 (draft) Copy-Field action has two OXM headers, one after
the other. Until now, Open vSwitch has implemented these as a pair of
ovs_be32 members, which meant that only 32-bit OXM could be supported. This
commit changes the implementation to use nx_pull_header(), which means that
in the future when that function supports 64-bit experimenter OXMs,
Copy-Field will automatically get that support too.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
This improves the general abstraction of OXM/NXM by eliminating direct
knowledge of it from the meta-flow code and other places.
Some function renaming might be called for; for example, mf_oxm_header()
may not be the best name now that the function is implemented within
nx-match. However, these renamings would make this commit larger and
harder to review, so I'm postponing them.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
This is a first step toward improving the abstraction of OXM and NXM in the
tree. As an immediate improvement, this commit removes all of the
definitions of the OXM and NXM constants from the top-level header files,
because they are no longer used anywhere.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Following this change, only meta-flow.c uses any explicit NXM_* or OXM_*
constants. An upcoming commit will actually remove the definitions of
these constants, hiding them behind a functional interface, for better
abstraction.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Send OFPET_BAD_INSTRUCTION/OFPBIC_BAD_TABLE_ID if table is invalid
in goto table instruction.
Signed-off-by: Selvamuthukumar <smkumar@merunetworks.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
MSVC complains about a void function returning a value if there is a
statement of the form - 'return foo()' even if foo() has a void return
type.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Fixing issue where "resubmit" action in a group action set was not
considered sufficient to retain the full action set. This patch allows
a group action set (considered terminal with OF1.4 and earlier spec)
to have the "output" action come from a different table.
Signed-off-by: Srini Seetharaman <srini.seetharaman@gmail.com>
[blp@nicira.com added documentation]
Signed-off-by: Ben Pfaff <blp@nicira.com>
Treating OFPACT_REG_MOVE as a "set" action preserves the order of loads
and moves and allows a load to overwrite a previous move to the same
register.
This makes the following work:
add-group br0 group_id=1234,type=all, \
bucket=output:10,move:NXM_NX_REG1[]->NXM_OF_IP_SRC[], \
bucket=output:11
add-flow br0 ip actions=load:0xffffffff->NXM_NX_REG1[],group:1234
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Until now, knowledge about OpenFlow has been somewhat scattered around the
tree. Some of it is in ofp-actions, some of it is in ofp-util, some in
separate files for individual actions, and most of the wire format
declarations are in include/openflow. This commit centralizes all of that
in ofp-actions.
Encoding and decoding OpenFlow actions was previously broken up by OpenFlow
version. This was OK with only OpenFlow 1.0 and 1.1, but each additional
version added a new wrapper around the existing ones, which started to
become hard to understand. This commit merges all of the processing for
the different versions, to the extent that they are similar, making the
version differences clearer.
Previously, ofp-actions contained OpenFlow encoding and decoding, plus
ofpact formatting, but OpenFlow parsing was separated into ofp-parse, which
seems an odd division. This commit moves the parsing code into ofp-actions
with the rest of the code.
Before this commit, the four main bits of code associated with a particular
ofpact--OpenFlow encoding and decoding, ofpact formatting and parsing--were
all found far away from each other. This often made it hard to see what
was going on for a particular ofpact, since you had to search around to
many different pieces of code. This commit reorganizes so that all of the
code for a given ofpact is in a single place.
As a code refactoring, this commit has little visible behavioral change.
The update to ofproto-dpif.at illustrates one minor bug fix as a side
effect: a flow that was added with the action "dec_ttl" (a standard
OpenFlow action) was previously formatted as "dec_ttl(0)" (using a Nicira
extension to specifically direct packets bounced to the controller because
of too-low TTL), but after this commit it is correctly formatted as
"dec_ttl".
The other visible effect is to drop support for the Nicira extension
dec_ttl action in OpenFlow 1.1 and later in favor of the equivalent
standard action. It seems unlikely that anyone was really using the
Nicira extension in OF1.1 or later.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
This allows callers to be more uniform, because they don't have to pick
out whether they should parse actions or instructions based on the OpenFlow
version in use. It also allows the Write-Metadata instruction emulation
in OpenFlow 1.0 to be treated just as in OpenFlow 1.1 in the sense that
it is allowed in contexts where instructions are allowed in OpenFlow 1.1
and not elsewhere. (The changes in the tests reflect this more accurate
treatment.)
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
This will allow, later, to centralize all of the knowledge of instruction
encoding inside ofp-actions.
OFPIT11_ALL and OFPIT13_ALL are no longer used, so this commit removes
them. Their definitions were wrong (they did not shift each bit into
position correctly), so this commit is also a small bug fix.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Until now, sets of actions have been abstracted separately outside
ofp-actions, as enum ofputil_action_bitmap. Drawing sets of actions into
ofp-actions, as done in this commit, makes for a better overall
abstraction of actions, with better consistency.
A big part of this commit is shifting from using ofp12_table_stats as if
it were an abstraction for OpenFlow table stats, toward using a new
struct ofputil_table_stats, which is what we generally do with other
OpenFlow structures and fits better with the rest of the code.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Add OFPERR_OFPBIC_DUP_INST (type = OFPET_BAD_INSTRUCTION, code = 9)
and use it for OpenFlow1.4+.
For OpenFlow1.1 - 1.3 map this error to ONFBIC_DUP_INSTRUCTION
(experimenter = ONF, type = 2600) which is proposed in
OpenFlow enhancement proposal EXT-260 "Add error
code for duplicate instruction.".
Previously ONFBIC_DUP_INSTRUCTION was used for OpenFlow1.3+.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Rename 'l2' to 'frame' and add new ofpbuf_set_frame() and ofpbuf_l2().
ofpbuf_set_frame() alse resets all the layer offsets. ofpbuf_l2()
returns NULL if the packet has no Ethernet header, as indicated either
by unset l3 offset or NULL frame pointer. Callers of ofpbuf_l2() are
supposed to check the return value, unless they can otherwise be sure
that the packet has a valid Ethernet header.
The recent commit 437d0d22 made some assumptions that were not valid
regarding the use of the 'l2' pointer in rconn module and by
compose_rarp(). This is now fixed as follows: rconn now relies on the
fact that once OpenFlow messages are given to rconn for transport, the
frame pointer is no longer needed to refer to the OpenFlow header; and
compose_rarp() now sets the frame pointer and offsets as expected.
In addition to storing network frames, ofpbufs are also used for
handling OpenFlow messages and action lists. lib/ofpbuf.h now has a
comment documenting the current usage conventions and invariants.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This patch shrinks the struct ofpbuf from 104 to 48 bytes on 64-bit
systems, or from 52 to 36 bytes on 32-bit systems (counting in the
'l7' removal from an earlier patch). This may help contribute to
cache efficiency, and will speed up initializing, copying and
manipulating ofpbufs. This is potentially important for the DPDK
datapath, but the rest of the code base may also see a little benefit.
Changes are:
- Remove 'l7' pointer (previous patch).
- Use offsets instead of layer pointers for l2_5, l3, and l4 using
'l2' as basis. Usually 'data' is the same as 'l2', but this is not
always the case (e.g., when parsing or constructing a packet), so it
can not be easily used as the offset basis. Also, packet parsing is
faster if we do not need to maintain the offsets each time we pull
data from the ofpbuf.
- Use uint32_t for 'allocated' and 'size', as 2^32 is enough even for
largest possible messages/packets.
- Use packed enum for 'source'.
- Rearrange to avoid unnecessary padding.
- Remove 'private_p', which was used only in two cases, both of which
had the invariant ('l2' == 'data'), so we can temporarily use 'l2'
as a private pointer.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
PAD_SIZE(x,y) is a little shorter and may have a more obvious meaning
than ROUND_UP(x,y) - x.
I intend to add more users in an upcoming comment.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
Correct pop MPLS ethtype consistency check to verify that
the packet has an MPLS ethtype before the pop action rather than after:
an MPLS ethtype is a pre-condition but not a post-condition of pop MPLS.
With this change the consistency check in ofpact_check__()
becomes consistent with that in ofpact_from_nxast().
This was found using Ryu tests via the new make check-ryu target.
It allows all of the "POP_MPLS"[1] tests to pass where they previously
failed.
[1] http://osrg.github.io/ryu-certification/switch/ovs
Cc: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
OpenFlow 1.1 and 1.2 always inserted MPLS labels after VLAN tags.
OpenFlow 1.3 and 1.4 insert MPLS labels before VLAN tags.
OpenFlow 1.3.4 and 1.5, both in preparation, recognize that the change in
1.3 was an error and revert it. This commit implements that reversion
in Open vSwitch.
EXT-457.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Simon Horman <horms@verge.net.au>
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>
Until now ofpacts_check() has been told either to enforce consistency or
not, but that means that the caller has to know exactly what protocol is
going to be in use (because some protocols require consistency to be
enforced and others don't). This commit changes ofpacts_check() to just
rule out protocols that require enforcement when it detects
inconsistencies.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
After an mpls_push action the resulting packet is MPLS and
the MPLS payload is opaque. Thus nothing can be assumed
about the packets network protocol and it is inconsistent
to apply L4 actions.
With regards to actions that affect the packet at other layers
of the protocol stack:
* L3: The consistency of L3 actions should already be handled correctly
by virtue of the dl_type of the flow being temporarily altered
during consistency checking by both push_mpls and pop_mpls actions.
* MPLS: The consistency checking of MPLS actions appear to already be
handled correctly.
* VLAN: At this time Open vSwitch on mpls_push an MPLS LSE is always
added after any VLAN tags that follow the ethernet header.
That is the tag ordering defined prior to OpenFlow1.3. As such
VLAN actions should sill be equally valid before and after mpls_push
and mpls_pop actions.
* L2 actions are equally valid before and after mpls_push and mpls_pop actions.
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The formatting of the "enqueue" action uses a "q" to separate the port
number from the queue number, as in "enqueue:123q456". This is different
from every other action. This commit improves the situation by:
* Switching the formatting to use a colon (e.g. "enqueue:123:456"),
which is a little less odd-looking but still accepted by older
versions of Open vSwitch.
* Improving the parser to accept "enqueue(123,456)" also.
Signed-off-by: Ben Pfaff <blp@nicira.com>
