On 5/23/14, 12:19 AM, Ben Pfaff wrote:
On Tue, May 13, 2014 at 05:02:15PM +0300, Lorand Jakab wrote:
This commit relaxes the assumption that all packets have an Ethernet
header, and adds support for layer 3 flows. For each packet received on
the Linux kernel datapath the l2 and l3 members of struct ofpbuf are
intialized appropriately, and some functions now expect this (notably
flow_extract()), in order to differentiate between layer 2 and layer 3
packets. struct flow has now a new 'base_layer' member, because we
cannot assume that a flow has no Ethernet header when eth_src and
eth_dst are 0. For layer 3 packets, the protocol type is still stored
in the eth_type member.
Switching L2->L3 and L3->L2 are both implemented by adding the pop_eth
and push_eth actions respectively when a transition is detected. The
push_eth action puts 0s on both source and destination MACs. These
addresses can be modified with mod_dl_dst and mod_dl_src actions.
Added new prerequisite MFP_ETHERNET for fields MFF_ETH_SRC, MFF_ETH_DST,
MFF_VLAN_TCI, MFF_DL_VLAN, MFF_VLAN_VID and MFF_DL_VLAN_PCP.
Signed-off-by: Lorand Jakab <loja...@cisco.com>
GCC 4.7 is complaining about a possibly uninitialized variable:
cc1: warnings being treated as errors
../lib/flow.c: In function 'miniflow_extract':
../lib/flow.c:388: error: 'frame' may be used uninitialized in
this function
I couldn't reproduce this with gcc-4.7.3 (Gentoo 4.7.3-r1 p1.4,
pie-0.5.5), but changed the declaration to initialize *frame to NULL:
@@ -385,7 +385,7 @@ miniflow_extract(struct ofpbuf *packet, const struct
pkt_metadata *md,
size_t size = ofpbuf_size(packet);
uint32_t *values = miniflow_values(dst);
struct mf_ctx mf = { 0, values, values + FLOW_U32S };
- char *frame;
+ char *frame = NULL;
ovs_be16 dl_type;
uint8_t nw_frag, nw_tos, nw_ttl, nw_proto;
I see at least one direct reference to the 'data_' member of struct
ofpbuf in this patch. This will break builds that include DPDK
support. Please use the helper functions.
Updated patch with the following incremental:
@@ -340,8 +340,8 @@ get_l3_eth_type(struct ofpbuf *packet)
}
/* Initializes 'flow' members from 'packet' and 'md'. Expects
packet->frame
- * pointer to be equal to packet->data_, and packet->l3_ofs to be set
to 0 for
- * layer 3 packets.
+ * pointer to be equal to ofpbuf_data(packet), and packet->l3_ofs to be
set to
+ * 0 for layer 3 packets.
*
* Initializes the layer offsets as follows:
*
@@ -368,7 +368,7 @@ flow_extract(struct ofpbuf *packet, const struct
pkt_metadata *md,
COVERAGE_INC(flow_extract);
- ovs_assert(packet->frame == packet->data_);
+ ovs_assert(packet->frame == ofpbuf_data(packet));
miniflow_initialize(&m.mf, m.buf);
In miniflow_extract(), I am not sure that it is necessary to do this
in L3 case, because all of the contents of a miniflow is assumed to be
0 unless otherwise specified:
+ miniflow_push_be16(mf, vlan_tci, 0);
That's what I expected too. After I rebased my patches on top of the
miniflow changes I did not have that line, but flows were not identified
correctly. After adding that line, everything started to work again.
I suspect that xlate_actions__() needs to mark wc->base_layer as
"unwildcarded", just as it does with in_port, skb_priority, dl_type,
and so on.
Right, done.
Once Jesse finishes his review, I will post v4 with the above changes.
-Lori
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
