On Tue, Dec 24, 2013 at 04:02:35PM +0200, 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 and
> MFF_ETH_DST to avoid detecting layer 2 flows with all-zero MAC addresses
> as layer 3.
>
> Signed-off-by: Lorand Jakab <loja...@cisco.com>
Please update the comment on ofp_packet_to_string() to explain the
is_layer3 parameter.
In parse_odp_packet() it should be unnecessary to explicitly set l2 or
l3 to NULL. ofpbuf_use_stub() does that for us.
This deletes the description of setting l3 from the comment on
flow_extract(), but flow_extract() still sets l3 (for L2 packets).
In flow_extract() there's no minimum size check in the L3 branch. At
a minimum we need one byte to distinguish v4/v6, or we could check for
at least IP_HEADER_LEN.
In flow_extract() it seems odd to set l2 to NULL just after we
verified that it is NULL.
In struct flow, I'm not sure that it's safe to both use an enum and
assert on the size of the structure, because the compiler is allowed
to choose any appropriate size for an enum. I believe that the System
V ABI says that an enum should have type 'int', but I doubt that this
is universal across all ABIs, so I'd be inclined to use some explicit
type for the member, even though we use the LAYER_* values. (Another
idea would be to just use '2' or '3' as the values and skip the
enemas.)
In meta-flow.c, do the VLAN fields need an MFP_ETHERNET prerequisite?
Lots of unneeded () in compose_output_action__():
+ if ((in_xport) && !(in_xport->is_layer3) && xport->is_layer3) {
+ odp_put_pop_eth_action(&ctx->xout->odp_actions);
+ }
+
+ if (flow->base_layer == LAYER_3 && !(xport->is_layer3)) {
I find myself wondering whether we should require OpenFlow to
explicitly push and pop Ethernet headers for output, instead of
automatically pushing and popping. There could be some pretty weird
results if nothing sets an Ethernet source or dest address and outputs
all-zeros addresses to an L2 port as a result. And it would also be
pretty weird stripping the Ethernet header off, say, an ARP packet and
outputting it to an L3 port.
OpenFlow
--------
There is an OpenFlow extensibility working group proposal that matches
the requirements here:
https://rs.opennetworking.org/bugs/browse/EXT-112
I suggest implementing OpenFlow support as described there.
I think that we will need to include an indication of the packet type
(L2 or IPv4 or IPv6, I guess) in flow_metadata, at least eventually,
for proper OpenFlow support.
ofp_print_packet_in() and ofp_print_packet_out() will eventually need
to figure out whether a packet is L2 or L3 by looking at the flow,
although this requires additional OpenFlow extensions.
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
