diff mbox series

[ovs-dev,v2,6/7] tc: Pass encap entirely to nl_msg_put_act_tunnel_key_set

Message ID 20230425124122.2443376-7-roid@nvidia.com
State Changes Requested
Headers show
Series Add vxlan gbp offload with TC | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Roi Dayan April 25, 2023, 12:41 p.m. UTC 
From: Gavin Li <gavinl@nvidia.com>

Most of the data members of struct tc_action{ } are defined as anonymous
struct in place. Instead of passing all members of an anonymous struct,
which is not flexible to new members being added, expose encap as named
struct and pass it entirely.

Signed-off-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
---
 lib/tc.c | 57 +++++++++++++++++++++++---------------------------------
 lib/tc.h | 38 +++++++++++++++++++------------------
 2 files changed, 43 insertions(+), 52 deletions(-)

Comments

Simon Horman May 10, 2023, 3:24 p.m. UTC | #1 
On Tue, Apr 25, 2023 at 03:41:21PM +0300, Roi Dayan via dev wrote:
> From: Gavin Li <gavinl@nvidia.com>
> 
> Most of the data members of struct tc_action{ } are defined as anonymous
> struct in place. Instead of passing all members of an anonymous struct,
> which is not flexible to new members being added, expose encap as named
> struct and pass it entirely.
> 
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> Reviewed-by: Roi Dayan <roid@nvidia.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
diff mbox series

Patch

diff --git a/lib/tc.c b/lib/tc.c
index e72fa4235198..a4f88f4711e4 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2626,13 +2626,9 @@  nl_msg_put_act_tunnel_geneve_option(struct ofpbuf *request,
 }
 
 static void
-nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, bool id_present,
-                              ovs_be64 id, ovs_be32 ipv4_src,
-                              ovs_be32 ipv4_dst, struct in6_addr *ipv6_src,
-                              struct in6_addr *ipv6_dst,
-                              ovs_be16 tp_dst, uint8_t tos, uint8_t ttl,
-                              struct tun_metadata *tun_metadata,
-                              uint8_t no_csum, uint32_t action_pc)
+nl_msg_put_act_tunnel_key_set(struct ofpbuf *request,
+                              struct tc_action_encap *encap,
+                              uint32_t action_pc)
 {
     size_t offset;
 
@@ -2644,30 +2640,33 @@  nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, bool id_present,
 
         nl_msg_put_unspec(request, TCA_TUNNEL_KEY_PARMS, &tun, sizeof tun);
 
-        ovs_be32 id32 = be64_to_be32(id);
-        if (id_present) {
+        ovs_be32 id32 = be64_to_be32(encap->id);
+        if (encap->id_present) {
             nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_KEY_ID, id32);
         }
-        if (ipv4_dst) {
-            nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_SRC, ipv4_src);
-            nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_DST, ipv4_dst);
-        } else if (ipv6_addr_is_set(ipv6_dst)) {
+        if (encap->ipv4.ipv4_dst) {
+            nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_SRC,
+                            encap->ipv4.ipv4_src);
+            nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_DST,
+                            encap->ipv4.ipv4_dst);
+        } else if (ipv6_addr_is_set(&encap->ipv6.ipv6_dst)) {
             nl_msg_put_in6_addr(request, TCA_TUNNEL_KEY_ENC_IPV6_DST,
-                                ipv6_dst);
+                                &encap->ipv6.ipv6_dst);
             nl_msg_put_in6_addr(request, TCA_TUNNEL_KEY_ENC_IPV6_SRC,
-                                ipv6_src);
+                                &encap->ipv6.ipv6_src);
         }
-        if (tos) {
-            nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TOS, tos);
+        if (encap->tos) {
+            nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TOS, encap->tos);
         }
-        if (ttl) {
-            nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TTL, ttl);
+        if (encap->ttl) {
+            nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TTL, encap->ttl);
         }
-        if (tp_dst) {
-            nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT, tp_dst);
+        if (encap->tp_dst) {
+            nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT,
+                            encap->tp_dst);
         }
-        nl_msg_put_act_tunnel_geneve_option(request, tun_metadata);
-        nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, no_csum);
+        nl_msg_put_act_tunnel_geneve_option(request, &encap->data);
+        nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, encap->no_csum);
     }
     nl_msg_end_nested(request, offset);
 }
@@ -3290,17 +3289,7 @@  nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
                 }
 
                 act_offset = nl_msg_start_nested(request, act_index++);
-                nl_msg_put_act_tunnel_key_set(request, action->encap.id_present,
-                                              action->encap.id,
-                                              action->encap.ipv4.ipv4_src,
-                                              action->encap.ipv4.ipv4_dst,
-                                              &action->encap.ipv6.ipv6_src,
-                                              &action->encap.ipv6.ipv6_dst,
-                                              action->encap.tp_dst,
-                                              action->encap.tos,
-                                              action->encap.ttl,
-                                              &action->encap.data,
-                                              action->encap.no_csum,
+                nl_msg_put_act_tunnel_key_set(request, &action->encap,
                                               action_pc);
                 nl_msg_put_act_flags(request);
                 nl_msg_end_nested(request, act_offset);
diff --git a/lib/tc.h b/lib/tc.h
index 95fff37b9b61..1d648282a004 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -210,6 +210,25 @@  enum nat_type {
     TC_NAT_RESTORE,
 };
 
+struct tc_action_encap {
+    bool id_present;
+    ovs_be64 id;
+    ovs_be16 tp_src;
+    ovs_be16 tp_dst;
+    uint8_t tos;
+    uint8_t ttl;
+    uint8_t no_csum;
+    struct {
+        ovs_be32 ipv4_src;
+        ovs_be32 ipv4_dst;
+    } ipv4;
+    struct {
+        struct in6_addr ipv6_src;
+        struct in6_addr ipv6_dst;
+    } ipv6;
+    struct tun_metadata data;
+};
+
 struct tc_action {
     union {
         int chain;
@@ -233,24 +252,7 @@  struct tc_action {
             uint8_t bos;
         } mpls;
 
-        struct {
-            bool id_present;
-            ovs_be64 id;
-            ovs_be16 tp_src;
-            ovs_be16 tp_dst;
-            uint8_t tos;
-            uint8_t ttl;
-            uint8_t no_csum;
-            struct {
-                ovs_be32 ipv4_src;
-                ovs_be32 ipv4_dst;
-            } ipv4;
-            struct {
-                struct in6_addr ipv6_src;
-                struct in6_addr ipv6_dst;
-            } ipv6;
-            struct tun_metadata data;
-        } encap;
+        struct tc_action_encap encap;
 
         struct {
             uint16_t zone;