Hi Daniele,

> Have you tried avoiding also computing the new field if the mask is 0?
> For example, if
> mask->ipv4_src is 0, there's not reason to compute new_ip_src, or to
> extract ip_src_nh.

Yes, I have investigated this case in mid-October. My results was 
that execution of "output + dec_ttl" and "output + mod_nw_tos" became even 
faster, but execution of rest of the actions I investigated became slower
compared to the patch I posted previously. 
So I rejected this modification.

Now, I rebased the code to 9th Dec master and ran the tests with DPDK 16.11 
again. Please find the new rebased patch below.
The results of my original and the new patch:

       | T | T | I | I |                 
       | T | O | P | P |  Vanilla OVS  ||  + old patch  |  + new patch
       | L | S | s | d | (nsec/packet) || (nsec/packet) | (nsec/packet)
-------+---+---+---+---+---------------++---------------+---------------   
output |   |   |   |   |    67.19      ||    67.19      |    67.19        
       | X |   |   |   |    74.48      ||    69.53      |    68.78    (+)
       |   | X |   |   |    74.42      ||    71.20      |    70.07    (+) 
       |   |   | X |   |    84.62      ||    78.70      |    78.03    (+) 
       |   |   |   | X |    84.25      ||    78.51      |    77.94    (+) 
       |   |   | X | X |    97.46      ||    91.98      |    91.86    (+) 
       | X |   | X | X |   100.42      ||    95.00      |    96.00    (-) 
       | X | X | X | X |   102.80      ||   100.40      |   100.73    (-) 
       
I ran each test five times. The values are the mean of the readings obtained.

As you can see, there is an improvement in the first 5 cases. But, in case of 
"output + dec_ttl + mod_nw_src + mod_nw_dst" and 
"output + dec_ttl + mod_nw_tos + mod_nw_src + mod_nw_dst", the execution of
actions became slower compared to the old patch. However, this is still faster 
than vanilla OVS.

Could you please confirm the results? 
Are these values acceptable?
Is there anything still to improve in the new patch?

The patch was applied to: 7971b36c3acc279f8e931d360f16c200752a3be2

Best regards,
Zoltan

Signed-off-by: Zoltán Balogh <[email protected]>

---

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 65a6fcd..cc555b8 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -33,6 +33,7 @@
 #include "flow.h"
 #include "unaligned.h"
 #include "util.h"
+#include "csum.h"
 
 /* Masked copy of an ethernet address. 'src' is already properly masked. */
 static void
@@ -68,13 +69,50 @@ odp_set_ipv4(struct dp_packet *packet, const struct 
ovs_key_ipv4 *key,
              const struct ovs_key_ipv4 *mask)
 {
     struct ip_header *nh = dp_packet_l3(packet);
+    ovs_be32 ip_src_nh;
+    ovs_be32 ip_dst_nh;
+    ovs_be32 new_ip_src;
+    ovs_be32 new_ip_dst;
+    uint8_t new_tos;
+    uint8_t new_ttl;
+
+    if (mask->ipv4_src) {
+        ip_src_nh = get_16aligned_be32(&nh->ip_src);
+        new_ip_src = key->ipv4_src | (ip_src_nh & ~mask->ipv4_src);
+
+        if (ip_src_nh != new_ip_src) {
+            packet_set_ipv4_addr(packet, &nh->ip_src, new_ip_src);
+        }
+    }
 
-    packet_set_ipv4(
-        packet,
-        key->ipv4_src | (get_16aligned_be32(&nh->ip_src) & ~mask->ipv4_src),
-        key->ipv4_dst | (get_16aligned_be32(&nh->ip_dst) & ~mask->ipv4_dst),
-        key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos),
-        key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl));
+    if (mask->ipv4_dst) {
+        ip_dst_nh = get_16aligned_be32(&nh->ip_dst);
+        new_ip_dst = key->ipv4_dst | (ip_dst_nh & ~mask->ipv4_dst);
+
+        if (ip_dst_nh != new_ip_dst) {
+            packet_set_ipv4_addr(packet, &nh->ip_dst, new_ip_dst);
+        }
+    }
+
+    if (mask->ipv4_tos) {
+        new_tos = key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos);
+
+        if (nh->ip_tos != new_tos) {
+            nh->ip_csum = recalc_csum16(nh->ip_csum, htons((uint16_t) 
nh->ip_tos),
+                                        htons((uint16_t) new_tos));
+            nh->ip_tos = new_tos;
+        }
+    }
+
+    if (OVS_LIKELY(mask->ipv4_ttl)) {
+        new_ttl = key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl);
+
+        if (OVS_LIKELY(nh->ip_ttl != new_ttl)) {
+            nh->ip_csum = recalc_csum16(nh->ip_csum, htons(nh->ip_ttl << 8),
+                                        htons(new_ttl << 8));
+            nh->ip_ttl = new_ttl;
+        }
+    }
 }
 
 static const ovs_be32 *
diff --git a/lib/packets.c b/lib/packets.c
index ae81f46..4a5eb72 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -849,7 +849,7 @@ eth_compose(struct dp_packet *b, const struct eth_addr 
eth_dst,
     return data;
 }
 
-static void
+void
 packet_set_ipv4_addr(struct dp_packet *packet,
                      ovs_16aligned_be32 *addr, ovs_be32 new_addr)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 21bd35c..97d1b38 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1093,6 +1093,8 @@ void *snap_compose(struct dp_packet *, const struct 
eth_addr eth_dst,
                    unsigned int oui, uint16_t snap_type, size_t size);
 void packet_set_ipv4(struct dp_packet *, ovs_be32 src, ovs_be32 dst, uint8_t 
tos,
                      uint8_t ttl);
+void packet_set_ipv4_addr(struct dp_packet *packet, ovs_16aligned_be32 *addr,
+                          ovs_be32 new_addr);
 void packet_set_ipv6(struct dp_packet *, const ovs_be32 src[4],
                      const ovs_be32 dst[4], uint8_t tc,
                      ovs_be32 fl, uint8_t hlmit);


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

Reply via email to