On Thu, Apr 17, 2014 at 2:54 AM, <thomas.mo...@orange.com> wrote: > Hi Lori, all, > > While I've attempted to adapt the layer 3 port patch to GRE tunneling, > and obtained nice results for IP-over-GRE (as I reported last week), > I've had a harder time reusing this framework for <non-IP>-over-GRE, in > particular for MPLS-over-GRE. > > Given its name, I maybe shouldn't have expected <non-IP>-over-GRE to > work on a "layer 3 port", but I did, given that this patch seems to > really be a generic framework for popping/pushing the Ethernet header > before/after tunnelling encap/decap, which is essentially what we need > for any non-Ethernet tunnel. > > Unfortunately, as it is, the patch assumes in some places that the > tunnel payload will be IP if it is not Ethernet, and does not generally > support the tunnel payload being some other ethertype. > > Practically speaking, making your layer 3 port code work for > MOLS-over-GRE is not entirely trivial: > - on emission, compose_output_action__ pops the Ethernet header before > push_mpls is called (which is done during commit_odp_actions) ; this > does not work since push_mpls actually relies on the presence of the > Ethernet header to set the Ethernet ethertype to an MPLS ethertype. One > way to fix it would be to adapt MPLS code to support the case where the > frame is layer3. Another is to have compose_output_action__ call > odp_put_pop_eth_action after commit_odp_actions. I implemented the > latter and it works. This latter option has the benefit preserving > compatibility with other actions that can be done before outputting and > which rely on the Ethernet header being there (such as changing a > Ethernet dst) -- such operations do not make sense if the header is > popped afterward, but I think we should try to not break if the user is > to define such a flow.
I think it's actually OK to break (or really ignore) actions that don't make sense. There are some similar examples already - if you try to set the TCP port on an L2 packet then the action will just get ignored. > - on reception, although I haven't fully tried to reach a resolution, > there is at least an issue in flow_extract: with the current patch flow > extract does not parse MPLS for a layer3 frame. I guess that could be > solved although I haven't clarified exactly yet how flow_extract would > guess what payload is in the ofppuf->packet (as I understand, this is > initially determined in the kernel datapath and stored in skb->protocol, > but it has to be propagated to userspace). I think this would probably have to come as metadata from the originator in some form, similar to the input port which also can't be extracted from the packet. In theory, this could either be from the kernel or OpenFlow directly. > Based on the above, I wonder if the code couldn't be made overall > simpler by *always* pushing an Ethernet header on reception of a > non-Ethernet tunnelled packet (not only when, after flow identification, > we conclude that the in_port is layer 3 while the out_port is not). > Similarly, on emission, the popping of the Ethernet header would not > have to be conditioned to the in/out ports being layer 3 or not. The > coexistence of the layer 3 port code with operations relying on the fact > that the packet being manipulated is Ethernet, would become a non-issue > ; the code related to these operations could remain completely untouched. > > In practice, that could be done by doing pop_eth based on flow actions > (as currently done by your patch), but by doing push_eth unconditionally > on reception of a packet from a layer3 tunnel (*not* as a flow action as > currently done in your patch). This is pretty much what is already done in the checked-in LISP code. I think at some point, we need to bite the bullet and figure out how to make OVS independent of Ethernet so this seems to be the time to do it. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
