diff mbox series

[ovs-dev,RFC,v4,2/4] ovn: Add a new OVN field icmp4.frag_mtu

Message ID 20190320112619.22919-1-nusiddiq@redhat.com
State Superseded
Headers show
Series Address MTU issue for larger packets in OVN | expand

Commit Message

Numan Siddique March 20, 2019, 11:26 a.m. UTC
From: Numan Siddique <nusiddiq@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@redhat.com>
---
 include/ovn/actions.h                     |  25 +++-
 include/ovn/automake.mk                   |   3 +-
 include/ovn/expr.h                        |   5 +
 {ovn/lib => include/ovn}/logical-fields.h |  41 +++++++
 ovn/controller/lflow.c                    |   1 +
 ovn/controller/lflow.h                    |   2 +-
 ovn/controller/pinctrl.c                  |  23 +++-
 ovn/lib/actions.c                         | 141 +++++++++++++++++-----
 ovn/lib/automake.mk                       |   1 -
 ovn/lib/expr.c                            |  17 ++-
 ovn/lib/logical-fields.c                  |  36 +++++-
 ovn/northd/ovn-northd.c                   |   2 +-
 ovn/ovn-sb.xml                            |  11 ++
 ovn/utilities/ovn-trace.c                 |   5 +-
 tests/ovn.at                              |  18 ++-
 tests/test-ovn.c                          |   2 +-
 16 files changed, 288 insertions(+), 45 deletions(-)
 rename {ovn/lib => include/ovn}/logical-fields.h (71%)
diff mbox series

Patch

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 1c0c67ce6..095b60df0 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -80,7 +80,8 @@  struct ovn_extend_table;
     OVNACT(LOG,               ovnact_log)             \
     OVNACT(PUT_ND_RA_OPTS,    ovnact_put_opts)        \
     OVNACT(ND_NS,             ovnact_nest)            \
-    OVNACT(SET_METER,         ovnact_set_meter)
+    OVNACT(SET_METER,         ovnact_set_meter)       \
+    OVNACT(OVNFIELD_LOAD,     ovnact_load)
 
 /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -142,6 +143,20 @@  ovnact_end(const struct ovnact *ovnacts, size_t ovnacts_len)
 #define OVNACT_FOR_EACH(POS, OVNACTS, OVNACTS_LEN)                      \
     for ((POS) = (OVNACTS); (POS) < ovnact_end(OVNACTS, OVNACTS_LEN);  \
          (POS) = ovnact_next(POS))
+
+static inline int
+ovnacts_count(const struct ovnact *ovnacts, size_t ovnacts_len)
+{
+    uint8_t n_ovnacts = 0;
+    if (ovnacts) {
+        const struct ovnact *a;
+
+        OVNACT_FOR_EACH (a, ovnacts, ovnacts_len) {
+            n_ovnacts++;
+        }
+    }
+    return n_ovnacts;
+}
 
 /* Action structure for each OVNACT_*. */
 
@@ -229,6 +244,8 @@  struct ovnact_nest {
     struct ovnact ovnact;
     struct ovnact *nested;
     size_t nested_len;
+    struct ovnact *nested_ovnfields;
+    size_t nested_ovnfields_len;
 };
 
 /* OVNACT_GET_ARP, OVNACT_GET_ND. */
@@ -461,6 +478,12 @@  struct action_header {
 };
 BUILD_ASSERT_DECL(sizeof(struct action_header) == 8);
 
+OVS_PACKED(
+struct ovnfield_act_header {
+    ovs_be16 id; /* one of enum ovnfield_id. */
+    ovs_be16 len; /* Length of the ovnfield data. */
+});
+
 struct ovnact_parse_params {
     /* A table of "struct expr_symbol"s to support (as one would provide to
      * expr_parse()). */
diff --git a/include/ovn/automake.mk b/include/ovn/automake.mk
index d2924c2f6..54b0e2c0e 100644
--- a/include/ovn/automake.mk
+++ b/include/ovn/automake.mk
@@ -2,4 +2,5 @@  ovnincludedir = $(includedir)/ovn
 ovninclude_HEADERS = \
 	include/ovn/actions.h \
 	include/ovn/expr.h \
-	include/ovn/lex.h
+	include/ovn/lex.h  \
+	include/ovn/logical-fields.h
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 3995e62f0..a68fd6e1c 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -58,6 +58,7 @@ 
 #include "openvswitch/list.h"
 #include "openvswitch/match.h"
 #include "openvswitch/meta-flow.h"
+#include "logical-fields.h"
 
 struct ds;
 struct expr;
@@ -244,6 +245,7 @@  struct expr_symbol {
     int width;
 
     const struct mf_field *field;     /* Fields only, otherwise NULL. */
+    const struct ovn_field *ovn_field;  /* OVN Fields only, otherwise NULL. */
     const struct expr_symbol *parent; /* Subfields only, otherwise NULL. */
     int parent_ofs;                   /* Subfields only, otherwise 0. */
     char *predicate;                  /* Predicates only, otherwise NULL. */
@@ -284,6 +286,9 @@  struct expr_symbol *expr_symtab_add_string(struct shash *symtab,
 struct expr_symbol *expr_symtab_add_predicate(struct shash *symtab,
                                               const char *name,
                                               const char *expansion);
+struct expr_symbol *expr_symtab_add_ovn_field(struct shash *symtab,
+                                              const char *name,
+                                              enum ovn_field_id id);
 void expr_symtab_destroy(struct shash *symtab);
 
 /* Expression type. */
diff --git a/ovn/lib/logical-fields.h b/include/ovn/logical-fields.h
similarity index 71%
rename from ovn/lib/logical-fields.h
rename to include/ovn/logical-fields.h
index 95759a8bb..968faf875 100644
--- a/ovn/lib/logical-fields.h
+++ b/include/ovn/logical-fields.h
@@ -81,4 +81,45 @@  enum mff_log_flags {
     MLF_NESTED_CONTAINER = (1 << MLF_NESTED_CONTAINER_BIT),
 };
 
+/* OVN logical fields
+ * ===================
+ * These are the fields which OVN supports modifying within an OVN action
+ * which gets translated to OFFlow controller action. The value to be set
+ * for these fields will be encoded with in the 'userdata' field of the
+ * controller action.
+ *
+ * OpenvSwitch doesn't support modifying these fields yet. If a field is
+ * supported later by OpenvSwitch, it can be deleted from here.
+ */
+
+enum ovn_field_id {
+    /*
+     * Name: "icmp4.frag_mtu" -
+     * Type: be16
+     * Description: Sets the low-order 16 bits of the ICMP4 header field
+     * (that is labelled "unused" in the ICMP specification) of the ICMP4
+     * packet as per the RFC 1191.
+     */
+    OVN_ICMP4_FRAG_MTU,
+
+    OVN_FIELD_N_IDS
+};
+
+struct ovn_field {
+    enum ovn_field_id id;
+    const char *name;
+    unsigned int n_bytes;       /* Width of the field in bytes. */
+    unsigned int n_bits;        /* Number of significant bits in field. */
+};
+
+static inline const struct ovn_field *
+ovn_field_from_id(enum ovn_field_id id)
+{
+    extern const struct ovn_field ovn_fields[OVN_FIELD_N_IDS];
+    ovs_assert((unsigned int) id < OVN_FIELD_N_IDS);
+    return &ovn_fields[id];
+}
+
+const struct ovn_field *ovn_field_from_name(const char *name);
+void ovn_destroy_ovnfields(void);
 #endif /* ovn/lib/logical-fields.h */
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 8db81927e..432edbc70 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -495,4 +495,5 @@  lflow_destroy(void)
 {
     expr_symtab_destroy(&symtab);
     shash_destroy(&symtab);
+    ovn_destroy_ovnfields();
 }
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index d19338140..efa066737 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -16,7 +16,7 @@ 
 #ifndef OVN_LFLOW_H
 #define OVN_LFLOW_H 1
 
-#include "ovn/lib/logical-fields.h"
+#include "ovn/logical-fields.h"
 
 /* Logical_Flow table translation to OpenFlow
  * ==========================================
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 100a20ff2..fda594109 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -42,9 +42,9 @@ 
 #include "ovn/actions.h"
 #include "ovn/lex.h"
 #include "ovn/lib/acl-log.h"
-#include "ovn/lib/logical-fields.h"
 #include "ovn/lib/ovn-l7.h"
 #include "ovn/lib/ovn-util.h"
+#include "ovn/logical-fields.h"
 #include "openvswitch/poll-loop.h"
 #include "openvswitch/rconn.h"
 #include "socket-util.h"
@@ -450,6 +450,27 @@  pinctrl_handle_icmp(const struct flow *ip_flow, struct dp_packet *pkt_in,
         struct icmp_header *ih = dp_packet_put_zeros(&packet, sizeof *ih);
         dp_packet_set_l4(&packet, ih);
         packet_set_icmp(&packet, ICMP4_DST_UNREACH, 1);
+        uint8_t *n_ovnfield_acts = ofpbuf_try_pull(userdata,
+                                                   sizeof *n_ovnfield_acts);
+        if (n_ovnfield_acts && *n_ovnfield_acts) {
+            for (uint8_t i = 0; i < *n_ovnfield_acts; i++) {
+                 struct ovnfield_act_header *oah =
+                     ofpbuf_try_pull(userdata, sizeof *oah);
+
+                 if (!oah) {
+                    break;
+                 }
+                 switch (ntohs(oah->id)) {
+                 case OVN_ICMP4_FRAG_MTU: {
+                     ovs_be16 *mtu = ofpbuf_try_pull(userdata, sizeof *mtu);
+                     if (mtu) {
+                         ih->icmp_fields.frag.mtu = *mtu;
+                     }
+                     break;
+                 }
+                 }
+            }
+        }
     } else {
         struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh);
         struct icmp6_error_header *ih;
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 7b7a89478..36d21c400 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -23,7 +23,6 @@ 
 #include "ovn-l7.h"
 #include "hash.h"
 #include "lib/packets.h"
-#include "logical-fields.h"
 #include "nx-match.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/hmap.h"
@@ -194,6 +193,7 @@  struct action_context {
     struct ofpbuf *ovnacts;     /* Actions. */
     struct expr *prereqs;       /* Prerequisites to apply to match. */
     int depth;                  /* Current nested action depth. */
+    struct ofpbuf *ovnfield_acts; /* Actions for OVN fields. */
 };
 
 static void parse_actions(struct action_context *, enum lex_type sentinel);
@@ -366,19 +366,33 @@  ovnact_next_free(struct ovnact_next *a OVS_UNUSED)
 static void
 parse_LOAD(struct action_context *ctx, const struct expr_field *lhs)
 {
-    size_t ofs = ctx->ovnacts->size;
-    struct ovnact_load *load = ovnact_put_LOAD(ctx->ovnacts);
+    struct ofpbuf *ovnacts = ctx->ovnacts;
+    size_t ofs = ovnacts->size;
+    struct ovnact_load *load;
+    if (lhs->symbol->ovn_field) {
+        if (!ctx->ovnfield_acts) {
+            lexer_syntax_error(ctx->lexer,
+                               "Invalid usage of OVN field : %s",
+                               lhs->symbol->name);
+            return;
+        }
+        ovnacts = ctx->ovnfield_acts;
+        load = ovnact_put_OVNFIELD_LOAD(ovnacts);
+    } else {
+        load = ovnact_put_LOAD(ovnacts);
+    }
+
     load->dst = *lhs;
 
     char *error = expr_type_check(lhs, lhs->n_bits, true);
     if (error) {
-        ctx->ovnacts->size = ofs;
+        ovnacts->size = ofs;
         lexer_error(ctx->lexer, "%s", error);
         free(error);
         return;
     }
     if (!expr_constant_parse(ctx->lexer, lhs, &load->imm)) {
-        ctx->ovnacts->size = ofs;
+        ovnacts->size = ofs;
         return;
     }
 }
@@ -1089,7 +1103,7 @@  encode_CT_CLEAR(const struct ovnact_null *null OVS_UNUSED,
  * actions on a packet derived from the one being processed. */
 static void
 parse_nested_action(struct action_context *ctx, enum ovnact_type type,
-                    const char *prereq)
+                    const char *prereq, bool parse_ovn_fields)
 {
     if (!lexer_force_match(ctx->lexer, LEX_T_LCURLY)) {
         return;
@@ -1102,14 +1116,19 @@  parse_nested_action(struct action_context *ctx, enum ovnact_type type,
 
     uint64_t stub[1024 / 8];
     struct ofpbuf nested = OFPBUF_STUB_INITIALIZER(stub);
-
+    uint64_t ovnfields_stub[512 / 8];
+    struct ofpbuf nested_ovnfields = OFPBUF_STUB_INITIALIZER(ovnfields_stub);
     struct action_context inner_ctx = {
         .pp = ctx->pp,
         .lexer = ctx->lexer,
         .ovnacts = &nested,
         .prereqs = NULL,
         .depth = ctx->depth + 1,
+        .ovnfield_acts = NULL,
     };
+    if (parse_ovn_fields) {
+        inner_ctx.ovnfield_acts = &nested_ovnfields;
+    }
     parse_actions(&inner_ctx, LEX_T_RCURLY);
 
     if (prereq) {
@@ -1134,61 +1153,68 @@  parse_nested_action(struct action_context *ctx, enum ovnact_type type,
                                         OVNACT_ALIGN(sizeof *on));
     on->nested_len = nested.size;
     on->nested = ofpbuf_steal_data(&nested);
+    on->nested_ovnfields_len = nested_ovnfields.size;
+    if (parse_ovn_fields && on->nested_ovnfields_len) {
+        on->nested_ovnfields = ofpbuf_steal_data(&nested_ovnfields);
+    }
 }
 
 static void
 parse_ARP(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_ARP, "ip4");
+    parse_nested_action(ctx, OVNACT_ARP, "ip4", false);
 }
 
 static void
 parse_ICMP4(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_ICMP4, "ip4");
+    parse_nested_action(ctx, OVNACT_ICMP4, "ip4", true);
 }
 
 static void
 parse_ICMP6(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_ICMP6, "ip6");
+    parse_nested_action(ctx, OVNACT_ICMP6, "ip6", false);
 }
 
 static void
 parse_TCP_RESET(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_TCP_RESET, "tcp");
+    parse_nested_action(ctx, OVNACT_TCP_RESET, "tcp", false);
 }
 
 static void
 parse_ND_NA(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_ND_NA, "nd_ns");
+    parse_nested_action(ctx, OVNACT_ND_NA, "nd_ns", false);
 }
 
 static void
 parse_ND_NA_ROUTER(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_ND_NA_ROUTER, "nd_ns");
+    parse_nested_action(ctx, OVNACT_ND_NA_ROUTER, "nd_ns", false);
 }
 
 static void
 parse_ND_NS(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_ND_NS, "ip6");
+    parse_nested_action(ctx, OVNACT_ND_NS, "ip6", false);
 }
 
 static void
 parse_CLONE(struct action_context *ctx)
 {
-    parse_nested_action(ctx, OVNACT_CLONE, NULL);
+    parse_nested_action(ctx, OVNACT_CLONE, NULL, false);
 }
 
 static void
 format_nested_action(const struct ovnact_nest *on, const char *name,
-                     struct ds *s)
+                     struct ds *s, bool format_ovn_fields)
 {
     ds_put_format(s, "%s { ", name);
+    if (format_ovn_fields && on->nested_ovnfields_len) {
+        ovnacts_format(on->nested_ovnfields, on->nested_ovnfields_len, s);
+    }
     ovnacts_format(on->nested, on->nested_len, s);
     ds_put_format(s, " };");
 }
@@ -1196,56 +1222,57 @@  format_nested_action(const struct ovnact_nest *on, const char *name,
 static void
 format_ARP(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "arp", s);
+    format_nested_action(nest, "arp", s, false);
 }
 
 static void
 format_ICMP4(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "icmp4", s);
+    format_nested_action(nest, "icmp4", s, true);
 }
 
 static void
 format_ICMP6(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "icmp6", s);
+    format_nested_action(nest, "icmp6", s, false);
 }
 
 static void
 format_TCP_RESET(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "tcp_reset", s);
+    format_nested_action(nest, "tcp_reset", s, false);
 }
 
 static void
 format_ND_NA(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "nd_na", s);
+    format_nested_action(nest, "nd_na", s, false);
 }
 
 static void
 format_ND_NA_ROUTER(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "nd_na_router", s);
+    format_nested_action(nest, "nd_na_router", s, false);
 }
 
 static void
 format_ND_NS(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "nd_ns", s);
+    format_nested_action(nest, "nd_ns", s, false);
 }
 
 static void
 format_CLONE(const struct ovnact_nest *nest, struct ds *s)
 {
-    format_nested_action(nest, "clone", s);
+    format_nested_action(nest, "clone", s, false);
 }
 
 static void
 encode_nested_actions(const struct ovnact_nest *on,
                       const struct ovnact_encode_params *ep,
                       enum action_opcode opcode,
-                      struct ofpbuf *ofpacts)
+                      struct ofpbuf *ofpacts,
+                      bool encode_ovn_fields)
 {
     /* Convert nested actions into ofpacts. */
     uint64_t inner_ofpacts_stub[1024 / 8];
@@ -1258,6 +1285,19 @@  encode_nested_actions(const struct ovnact_nest *on,
      * switch inside an OFPT_PACKET_OUT message. */
     size_t oc_offset = encode_start_controller_op(opcode, false,
                                                   NX_CTLR_NO_METER, ofpacts);
+    if (encode_ovn_fields) {
+        uint64_t ovn_fields_stub[128 / 8];
+        struct ofpbuf ovn_fields_ofpacts =
+            OFPBUF_STUB_INITIALIZER(ovn_fields_stub);
+
+        ovnacts_encode(on->nested_ovnfields, on->nested_ovnfields_len,
+                       ep, &ovn_fields_ofpacts);
+        uint8_t *n_ovnfields_acts = ofpbuf_put_zeros(ofpacts, 1);
+        *n_ovnfields_acts = ovnacts_count(on->nested_ovnfields,
+                                          on->nested_ovnfields_len);
+        ofpbuf_put(ofpacts, ovn_fields_ofpacts.data, ovn_fields_ofpacts.size);
+        ofpbuf_uninit(&ovn_fields_ofpacts);
+    }
     ofpacts_put_openflow_actions(inner_ofpacts.data, inner_ofpacts.size,
                                  ofpacts, OFP13_VERSION);
     encode_finish_controller_op(oc_offset, ofpacts);
@@ -1271,7 +1311,7 @@  encode_ARP(const struct ovnact_nest *on,
            const struct ovnact_encode_params *ep,
            struct ofpbuf *ofpacts)
 {
-    encode_nested_actions(on, ep, ACTION_OPCODE_ARP, ofpacts);
+    encode_nested_actions(on, ep, ACTION_OPCODE_ARP, ofpacts, false);
 }
 
 static void
@@ -1279,7 +1319,7 @@  encode_ICMP4(const struct ovnact_nest *on,
              const struct ovnact_encode_params *ep,
              struct ofpbuf *ofpacts)
 {
-    encode_nested_actions(on, ep, ACTION_OPCODE_ICMP, ofpacts);
+    encode_nested_actions(on, ep, ACTION_OPCODE_ICMP, ofpacts, true);
 }
 
 static void
@@ -1287,7 +1327,7 @@  encode_ICMP6(const struct ovnact_nest *on,
              const struct ovnact_encode_params *ep,
              struct ofpbuf *ofpacts)
 {
-    encode_nested_actions(on, ep, ACTION_OPCODE_ICMP, ofpacts);
+    encode_nested_actions(on, ep, ACTION_OPCODE_ICMP, ofpacts, false);
 }
 
 static void
@@ -1295,7 +1335,7 @@  encode_TCP_RESET(const struct ovnact_nest *on,
                  const struct ovnact_encode_params *ep,
                  struct ofpbuf *ofpacts)
 {
-    encode_nested_actions(on, ep, ACTION_OPCODE_TCP_RESET, ofpacts);
+    encode_nested_actions(on, ep, ACTION_OPCODE_TCP_RESET, ofpacts, false);
 }
 
 static void
@@ -1303,7 +1343,7 @@  encode_ND_NA(const struct ovnact_nest *on,
              const struct ovnact_encode_params *ep,
              struct ofpbuf *ofpacts)
 {
-    encode_nested_actions(on, ep, ACTION_OPCODE_ND_NA, ofpacts);
+    encode_nested_actions(on, ep, ACTION_OPCODE_ND_NA, ofpacts, false);
 }
 
 static void
@@ -1311,7 +1351,8 @@  encode_ND_NA_ROUTER(const struct ovnact_nest *on,
              const struct ovnact_encode_params *ep,
              struct ofpbuf *ofpacts)
 {
-    encode_nested_actions(on, ep, ACTION_OPCODE_ND_NA_ROUTER, ofpacts);
+    encode_nested_actions(on, ep, ACTION_OPCODE_ND_NA_ROUTER, ofpacts,
+                          false);
 }
 
 static void
@@ -1319,7 +1360,7 @@  encode_ND_NS(const struct ovnact_nest *on,
              const struct ovnact_encode_params *ep,
              struct ofpbuf *ofpacts)
 {
-    encode_nested_actions(on, ep, ACTION_OPCODE_ND_NS, ofpacts);
+    encode_nested_actions(on, ep, ACTION_OPCODE_ND_NS, ofpacts, false);
 }
 
 static void
@@ -2295,6 +2336,42 @@  ovnact_set_meter_free(struct ovnact_set_meter *ct OVS_UNUSED)
 {
 }
 
+static void
+format_OVNFIELD_LOAD(const struct ovnact_load *load , struct ds *s)
+{
+    const struct ovn_field *f = ovn_field_from_name(load->dst.symbol->name);
+    switch (f->id) {
+    case OVN_ICMP4_FRAG_MTU:
+        ds_put_format(s, "%s = %u; ", f->name,
+                      ntohs(load->imm.value.be16_int));
+        break;
+
+    case OVN_FIELD_N_IDS:
+    default:
+        OVS_NOT_REACHED();
+    }
+}
+
+static void
+encode_OVNFIELD_LOAD(const struct ovnact_load *load,
+            const struct ovnact_encode_params *ep OVS_UNUSED,
+            struct ofpbuf *ofpacts)
+{
+    const struct ovn_field *f = ovn_field_from_name(load->dst.symbol->name);
+    struct ovnfield_act_header *oah = ofpbuf_put_zeros(ofpacts, sizeof *oah);
+    oah->id = htons(f->id);
+    switch (f->id) {
+    case OVN_ICMP4_FRAG_MTU:
+        oah->len = htons(sizeof(ovs_be16));
+        ofpbuf_put(ofpacts, &load->imm.value.be16_int, sizeof(ovs_be16));
+        break;
+
+    case OVN_FIELD_N_IDS:
+    default:
+        OVS_NOT_REACHED();
+    }
+}
+
 /* Parses an assignment or exchange or put_dhcp_opts action. */
 static void
 parse_set_action(struct action_context *ctx)
diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk
index 6178fc2d5..f8a5b5f0f 100644
--- a/ovn/lib/automake.mk
+++ b/ovn/lib/automake.mk
@@ -17,7 +17,6 @@  ovn_lib_libovn_la_SOURCES = \
 	ovn/lib/ovn-util.c \
 	ovn/lib/ovn-util.h \
 	ovn/lib/logical-fields.c \
-	ovn/lib/logical-fields.h
 nodist_ovn_lib_libovn_la_SOURCES = \
 	ovn/lib/ovn-nb-idl.c \
 	ovn/lib/ovn-nb-idl.h \
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 8cfdf34fa..e21206120 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -17,7 +17,6 @@ 
 #include <config.h>
 #include "byte-order.h"
 #include "openvswitch/json.h"
-#include "logical-fields.h"
 #include "nx-match.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/match.h"
@@ -26,6 +25,7 @@ 
 #include "openvswitch/shash.h"
 #include "ovn/expr.h"
 #include "ovn/lex.h"
+#include "ovn/logical-fields.h"
 #include "simap.h"
 #include "sset.h"
 #include "util.h"
@@ -1381,6 +1381,8 @@  expr_symbol_format(const struct expr_symbol *symbol, struct ds *s)
         expr_field_format(&f, s);
     } else if (symbol->predicate) {
         ds_put_cstr(s, symbol->predicate);
+    } else if (symbol->ovn_field) {
+        ds_put_cstr(s, symbol->name);
     } else {
         nx_format_field_name(symbol->field->id, OFP13_VERSION, s);
     }
@@ -1556,6 +1558,19 @@  expr_symtab_add_predicate(struct shash *symtab, const char *name,
     return symbol;
 }
 
+struct expr_symbol *
+expr_symtab_add_ovn_field(struct shash *symtab, const char *name,
+                          enum ovn_field_id id)
+{
+    const struct ovn_field *ovn_field = ovn_field_from_id(id);
+    struct expr_symbol *symbol;
+
+    symbol = add_symbol(symtab, name, ovn_field->n_bits, NULL,
+                        EXPR_L_NOMINAL, false, true);
+    symbol->ovn_field = ovn_field;
+    return symbol;
+}
+
 /* Destroys 'symtab' and all of its symbols. */
 void
 expr_symtab_destroy(struct shash *symtab)
diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index a8b5e3c51..579537d6b 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -15,13 +15,25 @@ 
 
 #include <config.h>
 
-#include "logical-fields.h"
-
 #include "openvswitch/shash.h"
 #include "ovn/expr.h"
+#include "ovn/logical-fields.h"
 #include "ovs-thread.h"
 #include "packets.h"
 
+/* Silence a warning. */
+extern const struct ovn_field ovn_fields[OVN_FIELD_N_IDS];
+
+const struct ovn_field ovn_fields[OVN_FIELD_N_IDS] = {
+    {
+        OVN_ICMP4_FRAG_MTU,
+        "icmp4.frag_mtu",
+        2, 16,
+    },
+};
+
+static struct shash ovnfield_by_name;
+
 static void
 add_subregister(const char *name,
                 const char *parent_name, int parent_idx,
@@ -203,4 +215,24 @@  ovn_init_symtab(struct shash *symtab)
     expr_symtab_add_predicate(symtab, "sctp", "ip.proto == 132");
     expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false);
     expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
+
+    shash_init(&ovnfield_by_name);
+    for (int i = 0; i < OVN_FIELD_N_IDS; i++) {
+        const struct ovn_field *of = &ovn_fields[i];
+        ovs_assert(of->id == i); /* Fields must be in the enum order. */
+        shash_add_once(&ovnfield_by_name, of->name, of);
+    }
+    expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU);
+}
+
+const struct ovn_field *
+ovn_field_from_name(const char *name)
+{
+    return shash_find_data(&ovnfield_by_name, name);
+}
+
+void
+ovn_destroy_ovnfields(void)
+{
+    shash_destroy(&ovnfield_by_name);
 }
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 2843969f4..b56ccd4dc 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -29,12 +29,12 @@ 
 #include "openvswitch/json.h"
 #include "ovn/lex.h"
 #include "ovn/lib/chassis-index.h"
-#include "ovn/lib/logical-fields.h"
 #include "ovn/lib/ovn-l7.h"
 #include "ovn/lib/ovn-nb-idl.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "ovn/lib/ovn-util.h"
 #include "ovn/actions.h"
+#include "ovn/logical-fields.h"
 #include "packets.h"
 #include "openvswitch/poll-loop.h"
 #include "smap.h"
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 4e080abff..977cb8bc0 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1843,6 +1843,17 @@ 
             <li><code>icmp4.code = 1</code> (host unreachable)</li>
           </ul>
 
+          <p>
+            This action also allows setting the fragmentation mtu in the
+            ICMPv4 packet header as per RFC 1191. To set this
+            <code>icmp4.frag_mtu</code> field can be used.
+          </p>
+
+          <ul>
+            <li><code>icmp4.type = 3</code> (destination unreachable)</li>
+            <li><code>icmp4.code = 4</code> (Fragmentation Needed)</li>
+            <li><code>icmp4.frag_mtu = 1500</code></li>
+          </ul>
           <p><b>Prerequisite:</b> <code>ip4</code></p>
         </dd>
 
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 8899496a5..b00f53a5c 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -35,8 +35,8 @@ 
 #include "ovn/actions.h"
 #include "ovn/expr.h"
 #include "ovn/lex.h"
+#include "ovn/logical-fields.h"
 #include "ovn/lib/acl-log.h"
-#include "ovn/lib/logical-fields.h"
 #include "ovn/lib/ovn-l7.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "ovn/lib/ovn-util.h"
@@ -2106,6 +2106,9 @@  trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
             execute_tcp_reset(ovnact_get_TCP_RESET(a), dp, uflow, table_id,
                               pipeline, super);
             break;
+
+        case OVNACT_OVNFIELD_LOAD:
+            break;
         }
 
     }
diff --git a/tests/ovn.at b/tests/ovn.at
index f2f2bc405..7f096699a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1280,14 +1280,28 @@  reg1[0] = put_nd_ra_opts(addr_mode = "slaac", mtu = 10.0.0.4, slla = ae:01:02:03
 
 # icmp4
 icmp4 { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
-    encodes as controller(userdata=00.00.00.0a.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64)
+    encodes as controller(userdata=00.00.00.0a.00.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64)
     has prereqs ip4
 
 icmp4 { };
     formats as icmp4 { drop; };
-    encodes as controller(userdata=00.00.00.0a.00.00.00.00)
+    encodes as controller(userdata=00.00.00.0a.00.00.00.00.00)
+    has prereqs ip4
+
+# icmp4 with icmp4.frag_mtu
+icmp4 { eth.dst = ff:ff:ff:ff:ff:ff; icmp4.frag_mtu = 1500; output; }; output;
+    formats as icmp4 { icmp4.frag_mtu = 1500; eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
+    encodes as controller(userdata=00.00.00.0a.00.00.00.00.01.00.00.00.02.05.dc.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64)
     has prereqs ip4
 
+# icmp4.frag_mtu - Should fail
+icmp6 { eth.dst = ff:ff:ff:ff:ff:ff; icmp4.frag_mtu = 1500; output; }; output;
+    Syntax error at `1500' Invalid usage of OVN field : icmp4.frag_mtu.
+
+# icmp4.frag_mtu - Should fail
+icmp4.frag_mtu = 1500;
+    Syntax error at `1500' Invalid usage of OVN field : icmp4.frag_mtu.
+
 # icmp6
 icmp6 { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
     encodes as controller(userdata=00.00.00.0a.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64)
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index 9e3112bf9..7cce9c2ae 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -31,7 +31,7 @@ 
 #include "ovn/actions.h"
 #include "ovn/expr.h"
 #include "ovn/lex.h"
-#include "ovn/lib/logical-fields.h"
+#include "ovn/logical-fields.h"
 #include "ovn/lib/ovn-l7.h"
 #include "ovn/lib/extend-table.h"
 #include "ovs-thread.h"