From: Chris Mi <c...@nvidia.com>

Currently, tc merges all header rewrite actions into one tc pedit
action. So the header rewrite actions order is lost. Save each header
rewrite action into one tc pedit action to keep the order. And only
append one tc csum action to the last pedit action of a series.

Signed-off-by: Chris Mi <c...@nvidia.com>
Reviewed-by: Roi Dayan <r...@nvidia.com>
---
v3:
- add csum calc after last pedit call.

v2:
- rebased to fix conflict.

 lib/netdev-offload-tc.c | 22 +++++++-------------
 lib/tc.c                | 55 +++++++++++++++++++++++++++++++++++--------------
 lib/tc.h                | 25 +++++++++++-----------
 3 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 9845e8d3feae..ab2a678c6923 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -481,10 +481,10 @@ netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
 
 static void
 parse_flower_rewrite_to_netlink_action(struct ofpbuf *buf,
-                                       struct tc_flower *flower)
+                                       struct tc_action *action)
 {
-    char *mask = (char *) &flower->rewrite.mask;
-    char *data = (char *) &flower->rewrite.key;
+    char *mask = (char *) &action->rewrite.mask;
+    char *data = (char *) &action->rewrite.key;
 
     for (int type = 0; type < ARRAY_SIZE(set_flower_map); type++) {
         char *put = NULL;
@@ -877,7 +877,7 @@ parse_tc_flower_to_match(struct tc_flower *flower,
             }
             break;
             case TC_ACT_PEDIT: {
-                parse_flower_rewrite_to_netlink_action(buf, flower);
+                parse_flower_rewrite_to_netlink_action(buf, action);
             }
             break;
             case TC_ACT_ENCAP: {
@@ -1222,8 +1222,8 @@ parse_put_flow_set_masked_action(struct tc_flower *flower,
     uint64_t set_stub[1024 / 8];
     struct ofpbuf set_buf = OFPBUF_STUB_INITIALIZER(set_stub);
     char *set_data, *set_mask;
-    char *key = (char *) &flower->rewrite.key;
-    char *mask = (char *) &flower->rewrite.mask;
+    char *key = (char *) &action->rewrite.key;
+    char *mask = (char *) &action->rewrite.mask;
     const struct nlattr *attr;
     int i, j, type;
     size_t size;
@@ -1265,14 +1265,6 @@ parse_put_flow_set_masked_action(struct tc_flower 
*flower,
         }
     }
 
-    if (!is_all_zeros(&flower->rewrite, sizeof flower->rewrite)) {
-        if (flower->rewrite.rewrite == false) {
-            flower->rewrite.rewrite = true;
-            action->type = TC_ACT_PEDIT;
-            flower->action_count++;
-        }
-    }
-
     if (hasmask && !is_all_zeros(set_mask, size)) {
         VLOG_DBG_RL(&rl, "unsupported sub attribute of set action type %d",
                     type);
@@ -1281,6 +1273,8 @@ parse_put_flow_set_masked_action(struct tc_flower *flower,
     }
 
     ofpbuf_uninit(&set_buf);
+    action->type = TC_ACT_PEDIT;
+    flower->action_count++;
     return 0;
 }
 
diff --git a/lib/tc.c b/lib/tc.c
index adb2d3182ad4..abc7aa996242 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1006,14 +1006,14 @@ static const struct nl_policy pedit_policy[] = {
 static int
 nl_parse_act_pedit(struct nlattr *options, struct tc_flower *flower)
 {
-    struct tc_action *action;
+    struct tc_action *action = &flower->actions[flower->action_count++];
     struct nlattr *pe_attrs[ARRAY_SIZE(pedit_policy)];
     const struct tc_pedit *pe;
     const struct tc_pedit_key *keys;
     const struct nlattr *nla, *keys_ex, *ex_type;
     const void *keys_attr;
-    char *rewrite_key = (void *) &flower->rewrite.key;
-    char *rewrite_mask = (void *) &flower->rewrite.mask;
+    char *rewrite_key = (void *) &action->rewrite.key;
+    char *rewrite_mask = (void *) &action->rewrite.mask;
     size_t keys_ex_size, left;
     int type, i = 0, err;
 
@@ -1092,7 +1092,6 @@ nl_parse_act_pedit(struct nlattr *options, struct 
tc_flower *flower)
         i++;
     }
 
-    action = &flower->actions[flower->action_count++];
     action->type = TC_ACT_PEDIT;
 
     return 0;
@@ -2399,14 +2398,14 @@ nl_msg_put_act_flags(struct ofpbuf *request) {
  * first_word_mask/last_word_mask - the mask to use for the first/last read
  * (as we read entire words). */
 static void
-calc_offsets(struct tc_flower *flower, struct flower_key_to_pedit *m,
+calc_offsets(struct tc_action *action, struct flower_key_to_pedit *m,
              int *cur_offset, int *cnt, ovs_be32 *last_word_mask,
              ovs_be32 *first_word_mask, ovs_be32 **mask, ovs_be32 **data)
 {
     int start_offset, max_offset, total_size;
     int diff, right_zero_bits, left_zero_bits;
-    char *rewrite_key = (void *) &flower->rewrite.key;
-    char *rewrite_mask = (void *) &flower->rewrite.mask;
+    char *rewrite_key = (void *) &action->rewrite.key;
+    char *rewrite_mask = (void *) &action->rewrite.mask;
 
     max_offset = m->offset + m->size;
     start_offset = ROUND_DOWN(m->offset, 4);
@@ -2473,7 +2472,8 @@ csum_update_flag(struct tc_flower *flower,
 
 static int
 nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
-                                 struct tc_flower *flower)
+                                 struct tc_flower *flower,
+                                 struct tc_action *action)
 {
     struct {
         struct tc_pedit sel;
@@ -2497,7 +2497,7 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
             continue;
         }
 
-        calc_offsets(flower, m, &cur_offset, &cnt, &last_word_mask,
+        calc_offsets(action, m, &cur_offset, &cnt, &last_word_mask,
                      &first_word_mask, &mask, &data);
 
         for (j = 0; j < cnt; j++,  mask++, data++, cur_offset += 4) {
@@ -2556,6 +2556,29 @@ nl_msg_put_flower_acts_release(struct ofpbuf *request, 
uint16_t act_index)
     nl_msg_end_nested(request, act_offset);
 }
 
+/* Aggregates all previous successive pedit actions csum_update_flags
+ * to flower->csum_update_flags. Only append one csum action to the
+ * last pedit action. */
+static void
+nl_msg_put_csum_act(struct ofpbuf *request, struct tc_flower *flower,
+                    uint16_t *act_index)
+{
+    size_t act_offset;
+
+    /* No pedit actions or processed already. */
+    if (!flower->csum_update_flags) {
+        return;
+    }
+
+    act_offset = nl_msg_start_nested(request, (*act_index)++);
+    nl_msg_put_act_csum(request, flower->csum_update_flags);
+    nl_msg_put_act_flags(request);
+    nl_msg_end_nested(request, act_offset);
+
+    /* Clear it. So we can have another series of pedit actions. */
+    flower->csum_update_flags = 0;
+}
+
 static int
 nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
 {
@@ -2572,20 +2595,22 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
 
         action = flower->actions;
         for (i = 0; i < flower->action_count; i++, action++) {
+            if (action->type != TC_ACT_PEDIT) {
+                nl_msg_put_csum_act(request, flower, &act_index);
+            }
             switch (action->type) {
             case TC_ACT_PEDIT: {
                 act_offset = nl_msg_start_nested(request, act_index++);
-                error = nl_msg_put_flower_rewrite_pedits(request, flower);
+                error = nl_msg_put_flower_rewrite_pedits(request, flower,
+                                                         action);
                 if (error) {
                     return error;
                 }
                 nl_msg_end_nested(request, act_offset);
 
-                if (flower->csum_update_flags) {
-                    act_offset = nl_msg_start_nested(request, act_index++);
-                    nl_msg_put_act_csum(request, flower->csum_update_flags);
-                    nl_msg_put_act_flags(request);
-                    nl_msg_end_nested(request, act_offset);
+                if (i == flower->action_count - 1) {
+                    /* If this is the last action check csum calc again. */
+                    nl_msg_put_csum_act(request, flower, &act_index);
                 }
             }
             break;
diff --git a/lib/tc.h b/lib/tc.h
index a147ca461dc5..f08786b72528 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -256,11 +256,23 @@ struct tc_action {
             bool force;
             bool commit;
         } ct;
+
+        struct {
+            struct tc_flower_key key;
+            struct tc_flower_key mask;
+        } rewrite;
      };
 
      enum tc_action_type type;
 };
 
+/* assert that if we overflow with a masked write of uint32_t to the last byte
+ * of action.rewrite we overflow inside struct tc_action.
+ * shouldn't happen unless someone moves rewrite to the end of action */
+BUILD_ASSERT_DECL(offsetof(struct tc_action, rewrite)
+                  + MEMBER_SIZEOF(struct tc_action, rewrite)
+                  + sizeof(uint32_t) - 2 < sizeof(struct tc_action));
+
 enum tc_offloaded_state {
     TC_OFFLOADED_STATE_UNDEFINED,
     TC_OFFLOADED_STATE_IN_HW,
@@ -333,12 +345,6 @@ struct tc_flower {
     struct ovs_flow_stats stats;
     uint64_t lastused;
 
-    struct {
-        bool rewrite;
-        struct tc_flower_key key;
-        struct tc_flower_key mask;
-    } rewrite;
-
     uint32_t csum_update_flags;
 
     bool tunnel;
@@ -352,13 +358,6 @@ struct tc_flower {
     enum tc_offload_policy tc_policy;
 };
 
-/* assert that if we overflow with a masked write of uint32_t to the last byte
- * of flower.rewrite we overflow inside struct flower.
- * shouldn't happen unless someone moves rewrite to the end of flower */
-BUILD_ASSERT_DECL(offsetof(struct tc_flower, rewrite)
-                  + MEMBER_SIZEOF(struct tc_flower, rewrite)
-                  + sizeof(uint32_t) - 2 < sizeof(struct tc_flower));
-
 int tc_replace_flower(struct tcf_id *id, struct tc_flower *flower);
 int tc_del_filter(struct tcf_id *id);
 int tc_get_flower(struct tcf_id *id, struct tc_flower *flower);
-- 
2.8.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to