On 14/08/2017 16:00, Simon Horman wrote:
On Tue, Aug 08, 2017 at 05:21:53PM +0300, Roi Dayan wrote:
From: Paul Blakey <pa...@mellanox.com>

To be later used to implement ovs action set offloading.

Signed-off-by: Paul Blakey <pa...@mellanox.com>
Reviewed-by: Roi Dayan <r...@mellanox.com>
---
  lib/tc.c | 372 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
  lib/tc.h |  12 +++
  2 files changed, 381 insertions(+), 3 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c

...

@@ -589,6 +758,10 @@ nl_parse_single_action(struct nlattr *action, struct 
tc_flower *flower)
          nl_parse_act_vlan(act_options, flower);
      } else if (!strcmp(act_kind, "tunnel_key")) {
          nl_parse_act_tunnel_key(act_options, flower);
+    } else if (!strcmp(act_kind, "pedit")) {
+        nl_parse_act_pedit(act_options, flower);
+    } else if (!strcmp(act_kind, "csum")) {
+        /* not doing anything for now */
      } else {
          VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind);
          return EINVAL;
@@ -790,6 +963,48 @@ tc_get_tc_cls_policy(enum tc_offload_policy policy)
  }
static void
+nl_msg_put_act_csum(struct ofpbuf *request, uint32_t flags)
+{
+    size_t offset;
+
+    nl_msg_put_string(request, TCA_ACT_KIND, "csum");
+    offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
+    {
+        struct tc_csum parm = { .action = TC_ACT_PIPE,
+                                .update_flags = flags };

I think it would be a bit nicer to move param to the top of the function
and remove the extra { } scope it is currently inside.

+
+        nl_msg_put_unspec(request, TCA_CSUM_PARMS, &parm, sizeof parm);
+    }
+    nl_msg_end_nested(request, offset);
+}
+
+static void
+nl_msg_put_act_pedit(struct ofpbuf *request, struct tc_pedit *parm,
+                     struct tc_pedit_key_ex *ex)
+{
+    size_t ksize = sizeof *parm + (parm->nkeys * sizeof(struct tc_pedit_key));
+    size_t offset, offset_keys_ex, offset_key;
+    int i;
+
+    nl_msg_put_string(request, TCA_ACT_KIND, "pedit");
+    offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
+    {

The extra { } scope here seems unnecessary.

+        parm->action = TC_ACT_PIPE;
+
+        nl_msg_put_unspec(request, TCA_PEDIT_PARMS_EX, parm, ksize);
+        offset_keys_ex = nl_msg_start_nested(request, TCA_PEDIT_KEYS_EX);
+        for (i = 0; i < parm->nkeys; i++, ex++) {
+            offset_key = nl_msg_start_nested(request, TCA_PEDIT_KEY_EX);
+            nl_msg_put_u16(request, TCA_PEDIT_KEY_EX_HTYPE, ex->htype);
+            nl_msg_put_u16(request, TCA_PEDIT_KEY_EX_CMD, ex->cmd);
+            nl_msg_end_nested(request, offset_key);
+        }
+        nl_msg_end_nested(request, offset_keys_ex);
+    }
+    nl_msg_end_nested(request, offset);
+}
+
+static void
  nl_msg_put_act_push_vlan(struct ofpbuf *request, uint16_t vid, uint8_t prio)
  {
      size_t offset;

...


Hi and thanks for the review,

The above style corresponds with the rest of the file (scope and nl_msg_put_act_* funcs) Maybe we do a style patch that remove all this extra scopes in a later patch?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to