[ovs-dev] [RFC PATCH v2 02/13] Format NSH keys to readable strings

[Simon Horman]

Hi Jan,

On Wed, Aug 10, 2016 at 12:21:08PM +0000, Jan Scheurich wrote:
> > -----Original Message-----
> > From: Simon Horman [mailto:simon.horman at netronome.com]
> > Sent: Wednesday, 10 August, 2016 12:14
> > >
> > > My suggestion for the action set execution order would therefore be:
> > > 3.a	push_nsh
> > > 3.b 	push_eth
> > > 3.c	push_mpls
> > > 4	push_pbb
> > > 5	push_vlan
> > >
> > > I agree that this is somewhat arbitrary and can be debated, but one can
> > achieve any wanted order using the Apply Actions instruction.
> > 
> > I think that sounds reasonable.
> > 
> > Actually, I think the important thing is to define the ordering clearly.
> > I wonder if taking it to the ONF earlier than later would be wise. It will be
> > painful if at some point they chose a different ordering to what is present in the
> > wild.
> 
> That makes sense.
> Does anyone have a reach into the ONF to propose this as OF extensions?

I investigate this if none else is so inclined.

> > > Conversely, when you compose the pop_nsh action you need to trigger a
> > recirculation of the packet after pop_nsh to have the datapath reparse the
> > inner packet and subject it to another megaflow lookup. You should do that by
> > calling  ctx_trigger_freeze(ctx) after having composed the pop_mpls action.
> > >
> > > The same goes for the actions push_eth and pop_eth.
> > 
> > Yes, I believe that is true except for the last statement.
> > 
> > I think that push/pop_nsh will likely follow a similar pattern to push/pop_mpls
> > as both sets of actions may change the ethernet type of a packet.  But I think
> > the case of push/pop_eth is a little different. The way this is modelled is that
> > those actions affect the presence of an ethernet header (which is in itself a bit
> > special) but not the type of the packet.
> 
> I agree with you if the push_eth action is executed on a non-ethernet packet. 
> 
> But we also should consider the case that push_eth is executed on an ethernet frame, effectively creating a raw MAC in MAC encapsulation. I suggested to use the ethertype 0x6658 (used e.g. as protocol type for Transparent Ethernet Bridging (Ethernet over GRE)) for this.
> 
> See also http://openvswitch.org/pipermail/dev/2016-July/075632.html
> 
> Similarly for pop_eth.

I think that makes sense but we also need to keep track of the type of the
Ethernet headerless packet so we know how to intemperate its headers.

The scheme that has evolved (and still evolved) during review of the
kernel datapath push/pop_eth changes is basically for skb->protocol = 0x6658
to be used when OvS sends and receives a packet but that internally
OvS uses the type that the packet would have been if an ethernet header was
present. And that type is stored in skb->protocol.

This seems to work for the current l3-vpn (e.g. MPLS over GRE) work.
It may need to be updated along the lines you suggest for NSH.