[ovs-dev] Datapath action verification (was: [PATCH 2/2] [RFC] datapath: Make arp headers modifiable.)

[Justin Pettit]

On Jul 15, 2013, at 2:14 PM, Jesse Gross <jesse at nicira.com> wrote:

> On Mon, Jul 15, 2013 at 11:06 AM, Ben Pfaff <blp at nicira.com> wrote:
>>    * I am not sure that set_arp() is called in a context where there is
>>      guaranteed to be a full Ethernet+IP ARP header present in the packet,
>>      given megaflows.
> 
> This is a larger problem with all actions that I had mentioned before
> but it slipped through the cracks.
> 
> I think the correct solution here is to have the kernel reject flows
> that might not be safe (i.e. you can't modify a TCP header if there
> isn't an exact match on the IP protocol). This is pretty easy to do
> (just use the masked flow for validation) and consistent with how
> things are done elsewhere.
> 
> However, from an OpenFlow perspective we've traditionally allowed such
> a flow but just trimmed out these actions during flow setup. I believe
> that this will continue to work OK because we need to unmask fields in
> order to generate a set action, so we should both satisfy the kernel's
> dependency requirements and be able to do the trimming. Justin, do you
> agree?

Yes.  The code for the set actions un-wildcards the IP proto (and the ethertype is always un-wildcarded).  For the generic set action, the NXM code should enforce prerequisites, which will then be un-wildcarded as well.  If there are cases where this doesn't happen, I would consider them bugs in userspace.

--Justin