As discussed here [0], a couple of functions that encode
CT-related actions were using older, manual, way of finishing
the action.

As amusil mentioned here [1], there's a shorter and more explicit
way of doing it. This change replaces manual way with the more
explicit aproach.

[0] https://mail.openvswitch.org/pipermail/ovs-dev/2024-June/414667.html
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2024-April/413317.html

Signed-off-by: Martin Kalcok <[email protected]>
---
 lib/actions.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/lib/actions.c b/lib/actions.c
index e8cc0994d..51da6210f 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -761,7 +761,6 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on,
                     struct ofpbuf *ofpacts)
 {
     size_t ct_offset = ofpacts->size;
-    ofpbuf_pull(ofpacts, ct_offset);
 
     struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
     ct->flags = NX_CT_F_COMMIT;
@@ -777,24 +776,19 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on,
      */
     if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) {
         size_t nat_offset = ofpacts->size;
-        ofpbuf_pull(ofpacts, nat_offset);
 
         struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
         nat->flags = 0;
         nat->range_af = AF_UNSPEC;
         nat->flags |= NX_NAT_F_SRC;
-        ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
-        ct = ofpacts->header;
+        ct = ofpbuf_at_assert(ofpacts, nat_offset, sizeof *ct);
     }
 
-    size_t set_field_offset = ofpacts->size;
-    ofpbuf_pull(ofpacts, set_field_offset);
-
     ovnacts_encode(on->nested, on->nested_len, ep, ofpacts);
-    ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset);
-    ct = ofpacts->header;
-    ofpact_finish(ofpacts, &ct->ofpact);
-    ofpbuf_push_uninit(ofpacts, ct_offset);
+
+    ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct);
+    ofpacts->header = ct;
+    ofpact_finish_CT(ofpacts, &ct);
 }
 
 static void
@@ -1027,7 +1021,6 @@ encode_ct_nat(const struct ovnact_ct_nat *cn,
               enum mf_field_id zone_src, struct ofpbuf *ofpacts)
 {
     const size_t ct_offset = ofpacts->size;
-    ofpbuf_pull(ofpacts, ct_offset);
 
     struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
     ct->recirc_table = cn->ltable + first_ptable(ep, ep->pipeline);
@@ -1040,7 +1033,6 @@ encode_ct_nat(const struct ovnact_ct_nat *cn,
     struct ofpact_nat *nat;
     size_t nat_offset;
     nat_offset = ofpacts->size;
-    ofpbuf_pull(ofpacts, nat_offset);
 
     nat = ofpact_put_NAT(ofpacts);
     nat->range_af = cn->family;
@@ -1081,13 +1073,13 @@ encode_ct_nat(const struct ovnact_ct_nat *cn,
         }
     }
 
-    ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
-    ct = ofpacts->header;
+    ct = ofpbuf_at_assert(ofpacts, nat_offset, sizeof *ct);
+    ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct);
     if (cn->commit) {
         ct->flags |= NX_CT_F_COMMIT;
     }
-    ofpact_finish(ofpacts, &ct->ofpact);
-    ofpbuf_push_uninit(ofpacts, ct_offset);
+    ofpacts->header = ct;
+    ofpact_finish_CT(ofpacts, &ct);
 }
 
 static void
@@ -1383,7 +1375,6 @@ encode_ct_lb(const struct ovnact_ct_lb *cl,
         /* ct_lb without any destinations means that this is an established
          * connection and we just need to do a NAT. */
         const size_t ct_offset = ofpacts->size;
-        ofpbuf_pull(ofpacts, ct_offset);
 
         struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
         struct ofpact_nat *nat;
@@ -1397,16 +1388,15 @@ encode_ct_lb(const struct ovnact_ct_lb *cl,
         ct->alg = 0;
 
         nat_offset = ofpacts->size;
-        ofpbuf_pull(ofpacts, nat_offset);
 
         nat = ofpact_put_NAT(ofpacts);
         nat->flags = 0;
         nat->range_af = AF_UNSPEC;
 
-        ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
-        ct = ofpacts->header;
-        ofpact_finish(ofpacts, &ct->ofpact);
-        ofpbuf_push_uninit(ofpacts, ct_offset);
+        ct = ofpbuf_at_assert(ofpacts, nat_offset, sizeof *ct);
+        ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct);
+        ofpacts->header = ct;
+        ofpact_finish_CT(ofpacts, &ct);
         return;
     }
 
-- 
2.40.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to