On Wed, Jan 08, 2014 at 07:06:32PM +0200, Lori Jakab wrote: > On 12/30/13 9:28 PM, Ben Pfaff wrote: > >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 <[email protected]>
... > >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.) > > I changed it to uint32_t. I would like to keep the enum, because a > match initialized to all zeroes will be automatically LAYER_2 and > there is no need to update the code for explicitly setting it. OK, that's a good point, thanks. > >In meta-flow.c, do the VLAN fields need an MFP_ETHERNET prerequisite? > > I think so, and MPLS fields as well in the future. For what it's worth, I think I was pointing out here that the VLAN fields lacked that prerequisite, not really asking a question. I agree with your answer, of course. > >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. > > It's definitely less error prone if we do that, I agree. However, > the NORMAL action would not send flooded packets to L3 ports, and > there is no MAC learning for packets received on L3 ports. There > needs to be a rule to send traffic explicitly to a L3 port. If that > rule includes packets that shouldn't go there, it's the user's job > to fix it. Same for receiving. Received packets should have > explicit rules to set the destination MAC and output port. If no > such rule exists, they should be dropped. To the best of my > knowledge, that's what happens currently. > > I don't have a strong opinion either way, or of both operating modes > should be supported, so I'll do as you prefer. I think I slightly > favor your approach, since then we don't have to eventually > implement ARP lookup internally, and it's the user/controller's job > to set MAC addresses. OK. Thanks for humoring me, and for the analysis. I had not thought about NORMAL or flooding or MAC learning before. > >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. > > Is this required before the series goes in? If there is going to be OpenFlow support, I want it considered, otherwise it is not necessary yet. Thanks, Ben. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
