Modifier: option to add empty fields

AdamZvara · sedmicha · commit b23d701aefd1 · 2025-03-04T15:53:19.000+01:00
diff --git a/include/ipfixcol2/modifier.h b/include/ipfixcol2/modifier.h
@@ -10,6 +10,8 @@
 
 #include <ipfixcol2/api.h>
 
+#define IPX_MODIFIER_SKIP -20
+
 /** Internal declaration of modifier */
 typedef struct ipx_modifier ipx_modifier_t;
 
@@ -35,8 +37,15 @@ struct ipx_modifier_output {
     uint8_t raw[UINT16_MAX];
 
     /** Length of returned data
-     *  If value is not available, length contains negative
-     *  number and represents error code */
+     *
+     *  If output is not available, length contains negative number and represents error code.
+     *  If length contains value #IPX_MODIFIER_SKIP, this field will not be used in modified record
+     *
+     * Example: {.raw = 10, .length =  2} => 2 bits are copied from output buffer with value 10
+     *          {.raw = 10, .length = -1} => no bits are copied, but modified record still
+     *                                          contains this field
+     *          {.raw = 10, .length = #IPX_MODIFIER_SKIP} => field is not copied to modified record
+     */
     int length;
 };
 
diff --git a/src/core/modifier.c b/src/core/modifier.c
@@ -520,7 +520,7 @@ new_template_size(const struct ipx_modifier_field *fields,
 {
     size_t new_size = 0;
     for (size_t i = 0; i < fields_cnt; i++) {
-        if (buffers[i].length >= 0) {
+        if (buffers[i].length != IPX_MODIFIER_SKIP) {
             new_size += (fields[i].en ? 8 : 4);
         }
     }
@@ -530,19 +530,27 @@ new_template_size(const struct ipx_modifier_field *fields,
 /**
  * \brief Calculate size needed for new data records
  *
+ * \param[in] fields     Field defintions
  * \param[in] buffers    Output buffers
  * \param[in] fields_cnt Fields array cnt
  * \return Space needed for new data records in output buffers
  */
 static inline size_t
-get_buffers_size(const struct ipx_modifier_output *buffers, const size_t fields_cnt)
+get_buffers_size(const struct ipx_modifier_field *fields, const struct ipx_modifier_output *buffers,
+    const size_t fields_cnt)
 {
     size_t new_size = 0;
     for (size_t i = 0; i < fields_cnt; i++) {
-        if (buffers[i].length >= 0) {
+        if (buffers[i].length == IPX_MODIFIER_SKIP) {
+            continue;
+        } else if (buffers[i].length >= 0) {
             // Take the worst case scanario where each new record
             // has variable length with 3 prefix octets
             new_size += buffers[i].length + 3;
+        } else {
+            // Invalid length in buffer but keep it ... use length from field defintion
+            // For variable length fields use smallest possible length (1 because of prefix octet)
+            new_size += fields[i].length == FDS_IPFIX_VAR_IE_LEN ? 1 : fields[i].length;
         }
     }
     return new_size;
@@ -608,7 +616,7 @@ ipfix_template_add_fields(const struct fds_template *tmplt,
 
     // Iterate through all valid values in output buffers and append their fields
     for (size_t i = 0; i < fields_cnt; i++) {
-        if (buffers[i].length >= 0) {
+        if (buffers[i].length != IPX_MODIFIER_SKIP) {
             field_size = ipfix_template_add_field(fields[i], (uint16_t *) iter);
             iter += field_size;
             tmplt_length += field_size;
@@ -648,7 +656,7 @@ ipfix_msg_add_drecs(struct fds_drec *rec,
     const size_t arr_cnt)
 {
     // Allocate memory for raw data record, copy previous record
-    size_t append_size = get_buffers_size(output, arr_cnt);
+    size_t append_size = get_buffers_size(fields, output, arr_cnt);
     uint8_t *new_data = malloc(rec->size + append_size);
     if (new_data == NULL) {
         return IPX_ERR_NOMEM;
@@ -658,15 +666,31 @@ ipfix_msg_add_drecs(struct fds_drec *rec,
     // Iterate through values in output buffer and append them to data record
     uint8_t *it = new_data + rec->size;
     for (size_t i = 0; i < arr_cnt; i++) {
-        if (output[i].length < 0) {
-            // Skip invalid value in buffer
+        if (output[i].length == IPX_MODIFIER_SKIP) {
+            // Skip field
+            continue;
+        } else if (output[i].length < 0) {
+            // Dont copy memory from buffer, but keep this field
+
+            if (fields[i].length == FDS_IPFIX_VAR_IE_LEN) {
+                // For variable length field append only prefix octet containing zero
+                *it = 0;
+                it++;
+                rec->size += 1;
+            } else {
+                memset(it, 0, fields[i].length);
+                it += fields[i].length;
+                rec->size += fields[i].length;
+            }
             continue;
         }
 
-        if (fields[i].length == FDS_IPFIX_VAR_IE_LEN) {
+        // Process output with normal length
+        else if (fields[i].length == FDS_IPFIX_VAR_IE_LEN) {
             // Append length octet prefix for variable length record
             if (output[i].length < 255) {
-                memcpy(it++, &(output[i].length), 1);
+                *it = output[i].length;
+                it++;
                 rec->size += 1;
             } else {
                 *it = 0xFF;
diff --git a/tests/unit/core/modifier/modifier_append_and_filter.cpp b/tests/unit/core/modifier/modifier_append_and_filter.cpp
@@ -152,7 +152,7 @@ void all_filter(uint8_t *filter) {
 }
 
 // Filter no fields from template
-TEST_F(Filter, ZeroTFields) {
+TEST_F(Filter, TemplateFields_None) {
     ASSERT_EQ(ipx_modifier_filter(&rec, filter), IPX_OK);
     fds_template *t = (fds_template *)(rec.tmplt);
 
@@ -170,7 +170,7 @@ TEST_F(Filter, ZeroTFields) {
 }
 
 // Filter out starting fields from template
-TEST_F(Filter, StartTFields) {
+TEST_F(Filter, TemplateFields_Start) {
     // Set filter
     start_filter(filter);
 
@@ -192,7 +192,7 @@ TEST_F(Filter, StartTFields) {
 }
 
 // Filter out last 2 fields from template
-TEST_F(Filter, EndTFields) {
+TEST_F(Filter, TemplateFields_End) {
     // Set filter
     end_filter(filter);
 
@@ -216,7 +216,7 @@ TEST_F(Filter, EndTFields) {
 }
 
 // Filter out random fields from template (start, middle, end fields)
-TEST_F(Filter, MixedTFields) {
+TEST_F(Filter, TemplateFields_MixedPosition) {
     // Set filter
     mixed_filter(filter);
 
@@ -238,7 +238,7 @@ TEST_F(Filter, MixedTFields) {
 }
 
 // Filter out all fields from template
-TEST_F(Filter, AllTFields) {
+TEST_F(Filter, TemplateFields_All) {
     // Set filter
     memset(filter, 1, tmplt->fields_cnt_total);
 
@@ -254,7 +254,7 @@ TEST_F(Filter, AllTFields) {
 }
 
 // Filtering zero fields from original data record
-TEST_F(Filter, NoneDrecs) {
+TEST_F(Filter, DataRecords_None) {
     uint16_t rec_prev_size = rec.size;
     uint8_t *orig_raw = new uint8_t[rec.size];
     memcpy(orig_raw, rec.data, rec.size);
@@ -272,7 +272,7 @@ TEST_F(Filter, NoneDrecs) {
 
 // Filtering out static (non variable length) fields from original data record
 // Also tests filtering last data record
-TEST_F(Filter, StaticLengthDrecs) {
+TEST_F(Filter, DataRecords_StaticLength) {
     uint16_t rec_prev_size = rec.size;
     static_filter(filter);
 
@@ -329,7 +329,7 @@ TEST_F(Filter, StaticLengthDrecs) {
 
 // Filtering out dynamic (variable length) fields from original data record
 // Also tests filtering first data record
-TEST_F(Filter, DynamicLengthDrecs) {
+TEST_F(Filter, DataRecords_VariableLength) {
     uint16_t rec_prev_size = rec.size;
     dynamic_filter(filter);
 
@@ -388,11 +388,6 @@ TEST_F(Filter, DynamicLengthDrecs) {
     fds_template_destroy((fds_template *)rec.tmplt);
 }
 
-TEST_F(Filter, InvalidArguments) {
-    EXPECT_EQ(ipx_modifier_filter(NULL, filter), IPX_ERR_ARG);
-    EXPECT_EQ(ipx_modifier_filter(&rec, NULL), IPX_ERR_ARG);
-}
-
 /** ------------------------- ADDING TESTS ------------------------- **/
 
 #define STATIC_CNT 4
@@ -447,7 +442,7 @@ class Adder : public Modifier {
             static_fields[i-1].en = i % 2;
             static_fields[i-1].id = i * 10;
             static_fields[i-1].length = i * 2;
-            static_output[i-1].length = -1;
+            static_output[i-1].length = IPX_MODIFIER_SKIP;
         }
     }
 
@@ -540,18 +535,42 @@ class Adder : public Modifier {
 
 };
 
-// Just copy template and drec, do not change anything
+// Append zero fields to record
 TEST_F(Adder, ZeroFields) {
-    // "Modify" record
+    // Modify record
     ASSERT_EQ(ipx_modifier_append(&rec, static_fields, static_output, STATIC_CNT), IPX_OK);
 
     // Check template and record
     cmp_template_overall(0, 0, 0);
     cmp_data_overall(0);
 }
 
+// Append zero fields to record but keep them in template and records as 0
+TEST_F(Adder, Static_ZeroFields_KeepEmptyOutputs) {
+    for (struct ipx_modifier_output &i : static_output) {
+        i.length = -1;
+    }
+    // Modify record
+    ASSERT_EQ(ipx_modifier_append(&rec, static_fields, static_output, STATIC_CNT), IPX_OK);
+
+    // Check template
+    cmp_template_overall(20, 4, 24);
+
+    // Check record
+    cmp_data_overall(20);
+
+    uint64_t zero = 0;
+    int p = 8;
+
+    for (auto i : static_fields) {
+        ASSERT_EQ(fds_drec_find(&rec, i.en, i.id, &field), p++);
+        cmp_data_record(field, &zero, i.length);
+    }
+
+}
+
 // Append single value to data record and template
-TEST_F(Adder, StaticSingleField) {
+TEST_F(Adder, Static_SingleField) {
     // Add single value to output buffer
     uint8_t pos = 1;
     uint32_t value = 422322;
@@ -572,7 +591,7 @@ TEST_F(Adder, StaticSingleField) {
 }
 
 // Append single value (EN specific) to data record and template
-TEST_F(Adder, StaticSingleFieldEN) {
+TEST_F(Adder, Static_SingleField_WithEN) {
     // Add single value (with EN) to output buffer
     uint8_t pos = 0;
     uint16_t value = 15213;
@@ -593,12 +612,11 @@ TEST_F(Adder, StaticSingleFieldEN) {
 }
 
 // Append multiple values (EN and non-EN specific) to data record and template
-TEST_F(Adder, StaticMultipleFields) {
+TEST_F(Adder, Static_MultipleFields) {
     // Add all values to output buffer
     int used_length = 0;
-    long value = 0;
+    long value = 0x123456789ABCDEF;
     for (uint8_t pos = 0; pos < STATIC_CNT; pos++) {
-        value = 0x123456789ABCDEF;
         ipx_modifier_field used = static_fields[pos];
         set_value(used, static_output[pos], &value, used.length);
         used_length += used.length;
@@ -627,8 +645,65 @@ TEST_F(Adder, StaticMultipleFields) {
     } while (fds_drec_iter_next(&it) != FDS_EOC);
 }
 
+// Append multiple values, some of which are kept in template and records with no values
+TEST_F(Adder, Static_MultipleFields_KeepEmptyOutputs) {
+    long value = 0x123456789ABCDEF;
+
+    // Keep first field (length = 2)
+    static_output[0].length = -1;
+    // Use second and third field (lengths = {4, 6})
+    set_value(static_fields[1], static_output[1], &value, 4);
+    set_value(static_fields[2], static_output[2], &value, 6);
+    // Do not use fourth field
+    static_output[3].length = IPX_MODIFIER_SKIP;
+
+    // Modify template
+    ASSERT_EQ(ipx_modifier_append(&rec, static_fields, static_output, STATIC_CNT), IPX_OK);
+
+    // Check modified template
+    cmp_template_overall(12, 3, 20); // 4 (non-EN) + 16 (2x8=EN)
+    for (int i = 0; i < 3; i++) {
+        cmp_template_field(rec.tmplt->fields_cnt_total - (3 - i), static_fields[i]);
+    }
+
+    // Check modified record
+    cmp_data_overall(12);
+    fds_drec_iter_init(&it, &rec, 0);
+    // Set iterator to first appended field
+    ASSERT_EQ(fds_drec_iter_find(&it, static_fields[0].en, static_fields[0].id), 8);
+    EXPECT_EQ(*(uint16_t *)it.field.data, 0);
+    ASSERT_EQ(fds_drec_iter_next(&it), 9);
+
+    // Iterate through all remaining data records and compare them
+    int size = 4;
+    do {
+        cmp_data_record(it.field, &value, size);
+        size += 2;
+    } while (fds_drec_iter_next(&it) != FDS_EOC);
+}
+
+// Append zero fields to record but keep them in template and records as 0
+TEST_F(Adder, Dynamic_ZeroFields_KeepEmptyOutputs) {
+    // Modify record
+    ASSERT_EQ(ipx_modifier_append(&rec, dynamic_fields, dynamic_output, DYNAMIC_CNT), IPX_OK);
+
+    // Check template
+    cmp_template_overall(2, 2, 8);
+
+    // Check record
+    cmp_data_overall(2);
+
+    uint64_t zero = 0;
+    int p = 8;
+
+    for (auto i : dynamic_fields) {
+        ASSERT_EQ(fds_drec_find(&rec, i.en, i.id, &field), p++);
+        cmp_data_record(field, &zero, 0);
+    }
+}
+
 // Append values with variable length (one with single prefix octet, second with 3 prefix octets)
-TEST_F(Adder, DynamicFields) {
+TEST_F(Adder, Dynamic_MultipleFields) {
     // Add all values to output buffer
     uint8_t values[1000] = {0x12, 0x34, 0x56, };
     values[678] = 0x78;
@@ -658,10 +733,4 @@ TEST_F(Adder, DynamicFields) {
     cmp_data_record(it.field, &values, 1000);
 
     EXPECT_EQ(fds_drec_iter_next(&it), FDS_EOC);
-}
-
-TEST_F(Adder, InvalidArguments) {
-    EXPECT_EQ(ipx_modifier_append(NULL, static_fields, static_output, 0), IPX_ERR_ARG);
-    EXPECT_EQ(ipx_modifier_append(&rec, NULL, static_output, 0), IPX_ERR_ARG);
-    EXPECT_EQ(ipx_modifier_append(&rec, static_fields, NULL, 0), IPX_ERR_ARG);
 }
