[ovs-dev] [RFC v3 3/5] ovn: Add a new OVN field icmp4.frag_mtu

[Ben Pfaff]

On Thu, Jan 10, 2019 at 11:30:58PM +0530, nusiddiq at redhat.com wrote:
> From: Numan Siddique <nusiddiq at redhat.com>
> 
> In order to support OVN specific fields (which are not yet
> supported in OpenvSwitch to set or modify values) a generic
> OVN field support is added in this patch. These OVN fields are
> expected to be used as nested OVN actions inside OVN actions
> like icmp4, icmp6 etc. This patch adds only one field for now
>  - icmp4.frag_mtu. It should be fairly straightforward to
> add similar fields in the near future.
> 
> This field is expected to be used as an inner field with in
> the 'icmp4' action.
> 
> Eg.
> "icmp4 {"eth.dst <-> eth.src; "
>         "icmp4.type = 3; /* Destination Unreachable */ "
>         "icmp4.code = 4; /* Fragmentation Needed */ "
>          icmp4.frag_mtu = 1442;
>          ...
>          "next; };",
> 
> pinctrl module of ovn-controller will set the specified value
> in the the low-order 16 bits of the ICMP4 header field that is
> labelled "unused" in the ICMP specification as defined in the RFC 1191.
> 
> Upcoming patch will use it to send an icmp4 packet if the
> source IPv4 packet destined to go via external gateway needs to
> be fragmented.
> 
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>

Thanks!

This ntohs in encode_OVNFIELD_LOAD should be htons:
        oah->len = ntohs(sizeof(ovs_be16));

In encode_nested_actions(), it's unsafe to use the pointer
n_ovnfields_acts after calling ovnacts_encode(), because the buffer
might have been reallocated.

I don't understand why icmp4.frag_mtu is so heavily special-cased that
it only works inside nested actions.

Thanks,

Ben.