[ovs-dev] [PATCH 2/4] Translation of generic encap and decap actions

Ben Pfaff blp at ovn.org
Wed Jul 12 03:58:51 UTC 2017


OK.

Are you willing to squash the first two patches for the next version?
It looks to me like they form a single logical change.

On Wed, Jul 12, 2017 at 12:41:10AM +0000, Yang, Yi Y wrote:
> Ben, encap action is just adding an empty header, set_field action will set the fields in the encapped header, so encap and set actions can be combined if we defer encap action, this is an optimization.
> 
> -----Original Message-----
> From: Ben Pfaff [mailto:blp at ovn.org] 
> Sent: Wednesday, July 12, 2017 8:28 AM
> To: Zoltán Balogh <zoltan.balogh at ericsson.com>
> Cc: 'dev at openvswitch.org' <dev at openvswitch.org>; Jan Scheurich <jan.scheurich at ericsson.com>; Georg Schmuecking <Georg.Schmuecking at ericsson.com>; Yang, Yi Y <yi.y.yang at intel.com>; Jiri Benc (jbenc at redhat.com) <jbenc at redhat.com>
> Subject: Re: [PATCH 2/4] Translation of generic encap and decap actions
> 
> On Fri, Jun 30, 2017 at 03:29:32PM +0000, Zoltán Balogh wrote:
> > From: Jan Scheurich <jan.scheurich at ericsson.com>
> > 
> > This commit implements a skeleton for the translation of generic encap 
> > and decap actions in ofproto-dpif and adds support to encap and decap 
> > an Ethernet header.
> > 
> > In general translation of encap commits pending actions and then 
> > rewrites struct flow in accordance with the new packet type and 
> > header. In the case of encap(ethernet) it suffices to change the 
> > packet type from (1, Ethertype) to (0,0) and set the dl_type 
> > accordingly. A new pending_encap flag in xlate ctx is set to mark that 
> > an corresponding datapath encap action must be triggered at the next 
> > commit. In the case of encap(ethernet) ofproto generetas a push_eth action.
> > 
> > The general case for translation of decap() is to emit a datapath 
> > action to decap the current outermost header and then recirculate the 
> > packet to reparse the inner headers. In the special case of an 
> > Ethernet packet,
> > decap() just changes the packet type from (0,0) to (1, dl_type) 
> > without a need to recirculate. The emission of the pop_eth action for 
> > the datapath is postponed to the next commit.
> > 
> > Hence encap(ethernet) and decap() on an Ethernet packet are OF octions 
> > that only incur a cost in the dataplane when a modifed packet is 
> > actually committed, e.g. because it is sent out. They can freely be 
> > used for normalizing the packet type in the OF pipeline without 
> > degrading performance.
> > 
> > Signed-off-by: Jan Scheurich <jan.scheurich at ericsson.com>
> > Signed-off-by: Yi Yang <yi.y.yang at intel.com>
> 
> Thanks for posting this.
> 
> It looks like a lot of the purpose of this commit is to fill in the blanks with the first commit in the series, fixing some of the issues and implementing bits that were missing.  I think that it would be easier to review if the two commits were squashed together.
> 
> I am not sure that I understand the way of doing deferred encap actions.  It looks like, for example, one can chain a number of encap actions, but that the result is just encapsulating once.  Am I reading that right?
> 
> Thanks,
> 
> Ben.


More information about the dev mailing list